Page MenuHomeFreeBSD

USE_GITHUB cleanup
ClosedPublic

Authored by bdrewery on Sep 9 2014, 7:45 PM.

Details

Reviewers
swills
AMDmi3
antoine
bapt
bdrewery
mat
Group Reviewers
portmgr
Summary
  • Cleanup GITHUB master sites.

While we currently have 3 github mirrors:

MASTER_SITE_GITHUB+= https://codeload.github.com/%SUBDIR% \

http://codeload.github.com/%SUBDIR%

MASTER_SITE_GITHUB_CLOUD+= http://cloud.github.com/downloads/%SUBDIR%

only first one works. Second one doesn't reply and clound reply with 403.

Leave only first one.

  • Use new GITHUB fetching scheme.

In addition to the scheme which we currently use:
https://codeload.github.com/Bertram25/ValyriaTear/legacy.tar.gz/1.0.0?dummy=/valyriatear-1.0.0.tar.gz

GitHub allows other one:
https://codeload.github.com/Bertram25/ValyriaTear/tar.gz/1.0.0?dummy=/valyriatear-1.0.0.tar.gz

The difference is that root directory of tarball is named
${GH_ACCOUNT}-${GH_PROJECT}-${GH_COMMIT} in the first case and
${GH_PROJECT}-${GH_TAGNAME} in the second one.

The second scheme is more convenient, as it doesn't require maintainer to retrieve and specify commit hash.

I propose to make use of the new scheme while leaving support for the old one. To select scheme, GH_COMMIT is checked. If it's defined, the old scheme is used. If not, the new one. Since GH_COMMIT is mandatory in the old scheme, this change is fully backwards compatible. Now, ports may just omit GH_COMMIT to use the new scheme.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

AMDmi3 updated this revision to Diff 1425.Sep 9 2014, 7:45 PM
AMDmi3 retitled this revision from to USE_GITHUB cleanup.
AMDmi3 updated this object.
AMDmi3 edited the test plan for this revision. (Show Details)
AMDmi3 added reviewers: bapt, bdrewery.
antoine added a subscriber: antoine.Sep 9 2014, 8:01 PM

mmmm GHC should still be kept in my opinion
See converters/lua-iconv for instance

antoine requested changes to this revision.Sep 9 2014, 8:01 PM
antoine added a reviewer: antoine.
This revision now requires changes to proceed.Sep 9 2014, 8:01 PM
bdrewery requested changes to this revision.Sep 9 2014, 8:04 PM
bdrewery edited edge metadata.

This is close to where I was going as well. Please give me some time to review this first.

In D748#4, @antoine wrote:

mmmm GHC should still be kept in my opinion
See converters/lua-iconv for instance

Agreed, GITHUB_CLOUD is different than USE_GITHUB.

AMDmi3 updated this revision to Diff 1427.Sep 9 2014, 8:27 PM
AMDmi3 edited edge metadata.

Do not remove GITHUB_CLOUD

antoine added inline comments.Sep 9 2014, 8:45 PM
Mk/bsd.sites.mk
542

With this, audio/lua51-mpd fails to fetch (it uses GHC but it's not explicit)

There's bunch of ports which do not set GH_COMMIT. I guess I need to process them first.

I've started testing this, should have more feedback soon.

bapt edited edge metadata.Oct 16 2014, 8:08 AM

@bdrewery any news on this?

swills added a subscriber: swills.Dec 19 2014, 3:30 PM

I have some other changes related to GITHUB stuff, see D1340. Perhaps I could combine them, but that might require additional testing.

Mk/bsd.port.mk
1557

Perhaps this should also check that GH_TAGNAME is defined?

Sorry this fell off my radar. Will test it tomorrow.

antoine added inline comments.Dec 19 2014, 11:41 PM
Mk/bsd.sites.mk
542

my comment was wrong, audia/lua51-mpd fails to fetch because it uses GH legacy but doesn't have a GH_COMMIT

I believe i fixed all the ports using legacy github that were not setting GH_COMMIT, so the change should be safe now (I need to test a bit)

antoine added inline comments.Dec 20 2014, 7:22 PM
Mk/bsd.sites.mk
546

I think I would change this condition to:
.if !defined(MASTER_SITES) || !${MASTER_SITES:MGH} && !${MASTER_SITES:MGHC} && !${MASTER_SITES:MGHR}
to not add a wrong master site to ports using GHC or GHR.

swills added inline comments.Dec 20 2014, 10:01 PM
Mk/bsd.sites.mk
546

Correct me if I'm wrong, but I think that's the same logic I added in rP375010 (D1340)

antoine added inline comments.Dec 20 2014, 10:03 PM
Mk/bsd.sites.mk
546

The result is not the same :-)

swills added inline comments.Dec 20 2014, 10:05 PM
Mk/bsd.port.mk
1560

GH_TAGNAME may not be defined if using a release, so a check that that's defined and using only ${GH_PROJECT}-${PORTVERSION} would be good.

swills added inline comments.Dec 20 2014, 10:38 PM
Mk/bsd.port.mk
1560

Nevermind, I was misunderstanding something.

swills commandeered this revision.Feb 21 2015, 12:05 AM
swills added a reviewer: AMDmi3.

Taking this over since it has sat for so long.

swills updated this revision to Diff 3882.Feb 21 2015, 12:33 AM
swills edited edge metadata.

Updating with Antoine's updated patch, plus fixing a few rejects that crept in

swills updated this revision to Diff 3995.Feb 26 2015, 4:26 PM

Change to bitcoin no longer needed

mat accepted this revision.Feb 26 2015, 4:35 PM
mat added a reviewer: mat.
mat added a subscriber: mat.

Macro shipit:

bdrewery accepted this revision.Mar 11 2015, 4:16 PM
bdrewery edited edge metadata.

I like it! Thanks for doing this.

bdrewery commandeered this revision.Mar 19 2015, 4:50 PM
bdrewery edited reviewers, added: swills; removed: bdrewery.

Takeover

bdrewery accepted this revision.Mar 19 2015, 4:51 PM
bdrewery added a reviewer: bdrewery.
antoine accepted this revision.Mar 19 2015, 4:55 PM
antoine edited edge metadata.
This revision is now accepted and ready to land.Mar 19 2015, 4:55 PM
bdrewery closed this revision.Mar 19 2015, 4:56 PM

Committed in rP381618

AMDmi3 edited edge metadata.EditedMar 24 2015, 1:30 AM

Just curious, why was internal github URL used (https://codeload.github.com/${ACCOUNT}/${PROJECT}/tar.gz/${TAG}) instead of public one (https://github.com/${ACCOUNT}/${PROJECT}/archive/${TAG}.tar.gz) ?

See https://developer.github.com/v3/repos/contents/#get-archive-link

It turns out that we now allow redirects just fine, so I see no point for not using official public URL. The change should be trivial and not affect any ports.

Also, do we really need ${GH_ACCOUNT} at the beginning of ${DISTNAME}? This makes port and distfile matching harder for user. I'd go with ${PORTNAME}-${PORTVERSION}-${GH_ACCOUNT}_${GH_TAGNAME} or similar.

In D748#51, @AMDmi3 wrote:

Just curious, why was internal github URL used (https://codeload.github.com/${ACCOUNT}/${PROJECT}/tar.gz/${TAG}) instead of public one (https://github.com/${ACCOUNT}/${PROJECT}/archive/${TAG}.tar.gz) ?
See https://developer.github.com/v3/repos/contents/#get-archive-link
It turns out that we now allow redirects just fine, so I see no point for not using official public URL. The change should be trivial and not affect any ports.
Also, do we really need ${GH_ACCOUNT} at the beginning of ${DISTNAME}? This makes port and distfile matching harder for user. I'd go with ${PORTNAME}-${PORTVERSION}-${GH_ACCOUNT}_${GH_TAGNAME} or similar.

Multiple reasons. 1. The API link redirects to the LEGACY url. Which invalidates your entire change pointless. 2. Using the main site URL just redirects to codeload anyhow but it ends up expanding whatever tag/hash you give until the full name. So we can no longer use a 4 char hash or 8 char hash, it forces the full hash length. It is an inconvenience for porters and would not allow a simple old->new USE_GITHUB transition as porters would have to go lookup the full hash, which Github tends to hide from you. The API URL issue is discussed in Bug 194898.