[RFC PATCH] sparse: Add cmd line --version option

[RFC PATCH] sparse: Add cmd line --version option

[Joe Perches]

There's no current way to know the version
of sparse.  Add --version to see it.

---

I'm not at all tied to this implementation
but it's always nice to be able to see what
version is being used.

Likely it needs something to always recompile
lib.o whenever appropriate.

 Makefile | 11 +++++++++--
 lib.c    |  7 +++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b195528..8d2ffea 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,12 @@
 VERSION=0.4.4
 
+HAVE_GIT:=$(shell git describe >/dev/null 2>&1 && echo 'yes')
+ifeq ($(HAVE_GIT),yes)
+SPARSE_VERSION=$(shell git describe)
+else
+SPARSE_VERSION=$(VERSION)
+endif
+
 OS = linux
 
 
@@ -27,7 +34,8 @@ HAVE_LLVM_VERSION:=$(shell llvm-config --version | grep "^[3-9].*" >/dev/null 2>
 LLVM_VERSION=$(shell llvm-config --version)
 
 GCC_BASE = $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\" \
+	       -DSPARSE_VERSION=\"$(SPARSE_VERSION)\"
 
 ifeq ($(HAVE_GCC_DEP),yes)
 BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
@@ -160,7 +168,6 @@ install: all-installable
 sparse.pc: sparse.pc.in
 	$(QUIET_GEN)sed $(SED_PC_CMD) sparse.pc.in > sparse.pc
 
-
 compile_EXTRA_DEPS = compile-i386.o
 
 $(foreach p,$(PROGRAMS),$(eval $(p): $($(p)_EXTRA_DEPS) $(LIBS)))
diff --git a/lib.c b/lib.c
index 4f69e11..ddc9a93 100644
--- a/lib.c
+++ b/lib.c
@@ -646,6 +646,12 @@ static char **handle_base_dir(char *arg, char **next)
 	return next;
 }
 
+static char **handle_version(char *arg, char **next)
+{
+	die("%s", SPARSE_VERSION);
+	return next;
+}
+
 struct switches {
 	const char *name;
 	char **(*fn)(char *, char **);
@@ -656,6 +662,7 @@ static char **handle_switch(char *arg, char **next)
 	static struct switches cmd[] = {
 		{ "nostdinc", handle_nostdinc },
 		{ "gcc-base-dir", handle_base_dir},
+		{ "-version", handle_version },
 		{ NULL, NULL }
 	};
 	struct switches *s;

Re: [RFC PATCH] sparse: Add cmd line --version option

[Christopher Li]

On Wed, Mar 6, 2013 at 9:22 AM, Joe Perches <joe@perches.com> wrote:
> There's no current way to know the version
> of sparse.  Add --version to see it.

Hi Joe,

Thanks for the patch.

I agree that this the "--version" switch has its value.
However, I don't like using "die" to print out the version string.
I see no reason why sparse should exit with error for printing
out version string. For example, gcc does not do that.

If you don't want to recompile lib.c every time, you can make
a version.c to store the version string.

Another thing is nice to have, but not required.
You consider to give "--" its own handing function. It is bit odd
seeing the "-version" mix with other switch command. The
patch is simpler. Either way is fine with me.

The exit code need to be fixed.

I will apply the patch once you fix the exit code.

Chris

Re: [RFC PATCH] sparse: Add cmd line --version option

[Joe Perches]

On Wed, 2013-03-06 at 13:19 -0800, Christopher Li wrote:
> On Wed, Mar 6, 2013 at 9:22 AM, Joe Perches <joe@perches.com> wrote:
> > There's no current way to know the version
> > of sparse.  Add --version to see it.
> 
> Hi Joe,

Hi Chris.
> I agree that this the "--version" switch has its value.
> However, I don't like using "die" to print out the version string.
> I see no reason why sparse should exit with error for printing
> out version string. For example, gcc does not do that.
> 
> If you don't want to recompile lib.c every time, you can make
> a version.c to store the version string.
> 
> Another thing is nice to have, but not required.
> You consider to give "--" its own handing function. It is bit odd
> seeing the "-version" mix with other switch command. The
> patch is simpler. Either way is fine with me.
> 
> The exit code need to be fixed.
> 
> I will apply the patch once you fix the exit code.

This still doesn't recompile lib.o every time.

Maybe:
---
 Makefile | 10 +++++++++-
 lib.c    | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b195528..e7db639 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,12 @@
 VERSION=0.4.4
 
+HAVE_GIT:=$(shell git describe >/dev/null 2>&1 && echo 'yes')
+ifeq ($(HAVE_GIT),yes)
+SPARSE_VERSION=$(shell git describe)
+else
+SPARSE_VERSION=$(VERSION)
+endif
+
 OS = linux
 
 
@@ -27,7 +34,8 @@ HAVE_LLVM_VERSION:=$(shell llvm-config --version | grep "^[3-9].*" >/dev/null 2>
 LLVM_VERSION=$(shell llvm-config --version)
 
 GCC_BASE = $(shell $(CC) --print-file-name=)
-BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\"
+BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\" \
+	       -DSPARSE_VERSION=\"$(SPARSE_VERSION)\"
 
 ifeq ($(HAVE_GCC_DEP),yes)
 BASIC_CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
diff --git a/lib.c b/lib.c
index 4f69e11..f1c284f 100644
--- a/lib.c
+++ b/lib.c
@@ -646,11 +646,39 @@ static char **handle_base_dir(char *arg, char **next)
 	return next;
 }
 
+static char **handle_version(char *arg, char **next)
+{
+	printf("%s\n", SPARSE_VERSION);
+	return next;
+}
+
 struct switches {
 	const char *name;
 	char **(*fn)(char *, char **);
 };
 
+static char **handle_options(char *arg, char **next)
+{
+	static struct switches cmd[] = {
+		{ "version", handle_version },
+		{ NULL, NULL }
+	};
+	struct switches *s;
+
+	s = cmd;
+	while (s->name) {
+		if (!strcmp(s->name, arg))
+			return s->fn(arg, next);
+		s++;
+	}
+
+	/*
+	 * Ignore unknown command line options:
+	 * they're probably gcc switches
+	 */
+	return next;
+}
+
 static char **handle_switch(char *arg, char **next)
 {
 	static struct switches cmd[] = {
@@ -952,6 +980,10 @@ struct symbol_list *sparse_initialize(int argc, char **argv, struct string_list
 		if (!arg)
 			break;
 
+		if (arg[0] == '-' && arg[1] == '-' && arg[2]) {
+			args = handle_options(arg+2, args);
+			continue;
+		}
 		if (arg[0] == '-' && arg[1]) {
 			args = handle_switch(arg+1, args);
 			continue;

Re: [RFC PATCH] sparse: Add cmd line --version option

[Josh Triplett]

On Wed, Mar 06, 2013 at 09:22:58AM -0800, Joe Perches wrote:
> --- a/Makefile
> +++ b/Makefile
> @@ -1,5 +1,12 @@
>  VERSION=0.4.4
>  
> +HAVE_GIT:=$(shell git describe >/dev/null 2>&1 && echo 'yes')
> +ifeq ($(HAVE_GIT),yes)
> +SPARSE_VERSION=$(shell git describe)
> +else
> +SPARSE_VERSION=$(VERSION)
> +endif
> +

The "dist" target already has a call to "git describe"; could you unify
the two?  (And, ideally, avoid calling git describe twice, once for
HAVE_GIT and once for SPARSE_VERSION?)

- Josh Triplett

Re: [RFC PATCH] sparse: Add cmd line --version option

[Joe Perches]

On Wed, 2013-03-06 at 13:45 -0800, Josh Triplett wrote:
> On Wed, Mar 06, 2013 at 09:22:58AM -0800, Joe Perches wrote:
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1,5 +1,12 @@
> >  VERSION=0.4.4
> >  
> > +HAVE_GIT:=$(shell git describe >/dev/null 2>&1 && echo 'yes')
> > +ifeq ($(HAVE_GIT),yes)
> > +SPARSE_VERSION=$(shell git describe)
> > +else
> > +SPARSE_VERSION=$(VERSION)
> > +endif
> > +
> 
> The "dist" target already has a call to "git describe"; could you unify
> the two?  (And, ideally, avoid calling git describe twice, once for
> HAVE_GIT and once for SPARSE_VERSION?)

I think the overhead is low and not worth the bother.

Go for it if it bothers you.

Re: [RFC PATCH] sparse: Add cmd line --version option

[Chris Li]

>
> This still doesn't recompile lib.o every time.
>
Ah, I finally get it what you mean the lib.o did not compile every
time. Here I add a new header file "version.h" which store the
SPARSE_VERSION. The makefile will regenerate the version.h
if it does not match the current 'git-describe'. Now lib.o will
recompile every time you change git branch.

Joe, I miss your sign off from the patch. Can you sign off on the patch?


> The "dist" target already has a call to "git describe"; could you unify
> the two?  (And, ideally, avoid calling git describe twice, once for
> HAVE_GIT and once for SPARSE_VERSION?)

That is a very good point. I make change the Makefile to only evaluate it once.

Chris

[PATCH] There's no current way to know the version of sparse.  Add
 --version to see it.

Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 Makefile | 10 +++++++++-
 lib.c    | 25 +++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index b195528..35e3801 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,13 @@
 VERSION=0.4.4

+# Generating file version.h if current version has changed
+SPARSE_VERSION:=$(shell git describe 2>/dev/null || echo '$(VERSION)')
+VERSION_H := $(shell cat version.h 2>/dev/null)
+ifneq ($(lastword $(VERSION_H)),"$(SPARSE_VERSION)")
+$(info $(shell echo '     GEN      'version.h))
+$(shell echo '#define SPARSE_VERSION "$(SPARSE_VERSION)"' > version.h)
+endif
+
 OS = linux


@@ -191,7 +199,7 @@ clean: clean-check
 	rm -f *.[oa] .*.d *.so $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc

 dist:
-	@if test "`git describe`" != "v$(VERSION)" ; then \
+	@if test "v$(SPARSE_VERSION)" != "v$(VERSION)" ; then \
 		echo 'Update VERSION in the Makefile before running "make dist".' ; \
 		exit 1 ; \
 	fi
diff --git a/lib.c b/lib.c
index 4f69e11..5e65a0c 100644
--- a/lib.c
+++ b/lib.c
@@ -27,6 +27,7 @@
 #include "scope.h"
 #include "linearize.h"
 #include "target.h"
+#include "version.h"

 int verbose, optimize, optimize_size, preprocessing;
 int die_if_error = 0;
@@ -646,11 +647,34 @@ static char **handle_base_dir(char *arg, char **next)
 	return next;
 }

+static char **handle_version(char *arg, char **next)
+{
+	printf("%s\n", SPARSE_VERSION);
+	exit(0);
+}
+
 struct switches {
 	const char *name;
 	char **(*fn)(char *, char **);
 };

+static char **handle_long_options(char *arg, char **next)
+{
+	static struct switches cmd[] = {
+		{ "version", handle_version },
+		{ NULL, NULL }
+	};
+	struct switches *s = cmd;
+
+	while (s->name) {
+		if (!strcmp(s->name, arg))
+			return s->fn(arg, next);
+		s++;
+	}
+	return next;
+
+}
+
 static char **handle_switch(char *arg, char **next)
 {
 	static struct switches cmd[] = {
@@ -676,6 +700,7 @@ static char **handle_switch(char *arg, char **next)
 	case 'G': return handle_switch_G(arg, next);
 	case 'a': return handle_switch_a(arg, next);
 	case 's': return handle_switch_s(arg, next);
+	case '-': return handle_long_options(arg + 1, next);
 	default:
 		break;
 	}
-- 
1.8.1.2

Re: [RFC PATCH] sparse: Add cmd line --version option

[Joe Perches]

On Wed, 2013-03-06 at 20:18 -0800, Chris Li wrote:
> >
> > This still doesn't recompile lib.o every time.
> >
> Ah, I finally get it what you mean the lib.o did not compile every
> time. Here I add a new header file "version.h" which store the
> SPARSE_VERSION. The makefile will regenerate the version.h
> if it does not match the current 'git-describe'. Now lib.o will
> recompile every time you change git branch.
> 
> Joe, I miss your sign off from the patch. Can you sign off on the patch?

It's your patch now, I was just maybe an instigator.
Looks good though.

If you want:

Reviewed-by: Joe Perches <joe@perches.com>

cheers,  Joe

> @@ -191,7 +199,7 @@ clean: clean-check
>  	rm -f *.[oa] .*.d *.so $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc
> 
>  dist:
> -	@if test "`git describe`" != "v$(VERSION)" ; then \
> +	@if test "v$(SPARSE_VERSION)" != "v$(VERSION)" ; then \
>  		echo 'Update VERSION in the Makefile before running "make dist".' ; \

You might make the makefile read the git version
so you don't need to edit Makefile at all.

Maybe something like:

git describe | cut -f1 -d"-"

Re: [RFC PATCH] sparse: Add cmd line --version option

[Christopher Li]

On Wed, Mar 6, 2013 at 8:33 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2013-03-06 at 20:18 -0800, Chris Li wrote:

> It's your patch now, I was just maybe an instigator.
> Looks good though.

Thanks. I start from your patch. I will just keep you
as the author for patch.

> Reviewed-by: Joe Perches <joe@perches.com>
>
> Maybe something like:
>
> git describe | cut -f1 -d"-"

You mean the first VERSION in the Makefile?
I want it to be there because sparse also release as
tar.gz package without the git tree. It is useful to keep a
version string outside of the git repository.

Chris

Re: [RFC PATCH] sparse: Add cmd line --version option

[Joe Perches]

On Wed, 2013-03-06 at 20:18 -0800, Chris Li wrote:
> >
> > This still doesn't recompile lib.o every time.
> >
> Ah, I finally get it what you mean the lib.o did not compile every
> time. Here I add a new header file "version.h" which store the
> SPARSE_VERSION. The makefile will regenerate the version.h
> if it does not match the current 'git-describe'. Now lib.o will
> recompile every time you change git branch.
> 
> Joe, I miss your sign off from the patch. Can you sign off on the patch?
> 
> 
> > The "dist" target already has a call to "git describe"; could you unify
> > the two?  (And, ideally, avoid calling git describe twice, once for
> > HAVE_GIT and once for SPARSE_VERSION?)
> 
> That is a very good point. I make change the Makefile to only evaluate it once.
> 
> Chris
> 
> [PATCH] There's no current way to know the version of sparse.  Add
>  --version to see it.
> 
> Signed-off-by: Christopher Li <sparse@chrisli.org>
> ---
>  Makefile | 10 +++++++++-
>  lib.c    | 25 +++++++++++++++++++++++++
>  2 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index b195528..35e3801 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,5 +1,13 @@
>  VERSION=0.4.4
> 
> +# Generating file version.h if current version has changed
> +SPARSE_VERSION:=$(shell git describe 2>/dev/null || echo '$(VERSION)')
> +VERSION_H := $(shell cat version.h 2>/dev/null)
> +ifneq ($(lastword $(VERSION_H)),"$(SPARSE_VERSION)")
> +$(info $(shell echo '     GEN      'version.h))
> +$(shell echo '#define SPARSE_VERSION "$(SPARSE_VERSION)"' > version.h)
> +endif
> +
>  OS = linux
> 
> 
> @@ -191,7 +199,7 @@ clean: clean-check
>  	rm -f *.[oa] .*.d *.so $(PROGRAMS) $(SLIB_FILE) pre-process.h sparse.pc
> 
>  dist:
> -	@if test "`git describe`" != "v$(VERSION)" ; then \
> +	@if test "v$(SPARSE_VERSION)" != "v$(VERSION)" ; then \
>  		echo 'Update VERSION in the Makefile before running "make dist".' ; \
>  		exit 1 ; \
>  	fi
> diff --git a/lib.c b/lib.c
> index 4f69e11..5e65a0c 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -27,6 +27,7 @@
>  #include "scope.h"
>  #include "linearize.h"
>  #include "target.h"
> +#include "version.h"
> 
>  int verbose, optimize, optimize_size, preprocessing;
>  int die_if_error = 0;
> @@ -646,11 +647,34 @@ static char **handle_base_dir(char *arg, char **next)
>  	return next;
>  }
> 
> +static char **handle_version(char *arg, char **next)
> +{
> +	printf("%s\n", SPARSE_VERSION);
> +	exit(0);
> +}
> +
>  struct switches {
>  	const char *name;
>  	char **(*fn)(char *, char **);
>  };
> 
> +static char **handle_long_options(char *arg, char **next)
> +{
> +	static struct switches cmd[] = {
> +		{ "version", handle_version },
> +		{ NULL, NULL }
> +	};
> +	struct switches *s = cmd;
> +
> +	while (s->name) {
> +		if (!strcmp(s->name, arg))
> +			return s->fn(arg, next);
> +		s++;
> +	}
> +	return next;
> +
> +}
> +
>  static char **handle_switch(char *arg, char **next)
>  {
>  	static struct switches cmd[] = {
> @@ -676,6 +700,7 @@ static char **handle_switch(char *arg, char **next)
>  	case 'G': return handle_switch_G(arg, next);
>  	case 'a': return handle_switch_a(arg, next);
>  	case 's': return handle_switch_s(arg, next);
> +	case '-': return handle_long_options(arg + 1, next);
>  	default:
>  		break;
>  	}