Bug 240030

Summary: dns/py-tldextract: Update to 2.2.1
Product: Ports & Packages Reporter: Adam Jimerson <vendion>
Component: Individual Port(s)Assignee: Dan Langille <dvl>
Status: Closed FIXED    
Severity: Affects Only Me CC: koobs, python, vendion
Priority: --- Flags: bugzilla: maintainer-feedback? (dvl)
Version: Latest   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Updated tldextract files
none
New py-tldextract patch none

Description Adam Jimerson 2019-08-22 03:19:32 UTC
Created attachment 206775 [details]
Updated tldextract files

This contains an updated Makefile and distinfo file for tldextract 2.2.1 as well as adds the pkg-plist file which was missing.
Comment 1 Adam Jimerson 2019-08-22 03:21:08 UTC
I'm still new to writing/updating port makefiles, although this is something I would like to get into. If I missed something or did something wrong let me know so I can correct.
Comment 2 Dan Langille freebsd_committer 2019-08-22 13:39:47 UTC
Jim,

The patch is a shar file, which is proper for a new port.

For an update to an existing port, a diff is what should be supplied.

See https://www.freebsd.org/doc/en/books/porters-handbook/porting-submitting.html

Suggestion: submit a new patch as a diff.

For the supplied patch:

* I tested it, it works
* the pkg-plist file is not required because Makefile contains: distutils

There is a small discussion about that here: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/using-python.html but note the formatting for Table 6.26 is mildly wonky.

Good start, thank you.
Comment 3 Adam Jimerson 2019-08-22 13:52:25 UTC
Thanks for the feedback Dan, I was unsure about the best way to the patch as my ports tree is from portsnap and thus not a subversion repo so the subversion commands didn't make sense which is why I did a shar file.

I can re-roll my changes using a proper diff, if using the diff utility instead of subversions diff subcommand is acceptable. Or should I do this https://wiki.freebsd.org/PortsSubversionPrimer#Anonymous_Checkout to get a copy of the ports tree that is managed by subversion (this is not an issue for me if this is the best way)?

Thanks for the link to the "Using Python" chapter I overlooked that section, I will drop the pkg-plist file from my changes then since it's not needed in this case. As I said in my original email I'm still new to this process, and was bound to make mistakes.
Comment 4 Dan Langille freebsd_committer 2019-08-22 13:55:44 UTC
(In reply to Adam Jimerson from comment #3)

Assuming a clean ports tree, try this

cp -a py-tldextract py-tldextract.orig

Make your changes in py-tldextract

Produce your patch via:

diff -ruN py-tldextract.orig py-tldextract
Comment 5 Adam Jimerson 2019-08-22 16:52:46 UTC
Created attachment 206791 [details]
New py-tldextract patch

Attached is the a new patch to update this port as a proper diff
Comment 6 Dan Langille freebsd_committer 2019-08-22 16:57:52 UTC
(In reply to Adam Jimerson from comment #5)

It applies cleanly.  testport next.

[dan@r710-01:~/WorkingCopyPortsTree] $ patch < ~/py-tldextract.diff 
Hmm...  Looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- dns/py-tldextract/Makefile	(revision 509606)
|+++ dns/py-tldextract/Makefile	(working copy)
--------------------------
Patching file dns/py-tldextract/Makefile using Plan A...
Hunk #1 succeeded at 2 with fuzz 1.
Hmm...  The next patch looks like a unified diff to me...
The text leading up to this was:
--------------------------
|--- dns/py-tldextract/distinfo	(revision 509606)
|+++ dns/py-tldextract/distinfo	(working copy)
--------------------------
Patching file dns/py-tldextract/distinfo using Plan A...
Hunk #1 succeeded at 1.
done
Comment 7 Dan Langille freebsd_committer 2019-08-22 17:01:22 UTC
Test port passes.
Comment 8 commit-hook freebsd_committer 2019-08-22 17:03:50 UTC
A commit references this bug:

Author: dvl
Date: Thu Aug 22 17:02:56 UTC 2019
New revision: 509611
URL: https://svnweb.freebsd.org/changeset/ports/509611

Log:
  Update to 2.2.1

  PR:		240030
  Submitted by:	Adam Jimerson <vendion@gmail.com>

Changes:
  head/dns/py-tldextract/Makefile
  head/dns/py-tldextract/distinfo
Comment 9 Adam Jimerson 2019-08-22 17:04:56 UTC
(In reply to Dan Langille from comment #6)

Nice to know, I ended up just checking out a read-only copy of the ports repo and doing it there.

Thanks again for your help here I would have been lost otherwise ;), is there anything else that I need to do here or is it all on your end now?
Comment 10 Dan Langille freebsd_committer 2019-08-22 17:05:36 UTC
Congratulations.  Thank you.
Comment 11 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-23 13:19:02 UTC
(In reply to Adam Jimerson from comment #9)

If you'd like support in your porting, we have dedicated IRC channels: #freebsd-ports and #freebsd-python on freenode IRC, where we're happy to help ramp you up and answer questions
Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-23 13:26:51 UTC
@Dan Note also that this port is missing dependency declarations, from setup.py:

INSTALL_REQUIRES = ["setuptools", "idna", "requests>=2.1.0", "requests-file>=1.4"]

Full changelog (1.7.5 -> 2.2.1): https://github.com/john-kurkowski/tldextract/blob/master/CHANGELOG.md

Since the package also installs console_scripts [1], the port should (must) use USE_PYTHON=concurrent.

The missing dependencies change (commit) should also be merged to quarterly as that branch version will also be affected. 

It's a shame these issues weren't picked up prior to, or during the the 2.2.1 update as they could have been committed independently and merged to the quarterly branch without the 2.2.1 update.

Personally, I'd merge the fixes and the 2.2.1 update to quarterly, as long as the current port version of its only dependency (dns/py-dns-lexicon) supports 2.2.1 too.

[1] entry_points={ 'console_scripts': [ 'tldextract = tldextract.cli:main', ] },
Comment 13 Adam Jimerson 2019-08-25 11:42:43 UTC
(In reply to Kubilay Kocak from comment #12)

If I was to submit a new patch to correct the missing Deps should I create a new PR or would re-opening this one and submitting a new patch here be the correct flow?
Comment 14 Adam Jimerson 2019-08-25 11:44:16 UTC
(In reply to Kubilay Kocak from comment #11)

Thanks for that tip, I will look into them.
Comment 15 Dan Langille freebsd_committer 2019-08-25 13:45:37 UTC
(In reply to Adam Jimerson from comment #13)

If you create a new PR, I'd be happy to work on it with you.
Comment 16 Dan Langille freebsd_committer 2019-08-25 18:29:39 UTC
(In reply to Kubilay Kocak from comment #12)

The issue with a MFH of the current version is the introduction of new features.

With just 40 days or so until the new branch, this issue goes away, given people have been living / coping with it for at least more than the latest quarterly branch was created.

I think a MFH should not be done because of the above. Happy to be told I'm wrong though.