{"thread":{"id":"31212","subject":"[PATCH] add test for 'git rebase --keep-empty'","startedAt":"2012-08-08T16:48:18Z","lastAt":"2012-08-10T16:59:37Z","messageCount":8,"participants":["Martin von Zweigbergk","Neil Horman","Junio C Hamano","Joachim Schmitz"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196672","messageId":"1344444498-29328-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"31212","inReplyTo":null,"subject":"[PATCH] add test for 'git rebase --keep-empty'","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-08T16:48:18Z","receivedAt":"2012-08-08T16:48:18Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Signed-off-by: Martin von Zweigbergk \n---\n\nWhile trying to use patch-id instead of\n--ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that\npatch-id ignores empty patches and I was surprised that tests still\npass. This test case would be useful to protect --keep-empty.\n\n t/t3401-rebase-partial.sh | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\nindex 7f8693b..b89b512 100755\n--- a/t/t3401-rebase-partial.sh\n+++ b/t/t3401-rebase-partial.sh\n@@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' '\n \tgit commit --allow-empty -m empty &&\n \ttest_commit D &&\n \tgit rebase C &&\n-\ttest $(git log --format=%s C..) = \"D\"\n+\ttest \"$(git log --format=%s C..)\" = \"D\"\n+'\n+\n+test_expect_success 'rebase --keep-empty' '\n+\tgit reset --hard D &&\n+\tgit rebase --keep-empty C &&\n+\ttest \"$(git log --format=%s C..)\" = \"D\n+empty\"\n '\n \n test_done\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196675","messageId":"20120808165931.GA1481@hmsreliant.think-freely.org","threadId":"31212","inReplyTo":"1344444498-29328-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH] add test for 'git rebase --keep-empty'","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-08-08T16:59:31Z","receivedAt":"2012-08-08T16:59:31Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Wed, Aug 08, 2012 at 09:48:18AM -0700, Martin von Zweigbergk wrote:\n> Signed-off-by: Martin von Zweigbergk \n> ---\n> \n> While trying to use patch-id instead of\n> --ignore-if-in-upstream/--cherry-pick/cherry/etc, I noticed that\n> patch-id ignores empty patches and I was surprised that tests still\n> pass. This test case would be useful to protect --keep-empty.\n> \n> t/t3401-rebase-partial.sh | 9 ++++++++-\n> 1 file changed, 8 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\n> index 7f8693b..b89b512 100755\n> --- a/t/t3401-rebase-partial.sh\n> +++ b/t/t3401-rebase-partial.sh\n> @@ -47,7 +47,14 @@ test_expect_success 'rebase ignores empty commit' '\n> \tgit commit --allow-empty -m empty &&\n> \ttest_commit D &&\n> \tgit rebase C &&\n> -\ttest $(git log --format=%s C..) = \"D\"\n> +\ttest \"$(git log --format=%s C..)\" = \"D\"\n> +'\n> +\n> +test_expect_success 'rebase --keep-empty' '\n> +\tgit reset --hard D &&\n> +\tgit rebase --keep-empty C &&\n> +\ttest \"$(git log --format=%s C..)\" = \"D\n> +empty\"\n> '\n> \n> test_done\n> -- \n> 1.7.11.1.104.ge7b44f1\n> \n> \n\nAcked-by: Neil Horman \n"},{"id":"196734","messageId":"1344526791-13539-1-git-send-email-martin.von.zweigbergk@gmail.com","threadId":"31212","inReplyTo":"1344444498-29328-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"[PATCH v2] add tests for 'git rebase --keep-empty'","fromName":"Martin von Zweigbergk","fromEmail":"martin.von.zweigbergk@gmail.com","sentAt":"2012-08-09T15:39:51Z","receivedAt":"2012-08-09T15:39:51Z","isPatch":true,"sender":{"key":"martinvonz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/891642?v=4"},"body":"Add test cases for 'git rebase --keep-empty' with and without an\n\"empty\" commit already in upstream. The empty commit that is about to\nbe rebased should be kept in both cases.\n\nSigned-off-by: Martin von Zweigbergk \n---\n\nAdded another test for when the upstream already has an empty\ncommit. The test case protects the current behavior; I just assume the\ncurrent behavior is what we want.\n\nWhile writing the test case, I also noticed that an interrupted 'git\nrebase --keep-empty' can not be continued 'git rebase --continue', but\ninstead needs 'git cherry-pick --continue'. I guess this shouldn't\nreally be surprising given that it's implemented in terms of\ncherry-pick. This should be fixed once all the different kinds of\nrebase use the same way of finding the commits to rebase, so I\nwouldn't worry about fixing this specific problem right now.\n\n t/t3401-rebase-partial.sh | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\nindex 7f8693b..58f4823 100755\n--- a/t/t3401-rebase-partial.sh\n+++ b/t/t3401-rebase-partial.sh\n@@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' '\n \tgit commit --allow-empty -m empty &&\n \ttest_commit D &&\n \tgit rebase C &&\n-\ttest $(git log --format=%s C..) = \"D\"\n+\ttest \"$(git log --format=%s C..)\" = \"D\"\n+'\n+\n+test_expect_success 'rebase --keep-empty' '\n+\tgit reset --hard D &&\n+\tgit rebase --keep-empty C &&\n+\ttest \"$(git log --format=%s C..)\" = \"D\n+empty\"\n+'\n+\n+test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '\n+\tgit reset --hard A &&\n+\tgit commit --allow-empty -m also-empty &&\n+\tgit rebase --keep-empty D &&\n+\ttest \"$(git log --format=%s A..)\" = \"also-empty\n+D\n+empty\"\n '\n \n test_done\n-- \n1.7.11.1.104.ge7b44f1\n"},{"id":"196744","messageId":"7v628sou8v.fsf@alter.siamese.dyndns.org","threadId":"31212","inReplyTo":"1344526791-13539-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH v2] add tests for 'git rebase --keep-empty'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-09T17:22:40Z","receivedAt":"2012-08-09T17:22:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin von Zweigbergk writes:\n\n> Add test cases for 'git rebase --keep-empty' with and without an\n> \"empty\" commit already in upstream. The empty commit that is about to\n> be rebased should be kept in both cases.\n>\n> Signed-off-by: Martin von Zweigbergk \n> ---\n>\n> Added another test for when the upstream already has an empty\n> commit. The test case protects the current behavior; I just assume the\n> current behavior is what we want.\n\nThanks. I think it makes sense, as \"upstream already has an empty\ncommit\" together with \"want to keep empty while rebasing\" is a\nstrong sign that the user wants to have a history littered with many\nempty commits. Unlike a normal commit whose \"patch-id\" identity may\nhave meaningful significance (i.e. \"the change to do X is already\nin, or not yet in, this branch\"), all the empty commits share the\nsame emptiness, so having one empty somewhere long time ago in the\nhistory of where we are transplanting the commits shouldn't be a\nreason to countermand the \"want to keep empty\" wish by the user.\n\nAnd I do not think the conclusion would change even if we changed\nthe definition of \"identity\" for empty commits so that two empty\ncommits with the same message are detected as equal. The only semi\nsensible justification I heard from people who want to have empty\ncommits in their history is to keep in-history \"notes\" (e.g. \"at\nthis point in the series, the code stops to compile, but the next\none fixes it\", \"it is possible that we may want to redo the previous\npatch but I dunno\"), and it may not make sense to drop such an empty\ncommit under \"--keep-empty\" mode if there are similar or identical\nlooking \"notes\" in the \"upstream\" part of the history.\n"},{"id":"196790","messageId":"20120810132608.GA29609@hmsreliant.think-freely.org","threadId":"31212","inReplyTo":"1344526791-13539-1-git-send-email-martin.von.zweigbergk@gmail.com","subject":"Re: [PATCH v2] add tests for 'git rebase --keep-empty'","fromName":"Neil Horman","fromEmail":"nhorman@tuxdriver.com","sentAt":"2012-08-10T13:26:08Z","receivedAt":"2012-08-10T13:26:08Z","isPatch":true,"sender":{"key":"nhorman@tuxdriver.com","avatar":"https://avatars.githubusercontent.com/u/1032926?v=4"},"body":"On Thu, Aug 09, 2012 at 08:39:51AM -0700, Martin von Zweigbergk wrote:\n> Add test cases for 'git rebase --keep-empty' with and without an\n> \"empty\" commit already in upstream. The empty commit that is about to\n> be rebased should be kept in both cases.\n> \n> Signed-off-by: Martin von Zweigbergk \nAcked-by: Neil Horman \n\n> ---\n> \n> Added another test for when the upstream already has an empty\n> commit. The test case protects the current behavior; I just assume the\n> current behavior is what we want.\n> \n> While writing the test case, I also noticed that an interrupted 'git\n> rebase --keep-empty' can not be continued 'git rebase --continue', but\n> instead needs 'git cherry-pick --continue'. I guess this shouldn't\n> really be surprising given that it's implemented in terms of\n> cherry-pick. This should be fixed once all the different kinds of\n> rebase use the same way of finding the commits to rebase, so I\n> wouldn't worry about fixing this specific problem right now.\n> \n> t/t3401-rebase-partial.sh | 18 +++++++++++++++++-\n> 1 file changed, 17 insertions(+), 1 deletion(-)\n> \n> diff --git a/t/t3401-rebase-partial.sh b/t/t3401-rebase-partial.sh\n> index 7f8693b..58f4823 100755\n> --- a/t/t3401-rebase-partial.sh\n> +++ b/t/t3401-rebase-partial.sh\n> @@ -47,7 +47,23 @@ test_expect_success 'rebase ignores empty commit' '\n> \tgit commit --allow-empty -m empty &&\n> \ttest_commit D &&\n> \tgit rebase C &&\n> -\ttest $(git log --format=%s C..) = \"D\"\n> +\ttest \"$(git log --format=%s C..)\" = \"D\"\n> +'\n> +\n> +test_expect_success 'rebase --keep-empty' '\n> +\tgit reset --hard D &&\n> +\tgit rebase --keep-empty C &&\n> +\ttest \"$(git log --format=%s C..)\" = \"D\n> +empty\"\n> +'\n> +\n> +test_expect_success 'rebase --keep-empty keeps empty even if already in upstream' '\n> +\tgit reset --hard A &&\n> +\tgit commit --allow-empty -m also-empty &&\n> +\tgit rebase --keep-empty D &&\n> +\ttest \"$(git log --format=%s A..)\" = \"also-empty\n> +D\n> +empty\"\n> '\n> \n> test_done\n> -- \n> 1.7.11.1.104.ge7b44f1\n> \n> \n"},{"id":"196796","messageId":"003901cd7708$fa482c10$eed88430$@schmitz-digital.de","threadId":"31212","inReplyTo":"20120810132608.GA29609@hmsreliant.think-freely.org","subject":"RE: [PATCH v2] add tests for 'git rebase --keep-empty'","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-10T15:01:09Z","receivedAt":"2012-08-10T15:01:09Z","isPatch":true,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"Hi folks\n\nI'm a brand new subscriper of this mailing list, so please forgive if I\nviolate some protocol or talk about things that had been discussed to death\nearlier.\n\nI'm currently in the process of porting git (1.7.11.4 for now) to the HP\nNonStop platform and found several issues:\n\n- HP NonStop is lacking poll(), git is making quite some use of it.\nMy Solution: I 'stole' the implementation from GNUlib, which implements\npoll() using select().\nGit should either provide its own poll(), not use it at all or resort to\nusing GNUlib, what do you think?.\n\n- HP NonStop is lacking getrlimit(), fsync(), setitimer() and memory mapped\nIO.\nFor now I've commented out the part that used getrlimit() and use a home\nbrewed implementation for fsync(), setitimer() and mmap().\n\n- git makes use of some C99 features or at least feature that are not\navailabe in C89, like 'inline'\nC89 is the default compiler on HP NonStop, but we also habe a c99 compiler,\nso telling configure to search for c99 should help here.\n\n- libintl and libiconv sem to get linked in the wrong order, resulting in\nunresolved symbols.\nI've just moved the \"ifndef NO_GETTEXT\" section of Makefile to above the\n\"ifdef NEEDS_LIBICONF\" section.\n\n- HP NonStop doesn't have stat.st_blocks, this is used in\nbuiltin/count-objects.c around line 45, not sure yet how to fix that.\n\n- HP NonStop doesn't have stat.st_?time.nsec, there are several places what\nan \"#ifdef USE_NSEC\" is missing, I can provide a diff if needed (offending\nfiles: builtin/fetch-pack.c and read-cache.c). \n\n- HP NonStop doesn't know SA_RESTART\nI fixed that with a \"#define SA_RESTART 0\" in the 3 files affected\n(builtin/log.c, fast-import.c and progress.c)\n\n- using C99 but not using #include results in compiler errors\ndue to a missing prototype for strcasecmp()\nI fixed it by adding that to git-compat-util.h\n\n- HP NonStop doesn't have intptr_t and uintpr_t (in its stdint.h)\nI added them to git-compat-util.h\n\n- HP NonStop doesn't need the \" #define _XOPEN_SOURCE 600\", just like\n__APPLE__, __FreeBSD__ etc, so I added a \"&& !defined(__TANDEM) in\ngit-compat-util.h\n\n- there seems to be an issue with compat/fnmatch/fnmatch.c not including\nstring.h, seems that HAVE_STRING_H is not #define'd anywhere.\n\n\n- Once compiled and installed, a simple \njojo@\\hpitug:/home/jojo/GitHub $ git clone git://github.com/git/git.git\nfails with:\n/home/jojo/GitHub/git/.git/branches/: No such file or directory\nAfter creating those manually it fails because the directory isn't empty,\ncatch-22\nAfter some trial'n'error I found that the culprit seems to be the\nsubdirectories branches, hook and info in\n/usr/local/share/git-core/templates/, if I remove/rename those, the above\ncommand works fine.\nI have no idea why that is nor how to properly fix it, anyone out there?\n\nBye, Jojo\n"},{"id":"196804","messageId":"7v1ujelnvm.fsf_-_@alter.siamese.dyndns.org","threadId":"31212","inReplyTo":"003901cd7708$fa482c10$eed88430$@schmitz-digital.de","subject":"Porting to a new platform","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-10T16:20:45Z","receivedAt":"2012-08-10T16:20:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Joachim Schmitz\" writes:\n\n> - HP NonStop is lacking poll(),...\n> - HP NonStop is lacking getrlimit(), fsync(), setitimer()...\n\nI would check compat/win32 and friends and see what other platforms\nthat lack this and that do, if I were you.\n\n> so telling configure to search for c99 should help here.\n\nIn general, the top-level Makefile is designed to be usable without\never worrying about \"configure\" mess. Just define CC for the\nplatform section there, and optionally add a support to flip the\nsame in configure.ac. This applies equally to other conditional\ncompilation options you may have to add to support your platform.\n"},{"id":"196809","messageId":"004301cd7719$86b810b0$94283210$@schmitz-digital.de","threadId":"31212","inReplyTo":"7v1ujelnvm.fsf_-_@alter.siamese.dyndns.org","subject":"RE: Porting to a new platform","fromName":"Joachim Schmitz","fromEmail":"jojo@schmitz-digital.de","sentAt":"2012-08-10T16:59:37Z","receivedAt":"2012-08-10T16:59:37Z","isPatch":false,"sender":{"key":"jojo@schmitz-digital.de","avatar":"https://avatars.githubusercontent.com/u/1786669?v=4"},"body":"> From: Junio C Hamano [mailto:gitster@pobox.com]\n> Sent: Friday, August 10, 2012 6:21 PM\n> To: Joachim Schmitz\n> Cc: git@vger.kernel.org; rsbecker@nexbridge.com\n> Subject: Porting to a new platform\n> \n> \"Joachim Schmitz\" writes:\n> \n> > - HP NonStop is lacking poll(),...\n> > - HP NonStop is lacking getrlimit(), fsync(), setitimer()...\n> \n> I would check compat/win32 and friends and see what other platforms that\nlack\n> this and that do, if I were you.\n\nHmm, in compat/win32/poll.c I found exactly the same code I stole' for my\nimplementation (GNUlib's implementation) so I just managed to reinvent the\nwheel :-(\nThanks anyway for telling me about it.\n\nFor getrlimit(RLIMIT_NOFILE, ...), I'm now using sysconf(_SC_OPEN_MAX), does\nthat sound reasonable?\n\nI found no replacement for fsync() and setitimer(), but have my ones since\nlong, so no real need.\n\nAlso would compat/pread.c, another API HP NonStop is missing, but I had my\nown implementation for that since quite a while already (and it looks pretty\nsimilar to git's one).\n\nI don't quite understand though why neither compat/pread.c nor\ncompat/win32/poll.c are used automatically after having been proven absent\nin configure?\nAhh, I see, it could be done by adding a HP NonStop specific section in\nMakefile (\"ifeq ($(uname_S),NONSTOP_KERNEL)\"), right?\nI'll have a deeper look and see whether I can come up with something useful\nto feed back into git.\n\n> > so telling configure to search for c99 should help here.\n> \n> In general, the top-level Makefile is designed to be usable without ever\n> worrying about \"configure\" mess. Just define CC for the platform section\nthere,\n\nYes, that's what I did, sort of, I just set CC to c99 prior to executing\nconfigure.\nI've seen other configure though, that explicitly test for C99, so why not\nthis one?\n\n> and optionally add a support to flip the same in configure.ac. This\napplies\n> equally to other conditional compilation options you may have to add to\n> support your platform.\n\nBye, Jojo\n"}]}