Bug 225622 - [patch] security/ngrok: Use versioned urls rather than generic to avoid checksum obsolescence
Summary: [patch] security/ngrok: Use versioned urls rather than generic to avoid check...
Status: In Progress
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Guangyuan Yang
Keywords: patch
Depends on:
Reported: 2018-02-02 05:42 UTC by Guangyuan Yang
Modified: 2018-06-02 03:16 UTC (History)
3 users (show)

See Also:
jhixson: maintainer-feedback+

Proposed patch (1.51 KB, patch)
2018-02-02 05:42 UTC, Guangyuan Yang
no flags Details | Diff
Proposed patch updated (1.80 KB, patch)
2018-03-02 05:38 UTC, Guangyuan Yang
ygy: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Guangyuan Yang freebsd_committer 2018-02-02 05:42:25 UTC
Created attachment 190264 [details]
Proposed patch

Instead of using "stable" and just update the checksum following the changes, pin the version of the tarball should be a better solution. Otherwise, the checksum will be obsolescence when a new stable release comes out (see the current broken port for example).

Comment 1 John Hixson freebsd_committer 2018-02-07 19:26:50 UTC
Looks good to me
Comment 2 Adam Weinberger freebsd_committer 2018-02-08 00:46:02 UTC
Looks fine here. Feel free to add in (or not) the stuff below at your own discretion.

Usually, when introducing things that will change on every release, it's customary to break it out into separate variables. Just makes it easier to maintain.

... ${ARCH:S/i386/${i386_HASH}}

i386_HASH=   [...]
amd64_HASH=  [...]

Also why is this putting files into ${PORTNAME}/${PORTVERSION}? That's not really how we do distfiles.
Comment 3 Adam Weinberger freebsd_committer 2018-02-09 02:42:37 UTC
Oh! Sorry, it just clicked. Because the distfile's name doesn't change. Right. My apologies.
Comment 4 Guangyuan Yang freebsd_committer 2018-03-02 05:38:08 UTC
Created attachment 191125 [details]
Proposed patch updated
Comment 5 Guangyuan Yang freebsd_committer 2018-03-02 05:39:08 UTC
Please re-approve the patch so I can commit. Updated to newest version, also switch to use .tar.gz archives.
Comment 6 Ben Woods freebsd_committer 2018-03-27 22:39:07 UTC
It looks pretty good, but I think I agree with adamw:

Now that the DISTFILE filename does include the version string, I think we should no longer need to set DIST_SUBDIR. Do you agree? Obviously, this change (if adopted) would also need the distinfo to be regenerated and port build to be re-tested.
Comment 7 Guangyuan Yang freebsd_committer 2018-03-28 02:56:25 UTC
(In reply to Ben Woods from comment #6)

Right on, I agree about setting DIST_SUBDIR back to default. I will do the testing and post up-to-date patches to Phabricator for review. Thanks!
Comment 8 John Hixson freebsd_committer 2018-05-24 19:33:49 UTC
Comment 9 Ben Woods freebsd_committer 2018-06-02 03:16:37 UTC
Ping. Have you had a chance to look at this one again Guangyuan? Should be a quick fix :)