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

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 edited edge metadata.

Do not remove GITHUB_CLOUD

Mk/bsd.sites.mk
541

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.

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
1556

Perhaps this should also check that GH_TAGNAME is defined?

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

Mk/bsd.sites.mk
541

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)

Mk/bsd.sites.mk
545

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.

Mk/bsd.sites.mk
545

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

Mk/bsd.sites.mk
545

The result is not the same :-)

Mk/bsd.port.mk
1559

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.

Mk/bsd.port.mk
1559

Nevermind, I was misunderstanding something.

swills added a reviewer: AMDmi3.

Taking this over since it has sat for so long.

swills edited edge metadata.

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

Change to bitcoin no longer needed

mat added a reviewer: mat.
mat added a subscriber: mat.

Macro shipit:

bdrewery edited edge metadata.

I like it! Thanks for doing this.

bdrewery edited reviewers, added: swills; removed: bdrewery.

Takeover

antoine edited edge metadata.
This revision is now accepted and ready to land.Mar 19 2015, 4:55 PM

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.