[PATCH] add test for 'git rebase --keep-empty'

[PATCH] add test for 'git rebase --keep-empty'

[Martin von Zweigbergk]

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---

While trying to use patch-id instead of
--ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that
patch-id ignores empty patches and I was surprised that tests still
pass. This test case would be useful to protect --keep-empty.

 t/t3401-rebase-partial.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 7f8693b..b89b512 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' '
 	git commit --allow-empty -m empty &&
 	test_commit D &&
 	git rebase C &&
-	test $(git log --format=%s C..) = "D"
+	test "$(git log --format=%s C..)" = "D"
+'
+
+test_expect_success 'rebase --keep-empty' '
+	git reset --hard D &&
+	git rebase --keep-empty C &&
+	test "$(git log --format=%s C..)" = "D
+empty"
 '
 
 test_done
-- 
1.7.11.1.104.ge7b44f1

Re: [PATCH] add test for 'git rebase --keep-empty'

[Neil Horman]

On Wed, Aug 08, 2012 at 09:48:18AM -0700, Martin von Zweigbergk wrote:
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> ---
> 
> While trying to use patch-id instead of
> --ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that
> patch-id ignores empty patches and I was surprised that tests still
> pass. This test case would be useful to protect --keep-empty.
> 
>  t/t3401-rebase-partial.sh | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
> index 7f8693b..b89b512 100755
> --- a/t/t3401-rebase-partial.sh
> +++ b/t/t3401-rebase-partial.sh
> @@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' '
>  	git commit --allow-empty -m empty &&
>  	test_commit D &&
>  	git rebase C &&
> -	test $(git log --format=%s C..) = "D"
> +	test "$(git log --format=%s C..)" = "D"
> +'
> +
> +test_expect_success 'rebase --keep-empty' '
> +	git reset --hard D &&
> +	git rebase --keep-empty C &&
> +	test "$(git log --format=%s C..)" = "D
> +empty"
>  '
>  
>  test_done
> -- 
> 1.7.11.1.104.ge7b44f1
> 
> 

Acked-by: Neil Horman <nhorman@tuxdriver.com>

[PATCH v2] add tests for 'git rebase --keep-empty'

[Martin von Zweigbergk]

Add test cases for 'git rebase --keep-empty' with and without an
"empty" commit already in upstream. The empty commit that is about to
be rebased should be kept in both cases.

Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
---

Added another test for when the upstream already has an empty
commit. The test case protects the current behavior; I just assume the
current behavior is what we want.

While writing the test case, I also noticed that an interrupted 'git
rebase --keep-empty' can not be continued 'git rebase --continue', but
instead needs 'git cherry-pick --continue'. I guess this shouldn't
really be surprising given that it's implemented in terms of
cherry-pick. This should be fixed once all the different kinds of
rebase use the same way of finding the commits to rebase, so I
wouldn't worry about fixing this specific problem right now.

 t/t3401-rebase-partial.sh | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
index 7f8693b..58f4823 100755
--- a/t/t3401-rebase-partial.sh
+++ b/t/t3401-rebase-partial.sh
@@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' '
 	git commit --allow-empty -m empty &&
 	test_commit D &&
 	git rebase C &&
-	test $(git log --format=%s C..) = "D"
+	test "$(git log --format=%s C..)" = "D"
+'
+
+test_expect_success 'rebase --keep-empty' '
+	git reset --hard D &&
+	git rebase --keep-empty C &&
+	test "$(git log --format=%s C..)" = "D
+empty"
+'
+
+test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
+	git reset --hard A &&
+	git commit --allow-empty -m also-empty &&
+	git rebase --keep-empty D &&
+	test "$(git log --format=%s A..)" = "also-empty
+D
+empty"
 '
 
 test_done
-- 
1.7.11.1.104.ge7b44f1

Re: [PATCH v2] add tests for 'git rebase --keep-empty'

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> Add test cases for 'git rebase --keep-empty' with and without an
> "empty" commit already in upstream. The empty commit that is about to
> be rebased should be kept in both cases.
>
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
> ---
>
> Added another test for when the upstream already has an empty
> commit. The test case protects the current behavior; I just assume the
> current behavior is what we want.

Thanks.  I think it makes sense, as "upstream already has an empty
commit" together with "want to keep empty while rebasing" is a
strong sign that the user wants to have a history littered with many
empty commits.  Unlike a normal commit whose "patch-id" identity may
have meaningful significance (i.e. "the change to do X is already
in, or not yet in, this branch"), all the empty commits share the
same emptiness, so having one empty somewhere long time ago in the
history of where we are transplanting the commits shouldn't be a
reason to countermand the "want to keep empty" wish by the user.

And I do not think the conclusion would change even if we changed
the definition of "identity" for empty commits so that two empty
commits with the same message are detected as equal.  The only semi
sensible justification I heard from people who want to have empty
commits in their history is to keep in-history "notes" (e.g. "at
this point in the series, the code stops to compile, but the next
one fixes it", "it is possible that we may want to redo the previous
patch but I dunno"), and it may not make sense to drop such an empty
commit under "--keep-empty" mode if there are similar or identical
looking "notes" in the "upstream" part of the history.

Re: [PATCH v2] add tests for 'git rebase --keep-empty'

[Neil Horman]

On Thu, Aug 09, 2012 at 08:39:51AM -0700, Martin von Zweigbergk wrote:
> Add test cases for 'git rebase --keep-empty' with and without an
> "empty" commit already in upstream. The empty commit that is about to
> be rebased should be kept in both cases.
> 
> Signed-off-by: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

> ---
> 
> Added another test for when the upstream already has an empty
> commit. The test case protects the current behavior; I just assume the
> current behavior is what we want.
> 
> While writing the test case, I also noticed that an interrupted 'git
> rebase --keep-empty' can not be continued 'git rebase --continue', but
> instead needs 'git cherry-pick --continue'. I guess this shouldn't
> really be surprising given that it's implemented in terms of
> cherry-pick. This should be fixed once all the different kinds of
> rebase use the same way of finding the commits to rebase, so I
> wouldn't worry about fixing this specific problem right now.
> 
>  t/t3401-rebase-partial.sh | 18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh
> index 7f8693b..58f4823 100755
> --- a/t/t3401-rebase-partial.sh
> +++ b/t/t3401-rebase-partial.sh
> @@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' '
>  	git commit --allow-empty -m empty &&
>  	test_commit D &&
>  	git rebase C &&
> -	test $(git log --format=%s C..) = "D"
> +	test "$(git log --format=%s C..)" = "D"
> +'
> +
> +test_expect_success 'rebase --keep-empty' '
> +	git reset --hard D &&
> +	git rebase --keep-empty C &&
> +	test "$(git log --format=%s C..)" = "D
> +empty"
> +'
> +
> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '
> +	git reset --hard A &&
> +	git commit --allow-empty -m also-empty &&
> +	git rebase --keep-empty D &&
> +	test "$(git log --format=%s A..)" = "also-empty
> +D
> +empty"
>  '
>  
>  test_done
> -- 
> 1.7.11.1.104.ge7b44f1
> 
> 

RE: [PATCH v2] add tests for 'git rebase --keep-empty'

[Joachim Schmitz]

Hi folks

I'm a brand new subscriper of this mailing list, so please forgive if I
violate some protocol or talk about things that had been discussed to death
earlier.

I'm currently in the process of porting git (1.7.11.4 for now) to the HP
NonStop platform and found several issues:

- HP NonStop is lacking poll(), git is making quite some use of it.
My Solution: I 'stole' the implementation from GNUlib, which implements
poll() using select().
Git should either provide its own poll(), not use it at all or resort to
using GNUlib, what do you think?.

- HP NonStop is lacking getrlimit(), fsync(), setitimer() and memory mapped
IO.
For now I've commented out the part that used getrlimit() and use a home
brewed implementation for fsync(), setitimer() and mmap().

- git makes use of some C99 features or at least feature that are not
availabe in C89, like 'inline'
C89 is the default compiler on HP NonStop, but we also habe a c99 compiler,
so telling configure to search for c99  should help here.

- libintl and libiconv sem to get linked in the wrong order, resulting in
unresolved symbols.
I've just moved the "ifndef NO_GETTEXT" section of Makefile to above the
"ifdef NEEDS_LIBICONF" section.

- HP NonStop doesn't have stat.st_blocks, this is used in
builtin/count-objects.c around line 45, not sure yet how to fix that.

- HP NonStop doesn't have stat.st_?time.nsec, there are several places what
an "#ifdef USE_NSEC" is missing, I can provide a diff if needed (offending
files: builtin/fetch-pack.c and read-cache.c). 

- HP NonStop doesn't know SA_RESTART
I fixed that with a "#define SA_RESTART 0" in the 3 files affected
(builtin/log.c, fast-import.c and progress.c)

- using C99 but not using #include <strings.h> results in compiler errors
due to a missing prototype for strcasecmp()
I fixed it by adding that to git-compat-util.h

- HP NonStop doesn't have intptr_t and uintpr_t (in its stdint.h)
I added them to git-compat-util.h

- HP NonStop doesn't need the " #define _XOPEN_SOURCE 600", just like
__APPLE__, __FreeBSD__ etc, so I added a "&& !defined(__TANDEM) in
git-compat-util.h

- there seems to be an issue with compat/fnmatch/fnmatch.c not including
string.h, seems that HAVE_STRING_H is not #define'd anywhere.


- Once compiled and installed, a simple 
jojo@\hpitug:/home/jojo/GitHub $ git clone git://github.com/git/git.git
fails with:
/home/jojo/GitHub/git/.git/branches/: No such file or directory
After creating those manually it fails because the directory isn't empty,
catch-22
After some trial'n'error I found that the culprit seems to be the
subdirectories branches, hook and info in
/usr/local/share/git-core/templates/, if I remove/rename those, the above
command works fine.
I have no idea why that is nor how to properly fix it, anyone out there?

Bye, Jojo

Porting to a new platform

[Junio C Hamano]

"Joachim Schmitz" <jojo@schmitz-digital.de> writes:

> - HP NonStop is lacking poll(),...
> - HP NonStop is lacking getrlimit(), fsync(), setitimer()...

I would check compat/win32 and friends and see what other platforms
that lack this and that do, if I were you.

> so telling configure to search for c99  should help here.

In general, the top-level Makefile is designed to be usable without
ever worrying about "configure" mess.  Just define CC for the
platform section there, and optionally add a support to flip the
same in configure.ac.  This applies equally to other conditional
compilation options you may have to add to support your platform.

RE: Porting to a new platform

[Joachim Schmitz]

> From: Junio C Hamano [mailto:gitster@pobox.com]
> Sent: Friday, August 10, 2012 6:21 PM
> To: Joachim Schmitz
> Cc: git@vger.kernel.org; rsbecker@nexbridge.com
> Subject: Porting to a new platform
> 
> "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
> 
> > - HP NonStop is lacking poll(),...
> > - HP NonStop is lacking getrlimit(), fsync(), setitimer()...
> 
> I would check compat/win32 and friends and see what other platforms that
lack
> this and that do, if I were you.

Hmm, in compat/win32/poll.c I found exactly the same code I stole' for my
implementation (GNUlib's implementation) so I just managed to reinvent the
wheel :-(
Thanks anyway for telling me about it.

For getrlimit(RLIMIT_NOFILE, ...), I'm now using sysconf(_SC_OPEN_MAX), does
that sound reasonable?

I found no replacement for fsync() and setitimer(), but have my ones since
long, so no real need.

Also would compat/pread.c, another API HP NonStop is missing, but I had my
own implementation for that since quite a while already (and it looks pretty
similar to git's one).

I don't quite understand though why neither compat/pread.c nor
compat/win32/poll.c are used automatically after having been proven absent
in configure?
Ahh, I see, it could be done by adding a HP NonStop specific section in
Makefile ("ifeq ($(uname_S),NONSTOP_KERNEL)"), right?
I'll have a deeper look and see whether I can come up with something useful
to feed back into git.

> > so telling configure to search for c99  should help here.
> 
> In general, the top-level Makefile is designed to be usable without ever
> worrying about "configure" mess.  Just define CC for the platform section
there,

Yes, that's what I did, sort of, I just set CC to c99 prior to executing
configure.
I've seen other configure though, that explicitly test for C99, so why not
this one?

> and optionally add a support to flip the same in configure.ac.  This
applies
> equally to other conditional compilation options you may have to add to
> support your platform.

Bye, Jojo