[#LU-278] Tag check is broken in configure script

[LU-278] Tag check is broken in configure script Created: 04/May/11 Updated: 28/Mar/12 Resolved: 12/Dec/11
Status:	Resolved
Project:	Lustre
Component/s:	None
Affects Version/s:	Lustre 2.1.0
Fix Version/s:	Lustre 2.2.0

Type:

Improvement

Priority:

Major

Reporter:

Christopher Morrone

Assignee:

Brian Murrell (Inactive)

Resolution:

Fixed

Votes:

Labels:

None

Environment:

RHEL6

Rank (Obsolete):

4815

Description

The configure check defined for LB_BUILDID more or less makes it impossible have any recent tag that is not of the form either 0.0.0.0 or v0_0_0_0. For instance, say I have checked out a commit that is tagged "2.0.59-1morrone" and attempt to configure lustre like so:

./configure --enable-dist --with-downstream-release=1morrone
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
[cut]
checking whether to configure just enough for make dist... yes
checking if this distro uses dpkg... yes
checking for buildid... configure: error: most recent tag found: 2.0.59-1morrone does not match current version 2.0.59.

The build really needs to allow any tag that it doesn't recognize. Heck, I might just have a local tag "very_fine_patch" or "10_things_I_hate_about_autoconf", and the build system needs to not have a problem with that.

Comments

Comment by Peter Jones [ 06/May/11 ]

Brian

Could you please comment?

Thanks

Peter

Comment by Christopher Morrone [ 06/May/11 ]

Here are three patches that I needed to make to get lustre building at LLNL:

http://review.whamcloud.com/509
http://review.whamcloud.com/510
http://review.whamcloud.com/511

The 510 deals with the configure problem. 509 deals with the build system not allowing most git tags. 511 is just lustre.spec improvements for RHEL6.

Comment by Brian Murrell (Inactive) [ 09/May/11 ]

The configure check defined for LB_BUILDID more or less makes it impossible have any recent tag that is not of the form either 0.0.0.0 or v0_0_0_0. For instance, say I have checked out a commit that is tagged "2.0.59-1morrone" and attempt to configure lustre like so:

./configure --enable-dist --with-downstream-release=1morrone
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
[cut]
checking whether to configure just enough for make dist... yes
checking if this distro uses dpkg... yes
checking for buildid... configure: error: most recent tag found: 2.0.59-1morrone does not match current version 2.0.59.

Probably the tag matching code needs to be more precise, if it can be.

The build really needs to allow any tag that it doesn't recognize. Heck, I might just have a local tag "very_fine_patch" or "10_things_I_hate_about_autoconf", and the build system needs to not have a problem with that.

Have you tried using a tag such as "very_fine_patch"? I suspect that would in fact work since the code that is trying to match tags actually does apply a match pattern to try to find version stamping tags.

FWIW, the whole point of this tag searching/matching is two-fold. First (but not primarily) it's to ensure that the version stamp that will go on the package, courtesy of lustre-version.ac, is correct and matches what the code has been tagged as.

Second, and primarily, we want to find the place in the history that matches what the package will be labeled as – 2.0.59, for example. When we find that, we can find all (if any) of the patches that have been applied since that tag. The reason for this is that I find that it's disingenuous to check out "HEAD" and package it up as "2.0.59" when it's in fact (say) 20 commits beyond 2.0.59 (presumably on it's way to being 2.0.60). Somebody opening up the resulting source package (the SRPM or looking in /debian) should be able to find the real 2.0.59 (which would be the tarball) and then see the 20 patches that are applied on top of it to produce the packages that resulted from that HEAD checkout.

The end result (i.e. Patch$n: lines in the RPM spec) is not quite there yet (it was for debian builds but had to be backed out due to a defect in the way the debian build was using the information and with debian builds being low priority it was quicker to back it out than to fix the defect), but this is all of the groundwork that is needed for it to get there.

Comment by Christopher Morrone [ 09/May/11 ]

I frankly disagree with this approach.

We've already got code to include in the lustre version the number of commits we are beyond a tag, and the first 7 characters of the commit hash. I think that is plenty. Going to a system that converts commits into patches is unnecessary complexity in my book.

The build system is already hugely complex and fragile. Case in point, the build system flat out would not work for us. I think we need to look into ways to reduce the complexity, not increase it.

Comment by Christopher Morrone [ 11/May/11 ]

By the way, I am relatively sure that gerrit is generating an incorrect diff on the web for the change in http://review.whamcloud.com/511.

At the end of the diff it shows:

80 100 %endif
81 101 %if ! %

{is_client}
102 +%if ! %{is_client}

But that makes no sense. git itself shows the end of the patch to be:

%endif
+%endif
%if ! %

{is_client}

So please look at the patch in git, and don't trust gerrit for that review.

Comment by Christopher Morrone [ 12/May/11 ]

Brian, I saw your patch here:

http://review.whamcloud.com/517

I don't have a problem with it, per say, but it doesn't really address my core complaint that the build system is too frickin' restrictive. Even with your patch here's what I get in my build farm:

checking for buildid... fatal: No names found, cannot describe anything.
configure: error: most recent tag found: does not match current version 2.0.59.

So let me refine my argument into this assertion:

The build system must not abort the configure stage or fail to build simply because the build is not taking place in a git directory, nor should it fail when the build takes place in a git directory simply because some history of the current commit is not available.

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,client,el5,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,client,el5,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,client,ubuntu1004,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,client,el6,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,server,el5,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,client,ubuntu1004,ofa #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,client,el5,ofa #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,server,el6,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,client,el5,ofa #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,client,el6,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,server,el5,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,server,el5,ofa #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » x86_64,server,el5,ofa #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Build Master (Inactive) [ 18/May/11 ]

Integrated in lustre-master » i686,server,el6,inkernel #124
~~LU-278~~ only use the v* tag and limit the match

Oleg Drokin : 3bf2ba3f3aa847e6d5ea9e0653e853aad0f4b95b
Files :

build/autoconf/lustre-build.m4

Comment by Brian Murrell (Inactive) [ 01/Jun/11 ]

Brian, I saw your patch here:

http://review.whamcloud.com/517

I don't have a problem with it, per say, but it doesn't really address my core complaint that the build system is too frickin' restrictive.

But does it address the specific concnern/use-case you described? As you described it, it should.

Even with your patch here's what I get in my build farm:

checking for buildid... fatal: No names found, cannot describe anything.
configure: error: most recent tag found: does not match current version 2.0.59.

Can you describe the process that results in this situation? Is this build that fails working in a git checkout? Or it is working in a tree that was unpacked from a tarball that was either downloaded from us or built with a "autogen.sh; configure ...; make dist"? Or is it something different? If so, can you please describe?

So let me refine my argument into this assertion:

The build system must not abort the configure stage or fail to build simply because the build is not taking place in a git directory,

Agreed. It was written to work equally successfully in a tree extracted from a tarball created with "autogen.sh; configure ...; make dist". If that's not working, that's a bug.

nor should it fail when the build takes place in a git directory simply because some history of the current commit is not available.

Can you provide an example of such a situation? I'm not sure I understand what you mean exactly.

Comment by Christopher Morrone [ 02/Jun/11 ]

Can you provide an example of such a situation? I'm not sure I understand what you mean exactly.

$ mkdir foo
$ cd foo
$ git init .
Initialized empty Git repository in /tmp/foo/.git/
$ git remote add wc git://git.whamcloud.com/fs/lustre-release.git
$ git fetch --depth 1 wc master
remote: Counting objects: 1812, done.
remote: Compressing objects: 100% (1639/1639), done.
remote: Total 1812 (delta 358), reused 907 (delta 116)
Receiving objects: 100% (1812/1812), 5.11 MiB | 1.95 MiB/s, done.
Resolving deltas: 100% (358/358), done.
From git://git.whamcloud.com/fs/lustre-release
 * branch            master     -> FETCH_HEAD
$ git branch master FETCH_HEAD
$ git checkout master
Already on 'master'
$ sh autogen.sh
[cut]
$ ./configure
[cut]
checking for buildid... fatal: No names found, cannot describe anything.
configure: error: most recent tag found:  does not match current version 2.0.61.

Comment by Andreas Dilger [ 14/Jul/11 ]

I've uploaded a new patch to http://review.whamcloud.com/#change,510 that changes this error to a warning.