sysutils/py-diffoscope: Update to 86
ClosedPublic

Authored by ygy on Sep 12 2017, 4:58 AM.

Details

Summary
sysutils/py-diffoscope: Update to 86

 * Add textproc/diffutils as a dependency

PR: 220870
Approved by: koobs (maintainer)
Differential_Revision: D12328
Test Plan
  • portlint: OK
  • testport: OK (log attached)
  • maketest: OK (known failures due to upstream incompatibility with FreeBSD)

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ygy created this revision.Sep 12 2017, 4:58 AM
lwhsu added a reviewer: koobs.Sep 12 2017, 8:51 AM
koobs requested changes to this revision.Sep 12 2017, 9:09 AM
koobs retitled this revision from Update sysutils/py-diffoscope to version 86 to sysutils/py-diffoscope: Update to 86.
koobs edited the summary of this revision. (Show Details)
koobs edited the test plan for this revision. (Show Details)

Syntactically looks OK. Please confirm QA results/OK in TEST PLAN section

I've updated the review description to be a well-formed commit log message (subject to review also)

This revision now requires changes to proceed.Sep 12 2017, 9:10 AM
ygy added a comment.Sep 12 2017, 9:38 AM

Syntactically looks OK. Please confirm QA results/OK in TEST PLAN section

I've updated the review description to be a well-formed commit log message (subject to review also)

It is my first review for ports, thus I don't really know what to put in the test plan section, sorry!

This patch is tested and the ports can be installed successfully on 12-CURRENT.

koobs added a comment.Sep 12 2017, 1:14 PM

I added some details (to the TEST PLAN section of the review) of what needs to be tested (portlint, poudriere, make test). Just edit the review and add the results.

In particular, poudriere tests for complete packaging compliance (not just build or install)

For more information on testing, see: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing.html

If you have any questions don't hesitate to ask, and thank you for contributing :)

ygy edited the test plan for this revision. (Show Details)Sep 13 2017, 1:15 PM
ygy added a comment.Sep 13 2017, 4:33 PM

I added some details (to the TEST PLAN section of the review) of what needs to be tested (portlint, poudriere, make test). Just edit the review and add the results.

In particular, poudriere tests for complete packaging compliance (not just build or install)

For more information on testing, see: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing.html

If you have any questions don't hesitate to ask, and thank you for contributing :)

Thank you for the detailed instructions! It passed the portlint test, however, make test initially failed since it is looking for /usr/local/bin/perl5.24.2, but only /usr/local/bin/perl5.24.1 and /usr/local/bin/perl are available. I made a symlink to work it around (sudo ln -s /usr/local/bin/perl5.24.1 /usr/local/bin/perl5.24.2), but I am not sure what files to look at, and if it is a bug worth reporting.

The log is shown below:

➜ py-diffoscope (D12328) ✔ sudo make test

> Testing for py36-diffoscope-86

[...]

> p5-Locale-gettext-1.07 depends on package: perl5>=5.24<5.25 - found

> p5-Locale-gettext-1.07 depends on shared library: libintl.so - found (/usr/local/lib/libintl.so)

> Configuring for p5-Locale-gettext-1.07

env: /usr/local/bin/perl5.24.2: No such file or directory

  • Error code 127

Stop.
make[12]: stopped in /usr/home/guangyuan/freebsd-ports/devel/p5-Locale-gettext

  • Error code 1

[...]

Stop.
make: stopped in /usr/home/guangyuan/freebsd-ports/sysutils/py-diffoscope

This may be a perl ports framework issue or bug, as the port dependency is declared and satisfied at the first minor version > 5.24 < 5.25 level, but the framework (configure) target uses an exact build version (5.24.2) level to check for perl. There are some similar (recent, June, July, August) google results for the same error, for mytop, firefox, chromium and other ports.

However, it may have already been fixed, so first: ensure your ports tree is up to date and try again. If you're using quarterly packages, switch to 'latest'

If those dont' work:

Upgrade perl5.24 to the latest version (to 5.24.2) first on the system, then try again.

@mat may be able to shed some more light if the above is incorrect/incomplete or provide additional suggestions.

mat added a comment.Sep 14 2017, 2:59 PM

We do not support having out of sync ports tree vs /usr/local.

You need to upgrade your system to use the latest version of every software in the ports tree your are using for your tests.

Alternatively, you should read 9.5. Poudriere to learn how to use poudriere and never have those kind of issues when testing ports.

ygy added a comment.Sep 22 2017, 10:19 AM
In D12328#256592, @mat wrote:

We do not support having out of sync ports tree vs /usr/local.

You need to upgrade your system to use the latest version of every software in the ports tree your are using for your tests.

Alternatively, you should read 9.5. Poudriere to learn how to use poudriere and never have those kind of issues when testing ports.

@mat @koobs Thank you for the detailed guidance! I just upgraded my laptop to 12-CURRENT and will make sure I am always using "latest" for pkg/ports. I also learned Poudriere just now so I will be able to post the test results here soon.

ygy added a comment.Sep 26 2017, 5:31 PM

Hi, I finished a Poudriere testport, but am not sure what to look for and what to expect. The log looked fine to me though.

koobs added a comment.Sep 27 2017, 4:30 AM

@ygy Output looks fine (generally, look for warnings or exits with a failure/fatal error to identify issues)

Please update the TEST PLAN section with "OK" results

Still pending make test (test suite) output/confirmation

ygy added a comment.Sep 27 2017, 6:24 AM

@ygy Output looks fine (generally, look for warnings or exits with a failure/fatal error to identify issues)

Please update the TEST PLAN section with "OK" results

Still pending make test (test suite) output/confirmation

make test will successfully run with several (~10) tests failed. This is somewhat expected since Version 84 has test fails as well (see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220870). Will follow up the PR or create a new one for tracking, but that doesn't really affect this update for now IMO.

TEST PLAN is updated.

ygy edited the test plan for this revision. (Show Details)Sep 27 2017, 6:26 AM
koobs accepted this revision.Sep 27 2017, 6:28 AM
koobs edited the test plan for this revision. (Show Details)

LGTM, nice work!

Don't forget to remove any underscores from Property_lines: if/when copy/pasting commit log message from the SUMMARY section, otherwise the review wont automatically close on commit

This revision is now accepted and ready to land.Sep 27 2017, 6:29 AM
koobs edited the summary of this revision. (Show Details)Sep 27 2017, 6:29 AM
ygy added a reviewer: loader.Sep 27 2017, 9:09 AM

LGTM, nice work!

Don't forget to remove any underscores from Property_lines: if/when copy/pasting commit log message from the SUMMARY section, otherwise the review wont automatically close on commit

Thank you for all the guidance! @loader not sure if I still need to get mentor's approval, better safe than sorry XD

loader removed a reviewer: loader.Sep 27 2017, 10:58 AM
loader added a subscriber: loader.
In D12328#259536, @ygy wrote:

@loader not sure if I still need to get mentor's approval, better safe than sorry XD

Nope, only on the doc/zh_CN tree ...

emaste accepted this revision.Sep 27 2017, 5:25 PM

Nope, only on the doc/zh_CN tree ...

Our standard approach for cross-repo commits (doc committer committing to ports tree, ports committer committing to src tree, etc.) is that one asks for approval from a (non-mentored) committer to the appropriate repo. In this case @koobs is a ports committer in addition to maintainer of the py-diffoscope port, and the acceptance here is suitable for committing the ports change. It's still reasonable for the mentor to provide guidance/advice on this process though.

Hi @emaste, thank you for clarifying that. I mean I don't have a ports commit bit, I can only give an approval to commit to doc/zh_CN, Both @koobs and @mat are more professional than me, they are all very kind people and helped me a lot in the past. I konw @ygy is definitely in good hands.

This revision was automatically updated to reflect the committed changes.