[PATCH] Added sssd --version option

[Pavel Březina]

https://fedorahosted.org/sssd/ticket/953

[Stephen Gallagher]

On Wed, 2011-09-07 at 10:38 +0200, Pavel Březina wrote:
> https://fedorahosted.org/sssd/ticket/953

Nack.

The output of this patch isn't very clean. I think what we really want
is just to have a configure flag --with-distro-version that will take an
arbitrary string (provided by the packager) for what they'd like to see
as output for the version.

So without --with-distro-version:

$> sssd --version
1.7.0

With ./configure --with-distro-version="-3.fc15"
$> sssd --version
1.7.0-3.fc15

Or even
./configure --with-distro-version="-3.fc15\
Built: 2011-09-07"

Resulting in:
$> sssd --version
1.7.0-3.fc15
Built: 2011-09-07"

If prerelease_version is specified in version.m4, (i.e. as ".rhbz12345")
and with ./configure --with-distro-version="-3.fc15"

$> sssd --version
1.7.0-3.fc15.rhbz12345

(So prerelease version should always follow --with-distro-version)

Also, there is a line of whitespace in configure.ac. Please remove this.

[Jeff Schroeder]

On Thu, Sep 8, 2011 at 12:26 PM, Stephen Gallagher  wrote:
> On Wed, 2011-09-07 at 10:38 +0200, Pavel Březina wrote:
> > https://fedorahosted.org/sssd/ticket/953
>
> Nack.
>
> The output of this patch isn't very clean. I think what we really want
> is just to have a configure flag --with-distro-version that will take an
> arbitrary string (provided by the packager) for what they'd like to see
> as output for the version.
>
> So without --with-distro-version:
>
> $> sssd --version
> 1.7.0
>
> With ./configure --with-distro-version="-3.fc15"
> $> sssd --version
> 1.7.0-3.fc15

What is the point of this? Hi, my name is rpm -qi sssd

Not a *bad* feature per-se, but kind of strange for the default. I do
like the build in the --version output for when you are testing
scratch builds or cherrypicking fixes from git and testing things.

[Stephen Gallagher]

On Thu, 2011-09-08 at 12:45 -0700, Jeff Schroeder wrote:
> On Thu, Sep 8, 2011 at 12:26 PM, Stephen Gallagher  wrote:
> > On Wed, 2011-09-07 at 10:38 +0200, Pavel Březina wrote:
> > > https://fedorahosted.org/sssd/ticket/953
> >
> > Nack.
> >
> > The output of this patch isn't very clean. I think what we really want
> > is just to have a configure flag --with-distro-version that will take an
> > arbitrary string (provided by the packager) for what they'd like to see
> > as output for the version.
> >
> > So without --with-distro-version:
> >
> > $> sssd --version
> > 1.7.0
> >
> > With ./configure --with-distro-version="-3.fc15"
> > $> sssd --version
> > 1.7.0-3.fc15
>
> What is the point of this? Hi, my name is rpm -qi sssd
>
> Not a *bad* feature per-se, but kind of strange for the default. I do
> like the build in the --version output for when you are testing
> scratch builds or cherrypicking fixes from git and testing things.

Well, it's primarily targeted at non-RPM distributions that can't do a
simple 'rpm -q'. It also comes in handy for producing scratch builds (so
you can mark a build as unofficial, for example).

[Pavel Březina]

Dne 8.9.2011 21:26, Stephen Gallagher napsal(a):
> The output of this patch isn't very clean. I think what we really want
> is just to have a configure flag --with-distro-version that will take an
> arbitrary string (provided by the packager) for what they'd like to see
> as output for the version.

How about now?

[Stephen Gallagher]

On Wed, 2011-09-14 at 14:03 +0200, Pavel Březina wrote:
> Dne 8.9.2011 21:26, Stephen Gallagher napsal(a):
> > The output of this patch isn't very clean. I think what we really want
> > is just to have a configure flag --with-distro-version that will take an
> > arbitrary string (provided by the packager) for what they'd like to see
> > as output for the version.
>
> How about now?

This doesn't work with multi-line version strings.

I'm also re-thinking our approach here. I'd like to open this up for
discussion, but here are my new thoughts (sorry for going back and forth
on this so many times).

For this to be particularly useful, especially to developers, we should
probably make the DEFAULT version string be the base version plus a
timestamp. We could then override this for genuine releases with a
--with-release flag to configure. If no argument is provided, it would
be just the base version. If an argument is offered, it would be
concatenated to the base version.

So what we'd do instead is:

./configure (no args):
<build>
./sssd --version
sssd-1.7.0-pre201109201903Z

./configure --release
<build>
./sssd --version
sssd-1.7.0

./configure --release=-3.fc15
<build>
./sssd --version
sssd-1.7.0-3.fc15

I think this would be helpful for those of us doing development like
myself that regularly runs 'make install' from an in-progress tree.

I'm open to other suggestions, though.

[Jan Zelený]

> On Wed, 2011-09-14 at 14:03 +0200, Pavel Březina wrote:
> > Dne 8.9.2011 21:26, Stephen Gallagher napsal(a):
> > > The output of this patch isn't very clean. I think what we really want
> > > is just to have a configure flag --with-distro-version that will take
> > > an arbitrary string (provided by the packager) for what they'd like to
> > > see as output for the version.
> >
> > How about now?
>
> This doesn't work with multi-line version strings.
>
> I'm also re-thinking our approach here. I'd like to open this up for
> discussion, but here are my new thoughts (sorry for going back and forth
> on this so many times).
>
> For this to be particularly useful, especially to developers, we should
> probably make the DEFAULT version string be the base version plus a
> timestamp. We could then override this for genuine releases with a
> --with-release flag to configure. If no argument is provided, it would
> be just the base version. If an argument is offered, it would be
> concatenated to the base version.
>
> So what we'd do instead is:
>
> ./configure (no args):
>
> <build>
> ./sssd --version
> sssd-1.7.0-pre201109201903Z
>
> ./configure --release
>
> <build>
> ./sssd --version
> sssd-1.7.0
>
> ./configure --release=-3.fc15
>
> <build>
> ./sssd --version
> sssd-1.7.0-3.fc15
>
> I think this would be helpful for those of us doing development like
> myself that regularly runs 'make install' from an in-progress tree.
>
> I'm open to other suggestions, though.

I don't quite understand. Could you please elaborate what's wrong with the 
original approach you suggested? That means one configure argument allowing to 
specify the version with a default version string if no such argument is 
provided.

Thanks
Jan

[Stephen Gallagher]

On Wed, 2011-09-21 at 08:40 +0200, Jan Zelený wrote:
> I don't quite understand. Could you please elaborate what's wrong with the 
> original approach you suggested? That means one configure argument allowing to 
> specify the version with a default version string if no such argument is 
> provided.

Well, for one thing what I asked for in the first place was
impossible :-/

We can't have multi-line version strings in a configure option; autoconf
can't handle it. (All that happens is that the config.h is broken)

So I was trying to think of an alternative that would serve our needs
better. Though admittedly, at this point I'm starting to wonder at the
usefulness of this feature at all.

So my new thought was that we should default to displaying a version
that includes build-time information. Then if we wanted to display just
an official version (or a distro-specific build) we could do so by using
a --release argument.

This way at least, we would be able to identify work-in-progress builds
easily as compared to official release builds.

[Pavel Březina]

Dne 21.9.2011 13:32, Stephen Gallagher napsal(a):
> On Wed, 2011-09-21 at 08:40 +0200, Jan Zelený wrote:
> > I don't quite understand. Could you please elaborate what's wrong with the
> > original approach you suggested? That means one configure argument allowing to
> > specify the version with a default version string if no such argument is
> > provided.
>
> Well, for one thing what I asked for in the first place was
> impossible :-/
>
> We can't have multi-line version strings in a configure option; autoconf
> can't handle it. (All that happens is that the config.h is broken)

Actually, it can handle it. Although not directly (you have to put \n on 
the end of line):

$ ./configure --with-distro-version="-3.fc15\n\
> Built: 2011-09-07" --enable-silent-rules

$ ./sssd --version
1.7.0-3.fc15
Built: 2011-09-07
> So I was trying to think of an alternative that would serve our needs
> better. Though admittedly, at this point I'm starting to wonder at the
> usefulness of this feature at all.
>
> So my new thought was that we should default to displaying a version
> that includes build-time information. Then if we wanted to display just
> an official version (or a distro-specific build) we could do so by using
> a --release argument.
>
> This way at least, we would be able to identify work-in-progress builds
> easily as compared to official release builds.

[Jan Zelený]

> Dne 21.9.2011 13:32, Stephen Gallagher napsal(a):
> > On Wed, 2011-09-21 at 08:40 +0200, Jan Zelený wrote:
> > > I don't quite understand. Could you please elaborate what's wrong with
> > > the original approach you suggested? That means one configure argument
> > > allowing to specify the version with a default version string if no
> > > such argument is provided.
> >
> > Well, for one thing what I asked for in the first place was
> > impossible :-/
> >
> > We can't have multi-line version strings in a configure option; autoconf
> > can't handle it. (All that happens is that the config.h is broken)
>
> Actually, it can handle it. Although not directly (you have to put \n on
> the end of line):
>
> $ ./configure --with-distro-version="-3.fc15\n\
> > Built: 2011-09-07" --enable-silent-rules
>
> $ ./sssd --version
> 1.7.0-3.fc15
> Built: 2011-09-07

I can confirm this works as described by Pavel.
> > So I was trying to think of an alternative that would serve our needs
> > better. Though admittedly, at this point I'm starting to wonder at the
> > usefulness of this feature at all.
> >
> > So my new thought was that we should default to displaying a version
> > that includes build-time information. Then if we wanted to display just
> > an official version (or a distro-specific build) we could do so by using
> > a --release argument.
> >
> > This way at least, we would be able to identify work-in-progress builds
> > easily as compared to official release builds.

I don't know if that will do any good. The only thing it will do is make it 
possible to distinguish between work-in-progress build and official release. 
This can be achieved by current patch as well, you can add a value to 
PRERELEASE_VERSION when releasing new tarball (of course, the semantic of the 
PRERELEASE_VERSION would be a bit different at that point). Due to this, I'd 
like to consider this patch ACKed.

Any other opinions?

Thanks
Jan

[Jakub Hrozek]

On Tue, Nov 29, 2011 at 05:31:26PM +0100, Jan Zelený wrote:
> > Dne 21.9.2011 13:32, Stephen Gallagher napsal(a):
> > > On Wed, 2011-09-21 at 08:40 +0200, Jan Zelený wrote:
> > > > I don't quite understand. Could you please elaborate what's wrong with
> > > > the original approach you suggested? That means one configure argument
> > > > allowing to specify the version with a default version string if no
> > > > such argument is provided.
> > >
> > > Well, for one thing what I asked for in the first place was
> > > impossible :-/
> > >
> > > We can't have multi-line version strings in a configure option; autoconf
> > > can't handle it. (All that happens is that the config.h is broken)
> >
> > Actually, it can handle it. Although not directly (you have to put \n on
> > the end of line):
> >
> > $ ./configure --with-distro-version="-3.fc15\n\
> > > Built: 2011-09-07" --enable-silent-rules
> >
> > $ ./sssd --version
> > 1.7.0-3.fc15
> > Built: 2011-09-07
>
> I can confirm this works as described by Pavel.
> > > So I was trying to think of an alternative that would serve our needs
> > > better. Though admittedly, at this point I'm starting to wonder at the
> > > usefulness of this feature at all.
> > >
> > > So my new thought was that we should default to displaying a version
> > > that includes build-time information. Then if we wanted to display just
> > > an official version (or a distro-specific build) we could do so by using
> > > a --release argument.

I still prefer --version over --release. It's the standard switch
everyone uses.
> > > This way at least, we would be able to identify work-in-progress builds
> > > easily as compared to official release builds.
>
> I don't know if that will do any good. The only thing it will do is make it 
> possible to distinguish between work-in-progress build and official release. 
> This can be achieved by current patch as well, you can add a value to 
> PRERELEASE_VERSION when releasing new tarball (of course, the semantic of the 
> PRERELEASE_VERSION would be a bit different at that point). Due to this, I'd 
> like to consider this patch ACKed.
>
> Any other opinions?

I thought the real value was identifying distribution patchlevels
easily?

[Stephen Gallagher]

On Tue, 2011-11-29 at 17:31 +0100, Jan Zelený wrote:
> I can confirm this works as described by Pavel.

Pushed to master based on discussion at today's status meeting.