Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation for MXE_GET_GITHUB_TAG_SHA #649

Closed
wants to merge 1 commit into from
Closed

Conversation

0xACE
Copy link
Contributor

@0xACE 0xACE commented Mar 10, 2015

This is identical to MXE_GET_GITHUB_SHA. This will grab the SHA based on the latest tag.

Example of usage:

$(PKG)_UPDATE = $(call MXE_GET_GITHUB_TAG_SHA, owner/repository)

This is identical to MXE_GET_GITHUB_SHA. This will grab the SHA based on the latest tag.

Example of usage:

    $(PKG)_UPDATE = $(call MXE_GET_GITHUB_TAG_SHA, owner/repository)
0xACE added a commit to 0xACE/mxe that referenced this pull request Mar 10, 2015
I noticed how the author for jansson.mk in mxe#646 mentioned the lack for resolving the SHA for the PKG_UPDATE. This patch along with mxe#649 fixes this problem.

A side effect from this is that PKG_VERSION will have to be replaced by a checksum so that it resolves the correct github url. I'll leave it up to you how to deal with this.
@0xACE 0xACE mentioned this pull request Mar 10, 2015
@TimothyGu
Copy link
Member

Instead of printing the SHA-1, it is probably better to print the tag name instead, and download the tag tarball.

@TimothyGu
Copy link
Member

Can you test if the following works? If it does I'll commit my version, as it allows further post-processing of the version name.

Also, it is not always the best idea to choose the last tag, as it might not be the latest (e.g. 2.6-rc is sorted after 2.6).

define MXE_GET_GITHUB_TAGS
    $(WGET) -q -O- 'https://api.github.com/repos/$(strip $(1))/git/refs/tags/' | \
    $(SED) -n 's#.*"ref": "refs/tags/\([^"]*\).*#\1#p' | \
    $(SORT) -V
endef
define $(PKG)_UPDATE
    $(call MXE_GET_GITHUB_TAGS, akheron/jansson) | \
    $(SED) 's,^v,,g'
endef

(This won't work if the tag name contains ", but meh who cares at this point.)

@0xACE
Copy link
Contributor Author

0xACE commented Mar 10, 2015

Also, it is not always the best idea to choose the last tag

My bad, you're right. Nice catch.

Can you test if the following works? If it does I'll commit my version, as it allows further post-processing of the version name.

Yes, I tried it and it works.

The reason i chose the hash rather than the tagname was because i was worried someone would have weird tag names, not sure how this project handles spaces in filenames. As you pointed out tags containing " will cause issues. I thought about removing all but [0-9.\-_] but that also seemed like a poor hack as I imagined someone would use letters in the version number.

We could leave it to the person implementing PKG_UPDATE to create a sed pattern to deal with anamolies in tag names.

I'll leave the final decision to you.

@starius
Copy link
Member

starius commented Sep 25, 2015

MXE_GET_GITHUB_TAGS from #649 (comment) doesn't work for pire.
It joins multiple versions into one string.

Makefile:

define MXE_GET_GITHUB_TAGS
    $(WGET) -q -O- 'https://api.github.com/repos/$(strip $(1))/git/refs/tags/' | \
    $(SED) -n 's#.*"ref": "refs/tags/\([^"]*\).*#\1#p' | \
    $(SORT) -V
endef

src/pire.mk:

define $(PKG)_UPDATE
    $(call MXE_GET_GITHUB_TAGS, yandex/pire) | \
    $(SED) 's,^release-,,g'
endef

Change current version to some old version (e.g., 0.0.4) and try to update:

$ make update-package-pire
[using autodetected 4 job(s)]
OLD      pire  0.04 --> 0.0.3 0.0.4 0.0.5 ignoring
make: `update-package-pire' is up to date

@TimothyGu
Copy link
Member

Oh yeah I forgot the final tail -1

This should work:

define MXE_GET_GITHUB_TAGS
    $(WGET) -q -O- 'https://api.github.com/repos/$(strip $(1))/git/refs/tags/' | \
    $(SED) -n 's#.*"ref": "refs/tags/\([^"]*\).*#\1#p' | \
    $(SORT) -V |
    tail -1
endef

starius pushed a commit to LuaAndC/mxe that referenced this pull request Sep 26, 2015
@starius
Copy link
Member

starius commented Sep 26, 2015

I have created a pull request for this: #888

@starius
Copy link
Member

starius commented Sep 26, 2015

I think, this pull-request can be closed now, as #888 was merged.

@TimothyGu TimothyGu closed this Sep 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants