Page MenuHomeFreeBSD

irc/scrollz: Update to 2.3
AbandonedPublic

Authored by chris_bsdjunk.com on Mar 3 2016, 5:51 AM.
Tags
None
Referenced Files
F86189571: D5527.id14023.diff
Sun, Jun 16, 6:26 PM
Unknown Object (File)
Thu, Jun 13, 7:56 PM
Unknown Object (File)
May 15 2024, 1:46 PM
Unknown Object (File)
May 14 2024, 11:27 PM
Unknown Object (File)
May 14 2024, 3:55 PM
Unknown Object (File)
May 14 2024, 3:26 PM
Unknown Object (File)
May 12 2024, 2:53 AM
Unknown Object (File)
May 11 2024, 9:53 PM
Subscribers
None

Details

Reviewers
koobs
Summary

irc/scrollz: Summary

  • Update PORTVERSION and distinfo checksum (2.3)
  • Take MAINTAINERSHIP
  • Change configure options to port options (Barnerd)
  • Update pkg-list (Barnerd)
  • Remove REGEX_DESC and add it to bsd.options.desc.mk
  • Use github for disfiles
Test Plan
  • portlint: OK (portlint -A)
  • testport: OK (poudriere: 10amd64)

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

chris_bsdjunk.com retitled this revision from to Update irc scrollz to 2.3.
chris_bsdjunk.com updated this object.
chris_bsdjunk.com edited the test plan for this revision. (Show Details)
chris_bsdjunk.com added a reviewer: koobs.
chris_bsdjunk.com set the repository for this revision to rP FreeBSD ports repository.
chris_bsdjunk.com added a subscriber: koobs.
scrollz/Makefile
4 ↗(On Diff #14007)

This should be ${GH_ACCOUNT:tl}, or even, it should be the proper case, and then you don't have to define GH_* variables.

7 ↗(On Diff #14007)

This is not needed.

18 ↗(On Diff #14007)

This should be ${GH_ACCOUNT}

19 ↗(On Diff #14007)

This should be DISTVERSIONPREFIX=${GH_ACCOUNT}- in the top block.

Also when you upload a diff manually (as opposed to using arcanist), you should add more context so that the reviewer can have more context too. With svn or git:

svn diff -x -U9999
git diff -U9999

scrollz-2.3 depends on file: /usr/local/sbin/pkg - found
ScrollZ-scrollz-ScrollZ-2.3_GH0.tar.gz doesn't seem to exist in /usr/ports/distfiles/.
Attempting to fetch https://codeload.github.com/ScrollZ/scrollz/tar.gz/ScrollZ-2.3?dummy=/ScrollZ-scrollz-ScrollZ-2.3_GH0.tar.gz
ScrollZ-scrollz-ScrollZ-2.3_GH0.tar.gz 100% of 958 kB 922 kBps 00m01s
Fetching all distfiles required by scrollz-2.3 for building

After doing the above. "it works but looks ugly" is this how github names files or am I missing something ?

This comment was removed by chris_bsdjunk.com.

Update patch to diff -x -U9999 as I didn't see the - in -U

After doing the above. "it works but looks ugly" is this how github names files or am I missing something ?

Yes, it is how it is supposed to be, do not change the DISTNAME of a USE_GITHUB port. That way, it is guaranteed to be unique.

Update patch to diff -x -U9999 as I didn't see the - in -U

Thank you. When you do that, the reviewer get nice "show more lines" stuff, and we can have a look around to see if we find something strange, or bad, or useless, or... ?

scrollz/Makefile
7 ↗(On Diff #14020)

This should be above the CATEGORIES line, I think portlint will complain about it.

Update patch to change orientation of DISTVERSIONPREFIX

scrollz/Makefile
4 ↗(On Diff #14023)

Is there a specific reason why you don't want to have PORTNAME=ScrollZ ? I feel it'd be much simpler.

comments

scrollz/Makefile
4 ↗(On Diff #14023)

Then we would need to change the name of irc/bitchx to irc/BitchX :) but I hoenstly don't mind figured there was a reason why irc/BitchX was lower cased so I kept it?

chris_bsdjunk.com marked 2 inline comments as done.

Update with options from Barnerd and pkg-plist update. however I am unsure why it fails now
http://pkgs.bsdjunk.com/data/10amd64-cpet/2016-03-03_10h49m25s/logs/errors/ScrollZ-2.3.log

If anyone can shed some light as the only difference is a few additions. But it will no longer build.

What does make -V DATADIR says ? I guess it's installing in share/scrollz and DATADIR contains share/ScrollZ.

% make -VDATADIR
/usr/local/share/ScrollZ
%

however that shouldn't matter as %%DATADIR%% =PREFIX/share/PORTNAME.

Yes.

I'll repeat.

It's installing in share/scrollz (all lowercase) and datadir is share/ScrollZ (with caps)

I fixed it by re adding the older bits:

PORTNAME= ${GH_ACCOUNT:tl}
PORTVERSION= 2.3
DISTVERSIONPREFIX= ${GH_ACCOUNT}-
CATEGORIES= irc ipv6

USE_GITHUB= yes
GH_ACCOUNT= ScrollZ
GH_PROJECT= ScrollZ

But I believe that shouldn't matter but in reality it does.

http://pkgs.bsdjunk.com/data/10amd64-cpet/2016-03-03_17h02m52s/logs/scrollz-2.3.log
vs
http://pkgs.bsdjunk.com/data/10amd64-cpet/2016-03-03_11h28m46s/logs/errors/ScrollZ-2.3.log

No.

The correct fix is either to set DATADIR=${PREFIX}/share/scrollz, or to pass DATADIR to the port so that it installs in the right place.

Sorry I understand now,

I change it to:
PLIST_SUB+= SCROLLZ_VER="${PORTVERSION}" DATADIR="${DATADIR}"

chris_bsdjunk.com removed rP FreeBSD ports repository as the repository for this revision.

Hopefully this is the last one :)

I need to mention that the Options things and the pkg-desc edit was originally taken from Barnerd's review.
https://reviews.freebsd.org/D4847

Fix typo --with-ssil --with-ssl pointed out by Barnerd on IRC

irc/scrollz/Makefile
21

You don't need to add DATADIR here, it already is present.

Update patch to remove PLIST= .. DATADIR.. tested with make check-plist.

Thank you mat for taking the time to help me update irc/scrollz ..

no problem, I'm the one stupid enough to have asked phabricator to subscribe me to every ports review, might as well do something with it :-)

In D5527#118390, @mat wrote:

no problem, I'm the one stupid enough to have asked phabricator to subscribe me to every ports review, might as well do something with it :-)

:)

Can I tell the commiter to commit this ?

koobs requested changes to this revision.Mar 5 2016, 4:36 AM
koobs edited edge metadata.
koobs added inline comments.
irc/scrollz/Makefile
25–28

It looks like bsd.options.desc.mk only contains a PCRE option, which specifically only stipulates 'perl-compatible regular expressions'. Perhaps this is a good time to create a REGEX shared option description ?

This revision now requires changes to proceed.Mar 5 2016, 4:36 AM
chris_bsdjunk.com edited edge metadata.

Update bsd.options.desc.mk to include REGEX and make changes to suite.

would this not break every port now that has a REGEX_DESCR= listed ?

% find /backup/ports/ -name "Makefile" -exec grep REGEX_DESC {} + | wc -l

5

%

koobs requested changes to this revision.EditedMar 5 2016, 5:42 AM
koobs edited edge metadata.

would this not break every port now that has a REGEX_DESCR= listed ?

No, they will just not inherit the description from bsd.options.desc.mk, since its defined in the port itself.

Since a shared REGEX_DESC has been created, it's probably worth removing REGEX_DESC from those ports where the description exactly matches the shared one:

/usr/ports/www/libevhtp/Makefile:REGEX_DESC= Enable regex support

The other matches have more specific descriptions, which may mean the shared description is not suitable or specific enough:

/usr/ports/mail/perdition/Makefile:POSIX_REGEX_DESC= Native regex support
/usr/ports/textproc/libucl/Makefile:REGEX_DESC= Enable regex checking for schema
/usr/ports/security/pam-modules/Makefile:REGEX_DESC= Build pam_regex
/usr/ports/audio/pms/Makefile:REGEX_DESC= Support boost regex pattern matching

Mk/bsd.options.desc.mk
418

Minor nit, use the singular: Regular expression support

This revision now requires changes to proceed.Mar 5 2016, 5:42 AM
chris_bsdjunk.com updated this object.
chris_bsdjunk.com edited edge metadata.
chris_bsdjunk.com marked an inline comment as done.

Update REGEX to not be plural as suggested by koobs

koobs retitled this revision from Update irc scrollz to 2.3 to irc/scrollz: Update to 2.3.Mar 5 2016, 10:26 AM
koobs updated this object.
koobs edited the test plan for this revision. (Show Details)
koobs edited the test plan for this revision. (Show Details)
koobs removed subscribers: mat, koobs.
koobs updated this object.

I forgot I submitted a PR for this and asked vg@ if he could take care of it.

Seems fine now PR is opened for this as 207674