Bug 225622

Summary: [patch] security/ngrok: Use versioned urls rather than generic to avoid checksum obsolescence
Product: Ports & Packages Reporter: Guangyuan Yang <ygy>
Component: Individual Port(s)Assignee: Guangyuan Yang <ygy>
Status: In Progress ---    
Severity: Affects Only Me CC: adamw, jhixson, woodsb02
Priority: --- Keywords: patch
Version: LatestFlags: jhixson: maintainer-feedback+
Hardware: Any   
OS: Any   
Attachments:
Description Flags
Proposed patch
none
Proposed patch updated ygy: maintainer-approval?

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).

https://dl.equinox.io/ngrok/ngrok/stable/archive
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
LGTM
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 :)