patchadd test for 'git rebase --keep-empty'

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(-)

On Wed, Aug 08, 2012 at 09:48:18AM -0700, Martin von Zweigbergk wrote:

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

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(-)

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

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.

On Thu, Aug 09, 2012 at 08:39:51AM -0700, Martin von Zweigbergk wrote:

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

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

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

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

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.

lack

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.

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?

applies

Bye, Jojo