git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC/PATCH 0/2] Test the Git version string
@ 2013-04-14 14:27 Philip Oakley
  2013-04-14 14:27 ` [RFC/PATCH 1/2] test git " Philip Oakley
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Philip Oakley @ 2013-04-14 14:27 UTC (permalink / raw)
  To: GitList

In $gmane/217004 I was noted that the git version string is used
in the wild for confirming which git version is in use.

This patch series seeks to add tests for the version string format
and document it.

The key questions to be answered are:
* should the test be inside t0000, or somewhere else?
* should the version string be limited to one line, <80 characters?
* how to format the asciidoc of the ERE.

Philip Oakley (2):
  test git version string
  Doc: State the exact git version string

 Documentation/git.txt | 4 ++++
 t/t0000-basic.sh      | 8 ++++++++
 2 files changed, 12 insertions(+)

-- 
1.8.1.msysgit.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [RFC/PATCH 1/2] test git version string
  2013-04-14 14:27 [RFC/PATCH 0/2] Test the Git version string Philip Oakley
@ 2013-04-14 14:27 ` Philip Oakley
  2013-04-14 14:27 ` [RFC/PATCH 2/2] Doc: State the exact " Philip Oakley
  2013-04-14 19:22 ` [RFC/PATCH 0/2] Test the Git " Junio C Hamano
  2 siblings, 0 replies; 11+ messages in thread
From: Philip Oakley @ 2013-04-14 14:27 UTC (permalink / raw)
  To: GitList

The git version string is used in the wild, and in git gui,
for capability checking purposes. Test the string format
and version X.Y.Z number.

The version string shall be:
* a single line
* less that 80 characters
* start with `git version `
* with a following numeric X.Y.Z version.

The remaining characters are undefined.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---

Should this be applied to t0000, or another test file?
Should the version string be always limited to one line?
Should the maximum string length be <80 characters?
Is egrep OK, or should I use grep with a longer BRE?

 t/t0000-basic.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index cefe33d..052afca 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -610,4 +610,12 @@ test_expect_success 'very long name in the index handled sanely' '
 	test $len = 4098
 '
 
+test_expect_success 'git version string X.Y.Z' '
+	git --version >verstring &&
+	test_line_count =  1 verstring &&
+	len=$(wc -c verstring | sed "s/[^0-9]//g") &&
+	test $len -lt  80 &&
+	egrep -q "^git version [0-9]+\.[0-9]+\.[0-9]+.*" verstring
+'
+
 test_done
-- 
1.8.1.msysgit.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [RFC/PATCH 2/2] Doc: State the exact git version string
  2013-04-14 14:27 [RFC/PATCH 0/2] Test the Git version string Philip Oakley
  2013-04-14 14:27 ` [RFC/PATCH 1/2] test git " Philip Oakley
@ 2013-04-14 14:27 ` Philip Oakley
  2013-04-14 19:22 ` [RFC/PATCH 0/2] Test the Git " Junio C Hamano
  2 siblings, 0 replies; 11+ messages in thread
From: Philip Oakley @ 2013-04-14 14:27 UTC (permalink / raw)
  To: GitList

Provide the regular expression that matches the
'git --version' strings.

Scripts in the wild, including 'git gui', check the git version.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---

Should the string be described by a regular expression?

How to format the regular expression for asciidocs - it doesn't
get made correctly - I'm not sure what special magic incantation
is needed here.

 Documentation/git.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 7efaa59..627e462 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -365,6 +365,10 @@ OPTIONS
 -------
 --version::
 	Prints the Git suite version that the 'git' program came from.
+	The version string is a single line that matches the extended
+	regular expression
+	'^git version [0-9]+\.[0-9]+\.[0-9]+.*'
+	and is less than 80 characters.
 
 --help::
 	Prints the synopsis and a list of the most commonly used
-- 
1.8.1.msysgit.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-14 14:27 [RFC/PATCH 0/2] Test the Git version string Philip Oakley
  2013-04-14 14:27 ` [RFC/PATCH 1/2] test git " Philip Oakley
  2013-04-14 14:27 ` [RFC/PATCH 2/2] Doc: State the exact " Philip Oakley
@ 2013-04-14 19:22 ` Junio C Hamano
  2013-04-14 21:26   ` Philip Oakley
  2 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2013-04-14 19:22 UTC (permalink / raw)
  To: Philip Oakley; +Cc: GitList

Philip Oakley <philipoakley@iee.org> writes:

> This patch series seeks to add tests for the version string format
> and document it.
>
> The key questions to be answered are:
> * should the test be inside t0000, or somewhere else?
> * should the version string be limited to one line, <80 characters?

One line, perhaps, but I do not think any byte-limit is reasonable.

It is unreasonable to limit the form to X.Y.Z; after we hit Git 2.0,
it is likely that we would go to two-decimals.

If the "parsing" is done for white/blacklist purposes, is there a
need to straight-jacket the verison string format like this series?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-14 19:22 ` [RFC/PATCH 0/2] Test the Git " Junio C Hamano
@ 2013-04-14 21:26   ` Philip Oakley
  2013-04-15  1:39     ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Philip Oakley @ 2013-04-14 21:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GitList

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Sunday, April 14, 2013 8:22 PM
> Philip Oakley <philipoakley@iee.org> writes:
>
>> This patch series seeks to add tests for the version string format
>> and document it.
>>
>> The key questions to be answered are:
>> * should the test be inside t0000, or somewhere else?
>> * should the version string be limited to one line, <80 characters?
>
> One line, perhaps, but I do not think any byte-limit is reasonable.

OK

>
> It is unreasonable to limit the form to X.Y.Z; after we hit Git 2.0,
> it is likely that we would go to two-decimals.

Ah. And then maintenance releases could be the .Z maybe. I suppose it 
will depend on circumstances.

>
> If the "parsing" is done for white/blacklist purposes, is there a
> need to straight-jacket the verison string format like this series?

The purpose is to document what we felt we could guarantee, and that 
which was open to variation, so that those, like the Git-Gui, can code 
in a sensible check for the version. Two digits (X.Y) should pass the 
existing Git Gui check.

I'll drop the length limit, and keep to an X.Y check

Is the end of t0000-basic.sh a sensible place for the check?

> --
Philip 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-14 21:26   ` Philip Oakley
@ 2013-04-15  1:39     ` Junio C Hamano
  2013-04-16  7:25       ` Philip Oakley
  0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2013-04-15  1:39 UTC (permalink / raw)
  To: Philip Oakley; +Cc: GitList

"Philip Oakley" <philipoakley@iee.org> writes:

>> If the "parsing" is done for white/blacklist purposes, is there a
>> need to straight-jacket the verison string format like this series?
>
> The purpose is to document what we felt we could guarantee, and that
> which was open to variation, so that those, like the Git-Gui, can code
> in a sensible check for the version. Two digits (X.Y) should pass the
> existing Git Gui check.
>
> I'll drop the length limit, and keep to an X.Y check
>
> Is the end of t0000-basic.sh a sensible place for the check?

Sorry, but I still do not understand what you are trying to achieve.

What kind of benefit are you envisioning out of this?  For a future
version, people would not know what incompatibilities it would
introduce, so

	case "$(git version)" in
	"git verison"[2-9].*)
        	echo unsupported version
                exit 1
                ;;
	esac

is a nonsense check.

For all released versions, people know how they looked like and we
do not have anything further to specify.  Git 1.5.0 will forever
identify itself as:

	$ git version
        git version 1.5.0

Worse yet, for an untagged version, you may get something like

	git version 1.8.2.1-515-g78d2372

and it may or may not behave the same way as 1.8.2.1 depending on
what trait you are interested in.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-15  1:39     ` Junio C Hamano
@ 2013-04-16  7:25       ` Philip Oakley
  2013-04-16 18:12         ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Philip Oakley @ 2013-04-16  7:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GitList

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Monday, April 15, 2013 2:39 AM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>>> If the "parsing" is done for white/blacklist purposes, is there a
>>> need to straight-jacket the verison string format like this series?
>>
>> The purpose is to document what we felt we could guarantee, and that
>> which was open to variation, so that those, like the Git-Gui, can
>> code
>> in a sensible check for the version. Two digits (X.Y) should pass the
>> existing Git Gui check.
>>
>> I'll drop the length limit, and keep to an X.Y check
>>
>> Is the end of t0000-basic.sh a sensible place for the check?
>
> Sorry, but I still do not understand what you are trying to achieve.
>
> What kind of benefit are you envisioning out of this?

The purpose of tests is to detect mistakes and spot regressions.

A change to the 'git version X.Y.z' string would be a regression, as I 
spotted earlier, as it conflicts with expectations of standard 
programmes such as git-gui.

>For a future
> version, people would not know what incompatibilities it would
> introduce, so
>
> case "$(git version)" in
> "git verison"[2-9].*)
>        echo unsupported version
>                exit 1
>                ;;
> esac
>
> is a nonsense check.
>
> For all released versions, people know how they looked like
> and we
> do not have anything further to specify.  Git 1.5.0 will forever
> identify itself as:
>
> $ git version
>        git version 1.5.0
>
> Worse yet, for an untagged version, you may get something like
>
> git version 1.8.2.1-515-g78d2372

However, if it passes the test [all the tests], one expects it will be 
reasonably (almost completely) compatibility with external expectations, 
such as those of git gui.

The questions I'm posing is from the other direction - use of tests for 
quality control.

>
> and it may or may not behave the same way as 1.8.2.1 depending on
> what trait you are interested in.

That will depend on the tests if [deliberately?] failed.

I'll tidy up the patches and commit meesage and see how it looks then.

Philip

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-16  7:25       ` Philip Oakley
@ 2013-04-16 18:12         ` Junio C Hamano
  2013-04-16 18:24           ` David Aguilar
  2013-04-16 21:22           ` Philip Oakley
  0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2013-04-16 18:12 UTC (permalink / raw)
  To: Philip Oakley; +Cc: GitList

"Philip Oakley" <philipoakley@iee.org> writes:

>> What kind of benefit are you envisioning out of this?
>
> The purpose of tests is to detect mistakes and spot regressions.
>
> A change to the 'git version X.Y.z' string would be a regression, as I
> spotted earlier, as it conflicts with expectations of standard
> programmes such as git-gui.

Sorry, but I do not follow.

A released version says "git version 1.8.2.1".  In a month or so,
I'll have another one that says "git version 1.8.3".  Or I may
decide to bump in preparation for 2.0 and it may identify itself as
"git version 1.9".

Neither of which no existing "program such as git-gui" has ever
seen.

In what way is that a regression?

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-16 18:12         ` Junio C Hamano
@ 2013-04-16 18:24           ` David Aguilar
  2013-04-16 19:02             ` Junio C Hamano
  2013-04-16 21:22           ` Philip Oakley
  1 sibling, 1 reply; 11+ messages in thread
From: David Aguilar @ 2013-04-16 18:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Philip Oakley, GitList

On Tue, Apr 16, 2013 at 11:12 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
> >> What kind of benefit are you envisioning out of this?
> >
> > The purpose of tests is to detect mistakes and spot regressions.
> >
> > A change to the 'git version X.Y.z' string would be a regression, as I
> > spotted earlier, as it conflicts with expectations of standard
> > programmes such as git-gui.
>
> Sorry, but I do not follow.
>
> A released version says "git version 1.8.2.1".  In a month or so,
> I'll have another one that says "git version 1.8.3".  Or I may
> decide to bump in preparation for 2.0 and it may identify itself as
> "git version 1.9".
>
> Neither of which no existing "program such as git-gui" has ever
> seen.
>
> In what way is that a regression?

Sorry.  I was the one that first suggested that this was an issue.

The "regression" is that there are scripts and tools in the wild that
need to know the git version when deciding whether or not to use some
new feature.

e.g. "git status --ignore-submodules=dirty" did not appear until git 1.7.2.
A script may want to use this flag, but it will only want to use it
when available.

If this string started saying "The Git Version Control System v2.0" then these
scripts would be "broken" since they would no longer recognize this as a
"post-1.7.2 Git".

The unstated suggestion here is that it may be helpful to others if we
were to declare that the "git version" output is plumbing.

Folks are already using it that way so making it official would provide
a guarantee that we won't break them in the future.
--
David

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-16 18:24           ` David Aguilar
@ 2013-04-16 19:02             ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2013-04-16 19:02 UTC (permalink / raw)
  To: David Aguilar; +Cc: Philip Oakley, GitList

David Aguilar <davvid@gmail.com> writes:

> The "regression" is that there are scripts and tools in the wild that
> need to know the git version when deciding whether or not to use some
> new feature.
>
> e.g. "git status --ignore-submodules=dirty" did not appear until git 1.7.2.
> A script may want to use this flag, but it will only want to use it
> when available.
>
> If this string started saying "The Git Version Control System v2.0" then these
> scripts would be "broken" since they would no longer recognize this as a
> "post-1.7.2 Git".

Blacklisting known-bad version and hoping all other versions
including the ones you have never seen to behave in the way you
expect usually works but there is a limit.

A change to say "The Git Version Control System %s" will not happen
willy-nilly, but when there is a good reason to do so, we would.

I do not think a test that hardcodes the output is a good way to
make sure a change is being done with a good reason.  After all, a
patch that updates the "git version %s" string can just update the
expected output in the same patch.  The only reason such a change
will be caught is because during the review, somebody notices that
the change touches the expected output of a test; for that to
reliably protect the output, the test *has* to be commented to say
that this expected output should be changed very carefully.

A much better solution would be to leave that "very carefully"
comment next to the in-code string to warn people about ramifiations
of changing it.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [RFC/PATCH 0/2] Test the Git version string
  2013-04-16 18:12         ` Junio C Hamano
  2013-04-16 18:24           ` David Aguilar
@ 2013-04-16 21:22           ` Philip Oakley
  1 sibling, 0 replies; 11+ messages in thread
From: Philip Oakley @ 2013-04-16 21:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: GitList

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Tuesday, April 16, 2013 7:12 PM
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>>> What kind of benefit are you envisioning out of this?
>>
>> The purpose of tests is to detect mistakes and spot regressions.
>>
>> A change to the 'git version X.Y.z' string would be a regression, as 
>> I
>> spotted earlier, as it conflicts with expectations of standard
>> programmes such as git-gui.
>
> Sorry, but I do not follow.
>
> A released version says "git version 1.8.2.1".  In a month or so,
> I'll have another one that says "git version 1.8.3".  Or I may
> decide to bump in preparation for 2.0 and it may identify itself as
> "git version 1.9".
>
> Neither of which no existing "program such as git-gui" has ever
> seen.
>
> In what way is that a regression?

But they both pass the test and test suite, yes? And even if you use 
git-gui with them, git-gui will not barf on start up, which it would if 
the version string fails my test.

Passing the test suite should be a reasonble guarantee that co-tools 
will work, including those that check for version. This is a check for 
that version string.

However if someone[1] creates "My Special Git Version 1-9-3 (Index 
V7b)", and here I'm suggesting they may have other special changes, then 
the version check will tell them that even when they have fixed their 
special changes to pass the other parts of the test suite, other 
co-tools could barf.

Its about pushing the piece of string from the users end ;-) It also 
stops others trying to change 'git' to 'Git' within this message, just 
as I did.

Philip
>
[1] my first draft had 'you', but that is not a reasonable starting 
position. It's more about *others* attempting to create release 
versions, which announce their name, that we expect to be compatible 
with say git-gui (via the rest of the test suite), and need to check 
that announcement. 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-04-16 21:22 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-14 14:27 [RFC/PATCH 0/2] Test the Git version string Philip Oakley
2013-04-14 14:27 ` [RFC/PATCH 1/2] test git " Philip Oakley
2013-04-14 14:27 ` [RFC/PATCH 2/2] Doc: State the exact " Philip Oakley
2013-04-14 19:22 ` [RFC/PATCH 0/2] Test the Git " Junio C Hamano
2013-04-14 21:26   ` Philip Oakley
2013-04-15  1:39     ` Junio C Hamano
2013-04-16  7:25       ` Philip Oakley
2013-04-16 18:12         ` Junio C Hamano
2013-04-16 18:24           ` David Aguilar
2013-04-16 19:02             ` Junio C Hamano
2013-04-16 21:22           ` Philip Oakley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).