Jan Engelhardt (jengelh@inai.de) wrote:
On Saturday 2014-01-11 14:53, Adam Spiers wrote:
Furthermore, this would mean that the tar_scm source service would require an extra parameter (which only applies in the git case). It also would require a patch to tar_scm to add support for your tag offset mechanism. I believe in freedom of choice, so I would gladly accept a pull request from you which supports that scheme
""" The following changes since commit e55833cfb2164776db0c27c2b2acab97264279d5:
Merge pull request #25 from vuntz/add-back-tac (2013-12-12 07:24:53 -0800)
are available in the git repository at:
git://git.inai.de/obs-service-tar_scm master
for you to fetch changes up to 3c38cd32941dfc51ac1245c9de91fd63e29969fe:
tar_scm: support retrieving the git tag offset (2014-01-11 16:26:46 +0100)
---------------------------------------------------------------- Jan Engelhardt (4): tar_scm: check $? when it is fresh tar_scm: shorten commit IDs in a safer way tar_scm: more quoting in shell scripts is good tar_scm: support retrieving the git tag offset
tar_scm | 49 ++++++++++++++++++++++++++++++++----------------- tar_scm.service | 10 ++++++++++ 2 files changed, 42 insertions(+), 17 deletions(-) """
Excellent, thanks! However, please can you submit this to a fork on github, so that Travis can run the test suite, and we can go through the normal code review procedure. At first glance it looks mostly good, but I did spot a few issues, e.g. - It's missing a corresponding addition to the test suite. That's a show-stopper ;-) - I don't think "@REVISION@" is the best choice of placeholder name; it's too misleading. Something like @TAG_OFFSET@ would be better. - In general I agree that shell variables could be quoted during expansion. But quoting variables like $? and $# is overkill, since they will never be the empty string. Furthermore, in bash the quotes are redundant when you are using the double [[ ]] parentheses: bash$ a= bash$ if [[ $a == b ]]; then echo b; else echo empty; fi empty bash$
To use it in a bspkg (like openSUSE:Tools/obs-service-tar_scm itself), the following changes were made:
--- _service 2014-01-11 16:28:54.778664717 +0100 +++ _service.new 2014-01-11 16:22:06.624469218 +0100 @@ -4,7 +4,8 @@ <param name="scm">git</param> <param name="exclude">.git</param> <param name="version">git-master</param> - <param name="versionformat">0.3.2.%ct.%h</param> + <param name="versionformat">0.3.2+git@REVISION@</param> + <param name="basetag">v0.3.2</param> <param name="revision">master</param> <param name="changesgenerate">enable</param> </service>
Specifying the basetag also has the advantage that @REVISION@ does not go back to 0 without the bspkg maintainer noticing, which is also a property that %ct seemed to hold in case a new git tag (e.g. v0.3.3) was created while _service was left at versionformat=0.3.2.something.
Ah OK, you've changed your design! :-) Yes, this is *much* better than some fluffy git describe --match=GLOB logic; finally it addresses the main flaw in your scheme. There's still a small risk that upstream repositions the tag in question, but that's about on a par with the timestamps being rewritten, so I think we just have to accept that. The only other thing that could go wrong is if the upstream git repository removed the tag for some bizarre reason, and if that happened, it's a *good* thing that this would draw our attention to it, because it really shouldn't happen. Thanks for reacting to my feedback. We got there in the end ;-) -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org