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

Support git describe in versionformat #60

Open
boombatower opened this issue Nov 12, 2014 · 15 comments
Open

Support git describe in versionformat #60

boombatower opened this issue Nov 12, 2014 · 15 comments

Comments

@boombatower
Copy link
Contributor

For projects using git it seems to make sense to just use git describe directly instead of including all the parts in the format.

Perhaps %gd (gd = git describe)?

Before I was pointed to this functionality I created https://github.com/boombatower/obs-git-update/blob/master/obs-git-update which uses the following.

$(git describe --always --tags --long | sed 's/-/./g')

Which results in something like: v2.2.507.g0524a6f. Can even add flags for always returning a result even if no tags. Nice since all in one does what you normally want and even includes the number of commits past the tag in addition to the commit hash. So in the example v2.2 is latest tag and 507 is the number of commit since that tag, then the abbreviated hash after that.

Thoughts?

@boombatower
Copy link
Contributor Author

After looking around this seems like what is being asked for in #55 as well?

@aspiers
Copy link
Member

aspiers commented Dec 16, 2014

Yes, in principal I'm in favour of something more flexible, as long as it doesn't introduce security holes.

@aspiers
Copy link
Member

aspiers commented Dec 18, 2014

One approach might be to support something like this:

<param name="versionformat">%gd</param>
<param name="git-describe">--match='v[0-9]*'</param>
<param name="git-describe-sed">s/^v//; s/-g[0-9a-f]\{7\}$//; s/-/+/</param>

So that if the closest tag was v1.2.3 and there have been 4 commits since that tag, then the version would be 1.2.3+4.

Although this has the limitation of only allowing a single string to be extracted from git describe.

@frispete
Copy link

frispete commented Nov 7, 2015

@aspiers There a rpm version issue with +4, as this leads to the situation, where v1.2.3.1 would be seen below v1.2.3+1. Better would be using '~' instead of '+', as this is special cased.

@aspiers
Copy link
Member

aspiers commented Nov 10, 2015

@frispete Right - thanks for the heads up on that :)

@aspiers
Copy link
Member

aspiers commented Jun 21, 2016

Actually, I think a better way of solving this would be to allow execution of an arbitrary command (which could be a script within the package) which would be responsible for determining the version, e.g.

<param name="version-command">extract-git-version.sh</param>

or

<param name="version-command">git describe --match='v[0-9]*' | sed 's/^v//; s/-g[0-9a-f]\{7\}$//; s/-/+/'</param>

However this would allow arbitrary command execution within source service runs, which could be a gaping security hole. @adrianschroeter Are server-side source service runs done in a security-isolated sandbox? i.e. is this a problem we already have to deal with in other source services, or would it be introducing a big new security issue?

@adrianschroeter
Copy link
Member

On Dienstag, 21. Juni 2016, 04:52:30 CEST wrote Adam Spiers:

Actually, I think a better way of solving this would be to allow execution of an arbitrary command (which could be a script within the package) which would be responsible for determining the version, e.g.

this is IMHO not a good idea, it would make it quite dangerous to use the
service then.

It should be always safe to checkout and build a package source
without the need of checking the sources. Otherwise it is a security
issue.

<param name="version-command">extract-git-version.sh</param>

or

<param name="version-command">git describe --match='v[0-9]*' | sed 's/^v//; s/-g[0-9a-f]\{7\}$//; s/-/+/'</param>

However this would allow arbitrary command execution within source service runs, which could be a gaping security hole. @adrianschroeter Are server-side source service runs done in a security-isolated sandbox? i.e. is this a problem we already have to deal with in other source services, or would it be introducing a big new security issue?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#60 (comment)

Adrian Schroeter
email: adrian@suse.de

SUSE Linux GmbH, GF: Felix Imend�rffer, Jane Smithard, Graham Norton, HRB 21284 (AG N�rnberg)

Maxfeldstra�e 5
90409 N�rnberg
Germany

@aspiers
Copy link
Member

aspiers commented Jun 21, 2016

OK, forget that idea then ;-)

@aspiers aspiers removed the need info label Jun 21, 2016
@aspiers
Copy link
Member

aspiers commented Jun 21, 2016

I wrote:

Although this has the limitation of only allowing a single string to be extracted from git describe.

Right now I can't think of a strong reason why that would be a problem. However even sed scripts let you execute arbitrary commands so we would somehow need to sanitise them, which sounds really hard. Bah :-( Any other good suggestions?

@adrianschroeter
Copy link
Member

On Dienstag, 21. Juni 2016, 05:31:21 CEST wrote Adam Spiers:

I wrote:

Although this has the limitation of only allowing a single string to be extracted from git describe.

Right now I can't think of a strong reason why that would be a problem. However even sed scripts let you execute arbitrary commands so we would somehow need to sanitise them, which sounds really hard. Bah :-( Any other good suggestions?

Do you allow to inject sed regexp in one of our maintained services?

It is fine to have a personal service running at build time run any scripts though.

Adrian Schroeter
email: adrian@suse.de

SUSE Linux GmbH, GF: Felix Imend�rffer, Jane Smithard, Graham Norton, HRB 21284 (AG N�rnberg)

Maxfeldstra�e 5
90409 N�rnberg
Germany

@aspiers
Copy link
Member

aspiers commented Jun 21, 2016

Do you allow to inject sed regexp in one of our maintained services?

Not currently, but that was my previous suggestion.

It is fine to have a personal service running at build time run any scripts though.

Yeah, but at build-time is too late for determining the version, and that's the problem we're trying to solve here in a flexible manner. We could support the

<param name="version-command">extract-git-version.sh</param>

feature only for local source service runs, and disable on server-side runs, or do you even see that as a risk?

@aspiers
Copy link
Member

aspiers commented Apr 13, 2017

@boombatower Now that we merged your commit 960d0ce in #132, is that sufficient, or do we still need something more?

@boombatower
Copy link
Contributor Author

This is a different issue entirely, but up to you what you want to do with this. Such problems are fun as that commit took two years, hehe. :)

@aspiers
Copy link
Member

aspiers commented May 17, 2017

I'm probably being stupid, but I don't see how it's an entirely different issue - isn't it now possible to accomplish what you originally requested here, using @PARENT_TAG@, @TAG_OFFSET@, and versionrewrite-{pattern,replacement}? If not, please could you explain what's still missing? Thanks a lot!

@boombatower
Copy link
Contributor Author

I think the original post sums up the idea.

For projects using git it seems to make sense to just use git describe directly instead of including all the parts in the format.

Perhaps %gd (gd = git describe)?

Sure one could use the versionrewrite to drop the v afterwards, but the idea was to use git describe directly. The idea being that the common patterns are either release tags or rolling commit based packages. Rather than have to copy/paste (after finding) or piece together all the parts for such a version why not just provide one built in? Currently, there are a variety of needless subtle differences between such packages including different separator characters like ~, +, etc and different date/commit styles.

If anything this could be re-purposed as simply @ROLLING_VERSION@ which is substitured for some combination of the existing variables like you mention.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants