[PATCH 5/5] Add a --version option to sparse

[PATCH 5/5] Add a --version option to sparse

[Ramsay Jones]

In addition to a minimal --version implementation, using
the output of 'git describe', we also output the version
string when -v(erbose) output has been requested.

Also, when cgcc internally sets the $gcc_base_dir, the
compiler invocation is changed to use an explicit gcc
command, rather than $cc. (when --version is part of
$cc, this breaks badly).

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Chris,

This patch actually adds a feature :-)

This is a minimal implementation and doesn't address the issue
of a sparse library version number; should the library define
some version macros like __SPARSE_MAJOR, __SPARSE_MINOR, etc.?
Dunno.

ATB,
Ramsay Jones

 Makefile |    9 +++++++++
 cgcc     |    5 ++++-
 lib.c    |   14 ++++++++++++++
 lib.h    |    1 +
 sparse.c |    8 +++++++-
 5 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 70ccbc9..2dcbc5a 100644
--- a/Makefile
+++ b/Makefile
@@ -19,6 +19,7 @@ HAVE_LIBXML=$(shell pkg-config --exists libxml-2.0 2>/dev/null && echo 'yes')
 HAVE_GCC_DEP=$(shell touch .gcc-test.c && 				\
 		$(CC) -c -Wp,-MD,.gcc-test.d .gcc-test.c 2>/dev/null && \
 		echo 'yes'; rm -f .gcc-test.d .gcc-test.o .gcc-test.c)
+IN_GIT_REPO=$(shell test -d .git && git describe >/dev/null 2>&1 && echo 'yes')
 
 CFLAGS += -DGCC_BASE=\"$(shell $(CC) --print-file-name=)\"
 
@@ -26,6 +27,14 @@ ifeq ($(HAVE_GCC_DEP),yes)
 CFLAGS += -Wp,-MD,$(@D)/.$(@F).d
 endif
 
+BUILD_VERSION=$(VERSION)
+
+ifeq ($(IN_GIT_REPO),yes)
+BUILD_VERSION=$(shell git describe)
+endif
+
+CFLAGS += -DVERSION='"$(BUILD_VERSION)"'
+
 DESTDIR=
 PREFIX=$(HOME)
 BINDIR=$(PREFIX)/bin
diff --git a/cgcc b/cgcc
index fdda6d1..a12d4b1 100755
--- a/cgcc
+++ b/cgcc
@@ -23,6 +23,9 @@ while (@ARGV) {
     # Ditto for stdin.
     $do_check = 1 if $_ eq '-';
 
+    # Check for --version
+    $do_check = 1 if $_ eq '--version';
+
     $m32 = 1 if /^-m32$/;
     $m64 = 1 if /^-m64$/;
     $gendeps = 1 if /^-M$/;
@@ -65,7 +68,7 @@ if ($do_check) {
 	$check .= &add_specs ('host_os_specs');
     }
 
-    $gcc_base_dir = qx($cc -print-file-name=) if !$gcc_base_dir;
+    $gcc_base_dir = qx(gcc -print-file-name=) if !$gcc_base_dir;
     $check .= " -gcc-base-dir " . $gcc_base_dir if $gcc_base_dir;
 
     print "$check\n" if $verbose;
diff --git a/lib.c b/lib.c
index 42affcd..9390c7f 100644
--- a/lib.c
+++ b/lib.c
@@ -43,6 +43,13 @@ int gcc_patchlevel = __GNUC_PATCHLEVEL__;
 
 static const char *gcc_base_dir = GCC_BASE;
 
+static const char *sparse_version_string = VERSION;
+
+const char *sparse_version(void)
+{
+	return sparse_version_string;
+}
+
 struct token *skip_to(struct token *token, int op)
 {
 	while (!match_op(token, op) && !eof_token(token))
@@ -619,6 +626,12 @@ static char **handle_base_dir(char *arg, char **next)
 	return next;
 }
 
+static char **handle_version(char *arg, char **next)
+{
+	printf("sparse version %s\n", sparse_version());
+	exit(0);
+}
+
 struct switches {
 	const char *name;
 	char **(*fn)(char *, char **);
@@ -629,6 +642,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;
diff --git a/lib.h b/lib.h
index 919b5b1..80f8ec8 100644
--- a/lib.h
+++ b/lib.h
@@ -80,6 +80,7 @@ struct token *expect(struct token *, int, const char *);
 #define NORETURN_ATTR
 #define SENTINEL_ATTR
 #endif
+extern const char *sparse_version(void);
 extern void die(const char *, ...) FORMAT_ATTR(1) NORETURN_ATTR;
 extern void info(struct position, const char *, ...) FORMAT_ATTR(2);
 extern void warning(struct position, const char *, ...) FORMAT_ATTR(2);
diff --git a/sparse.c b/sparse.c
index 4026ba7..2c43e0f 100644
--- a/sparse.c
+++ b/sparse.c
@@ -276,10 +276,16 @@ static void check_symbols(struct symbol_list *list)
 int main(int argc, char **argv)
 {
 	struct string_list *filelist = NULL;
+	struct symbol_list *list;
 	char *file;
 
+	list = sparse_initialize(argc, argv, &filelist);
+
+	if (verbose)
+		fprintf(stderr, "sparse version %s\n", sparse_version());
+
 	// Expand, linearize and show it.
-	check_symbols(sparse_initialize(argc, argv, &filelist));
+	check_symbols(list);
 	FOR_EACH_PTR_NOTAG(filelist, file) {
 		check_symbols(sparse(file));
 	} END_FOR_EACH_PTR_NOTAG(file);
-- 
1.6.3.1

Re: [PATCH 5/5] Add a --version option to sparse

[Christopher Li]

On Sat, Jul 18, 2009 at 2:06 PM, Ramsay Jones<ramsay@ramsay1.demon.co.uk> wrote:
>
> In addition to a minimal --version implementation, using
> the output of 'git describe', we also output the version
> string when -v(erbose) output has been requested.
>
> Also, when cgcc internally sets the $gcc_base_dir, the
> compiler invocation is changed to use an explicit gcc
> command, rather than $cc. (when --version is part of
> $cc, this breaks badly).
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>

Sorry I did not have a chance to reply it earlier. I was on a trip, very limited
Internet access.

> ---
>
> Hi Chris,
>
> This patch actually adds a feature :-)
>
> This is a minimal implementation and doesn't address the issue
> of a sparse library version number; should the library define
> some version macros like __SPARSE_MAJOR, __SPARSE_MINOR, etc.?
> Dunno.

I don't mind sparse has a version number. Some thing like 4.2.0 etc.
I would rather not mix that version number with the git commit hash.
The git commit hash has very limited usage. I think just print it as
part of the generic verbose printing is good enough. Add the printing in
handle_switch_v_finalize() so other executable can get it as well.

Thanks

Chris

Re: [PATCH 5/5] Add a --version option to sparse

[Ramsay Jones]

Christopher Li wrote:
> I don't mind sparse has a version number. Some thing like 4.2.0 etc.
> I would rather not mix that version number with the git commit hash.

The commit hash is only part of the version number string when you
build in a git repo and your HEAD does not reference a release
commit (as indicated by a tag object on this commit).

Note that the "dist" make target already uses 'git describe' to check
that the release tag and VERSION makefile variable agree.

$ git checkout master
$ git describe
0.4.1-99-g37f041a
$ git checkout 0.4.1
Note: moving to '0.4.1' which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b <new_branch_name>
HEAD is now at a02aeb3... Makefile: VERSION=0.4.1
$ git describe
0.4.1
$ 

[Note that I don't check for locally modified files and add "-dirty"
to the version string like git does; that is partly what I meant by
"minimal implementation"]

> The git commit hash has very limited usage. I think just print it as
> part of the generic verbose printing is good enough. Add the printing in
> handle_switch_v_finalize() so other executable can get it as well.
> 

Hmm, Oops! Now I remember... ahem *blush*, I had intended to modify
this patch before publishing it. (As I said before, this is quite an
old patch; I had a TODO to fix it up).

However, the fixup I have in mind involves moving most of the handling
of the --version option out of the library and into the sparse executable.
(i.e. more or less the opposite of what you suggest above for -v ;-) ).

My intent with this patch was to provide sparse (the program) with a
version number; *not* the sparse library, which is a separate issue.
(If a library version number is implemented, then the sparse program
version number *could* be the same number, of course). I did not
consider the other example programs built with sparse, but I suppose
they could all share the same version number if necessary.

However, you would not want sparse (the library) to force *all*
applications built with the library to handle the --version option
by printing the sparse (the program) version string and exiting!

So, the library should just set a version_option seen flag and let
the application process this flag itself.

Hmm, I don't have time tonight to re-write the patch, but I will
send a new version soon.

ATB,
Ramsay Jones

Re: [PATCH 5/5] Add a --version option to sparse

[Christopher Li]

On Fri, Jul 24, 2009 at 12:30 PM, Ramsay
Jones<ramsay@ramsay1.demon.co.uk> wrote:
> $ git checkout master
> $ git describe
> 0.4.1-99-g37f041a

If works well if you are following the official branch. If you are working
on your own, you might have some private tag. Then that will mess up
the "git describe". My tag might not even look like a version number.

So for version numbers, I still prefer the dot and numbers. We can
follow what Linux kernel do for version strings.

> My intent with this patch was to provide sparse (the program) with a
> version number; *not* the sparse library, which is a separate issue.
> (If a library version number is implemented, then the sparse program
> version number *could* be the same number, of course). I did not
> consider the other example programs built with sparse, but I suppose
> they could all share the same version number if necessary.

In my mind, they share the same version number. Individual version
the program inside sparse package is kind of messy. I don't see
a need for that in the near future yet.

> However, you would not want sparse (the library) to force *all*
> applications built with the library to handle the --version option
> by printing the sparse (the program) version string and exiting!

That is what YOUR current patch do, no?

>
> So, the library should just set a version_option seen flag and let
> the application process this flag itself.

That will force every program to process the version itself, doing more
or less the same thing.

Let's say I don't need separate version inside sparse package.
Does "-v" printing out sparse version string in side lib.c satisfy
your need already? In that case, I don't see the need for a separate
"--version".

Chris

Re: [PATCH 5/5] Add a --version option to sparse

[Ramsay Jones]

Christopher Li wrote:
> On Fri, Jul 24, 2009 at 12:30 PM, Ramsay
> Jones<ramsay@ramsay1.demon.co.uk> wrote:
>> $ git checkout master
>> $ git describe
>> 0.4.1-99-g37f041a
> 
> If works well if you are following the official branch. If you are working
> on your own, you might have some private tag. Then that will mess up
> the "git describe". My tag might not even look like a version number.
> 

True, but I don't see that as a problem. :)

>> However, you would not want sparse (the library) to force *all*
>> applications built with the library to handle the --version option
>> by printing the sparse (the program) version string and exiting!
> 
> That is what YOUR current patch do, no?
> 

Exactly. ;-)
I was describing what is wrong with the patch and how I intended to
fix it. Indeed, I have fixed it. However, since there is no interest
in it, I'll refrain from spaming the list. ;-)

ATB,
Ramsay Jones