RE: [PATCH] remote-helpers: point at their upstream repositories
Junio C Hamano wrote: Two announcements for their version 0.2 on the list archive are not quite enough to advertise them to their users. Signed-off-by: Junio C Hamano gits...@pobox.com --- * I am inclined to queue this for 2.0, and we would also need an update to the release notes as well. I am undecided about the revert I sent earlier in $gmane/248937; with or without it, that is just a contrib/ thing that is not well maintained inside our tree anyway. contrib/remote-helpers/README | 11 +++ 1 file changed, 11 insertions(+) create mode 100644 contrib/remote-helpers/README NAK. diff --git a/contrib/remote-helpers/README b/contrib/remote-helpers/README new file mode 100644 index 000..72a2df4 --- /dev/null +++ b/contrib/remote-helpers/README @@ -0,0 +1,11 @@ +The remote-helper bridges to access data stored in Hg and bzr will be They are called Mercurial and Bazaar. +maintained outside the git.git tree in the repositories of its +primary author at: + +https://github.com/felipec/git-remote-hg +https://github.com/felipec/git-remote-bzr If this is formatted in asciidoc the links won't appear as links. Do it as I did: * https://github.com/felipec/git-remote-hg * https://github.com/felipec/git-remote-bzr +As a convenience, copies of the last-bundled version of these two +remote-helper bridges are kept here, but they may left stale. Users +are encouraged to visit the above authoritative repositories for the +latest versions to get involved in its further developments. 1) Most users will *never* see this README 2) Most packagers will never see this README 3) The people that do read this README will wonder *why* they are now maintained separately. Thanks for wasting all my hard work and sabotaging these projects. Just as a heads-up. I *will* complain about this publicly and my blog is visited by thousands of people. I am requesting one last time: 1) Add warnings *directly* into the tools themselves ASAP You didn't have any problems adding warnings for pre-v2.0 behavior that changed, nor did you have a problem adding the warning about the new zsh completion that moved out of the bash one. Why would you have a problem with this one? 2) Replace the tools with stubs that point to the right locations at the earliest convenience I already sent patches for both, and they were ignored. A failure to do both will result in a lack of visibility of the new projects, and a decrease in the quality the users of git.git contrib area's remote-helpers receive. I will consider that a deliberate attempt to make the new projects experience unnecessary hardship. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] Gitweb: Convert UTF-8 encoded file names
On Fri, May 16, 2014 at 3:26 AM, Junio C Hamano gits...@pobox.com wrote: Jakub Narębski jna...@gmail.com writes: On Thu, May 15, 2014 at 9:38 PM, Junio C Hamano gits...@pobox.com wrote: Jakub Narębski jna...@gmail.com writes: Writing test for this would not be easy, and require some HTML parser (WWW::Mechanize, Web::Scraper, HTML::Query, pQuery, ... or low level HTML::TreeBuilder, or other low level parser). Hmph. Is it more than just looking for a specific run of %xx we would expect to see in the output of the tree view for a repository in which there is one tree with non-ASCII name? There is if we want to check (in non-fragile way) that said specific run is in 'href' *attribute* of 'a' element (link target). Correct, but is where does it appear the question we are primarily interested in, wrt this breakage and its fix? That of course depends on how we want to test gitweb output. The simplest solution, comparing with known output with perhaps fragile / variable elements masked out could be done quickly... but changes in output (even if they don't change functionality, or don't change visible output) require regenerating test cases (expected output) to test against - which might be source of errors in test suite. Another simple solution, grepping for expected strings, also easy to create, has the disadvantage of being only positive test - you cannot [easily] test that there are no *wrong* output, only that right string exists somewhere. If gitweb output has some volatile parts that do not depend on the contents of the Git test repository (e.g. showing contents of /etc/motd, date/time of when the test was run, or the full pathname leading to the trash directory), then preparing a tree whose name is äéìõû and making sure that the properly encoded version of äéìõû appears anywhere in the output may not be sufficient to validate that we got the encoding right, as that string may appear in the parts that are totally unrelated to the contents being shown and not under our control. But is that really the case? Well, I guess that any test is better than no test (though OTOH Heartbleed and goto fail bugs shows the importance of negative tests). Also we may introduce a bug and misspell the attr name and produce an anchor element with hpef attribute with the properly encoded URL in it, and your parse HTML properly approach would catch it, but is that the kind of breakage under discussion? You hinted at new tests for UTF-8 encoding in the other message in the thread earlier, and I've been assuming that we were talking about the encoding test, not a test to catch s/href/hpef/ kind of breakage. One of tests possible with HTML parser (e.g. WWW::Mechanize::CGI) is to check that all [internal] links leads to 200-OK pages, which accidentally would also be a test against this breakage. -- Jakub Narebski -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] format-patch --signature-file file
On Thu, May 15, 2014 at 06:31:21PM -0700, Jeremiah Mahler wrote:
Added feature that allows a signature file to be used with format-patch.
$ git format-patch --signature-file ~/.signature -1
Now signatures with newlines and other special characters can be
easily included.
I think this version looks nicer than the original.
A few questions/comments:
+static int signature_file_callback(const struct option *opt, const char *arg,
+ int unset)
+{
+ const char **signature = opt-value;
+ static char buf[1024];
+ size_t sz;
+ FILE *fd;
+
+ fd = fopen(arg, r);
+ if (fd) {
+ sz = sizeof(buf);
+ sz = fread(buf, 1, sz - 1, fd);
+ if (sz) {
+ buf[sz] = '\0';
+ *signature = buf;
+ }
+ fclose(fd);
+ }
+ return 0;
+}
We have routines for reading directly into a strbuf, which eliminates
the need for this 1024-byte limit. We even have a wrapper that can make
this much shorter:
struct strbuf buf = STRBUF_INIT;
strbuf_read_file(buf, arg, 128);
*signature = strbuf_detach(buf, NULL);
I notice that you ignore any errors. Is that intentional (so that we
silently ignore missing --signature files)? If so, should we actually
treat it as an empty file (e.g., in my code above, we always set
*signature, even if the file was missing)?
Finally, I suspect that:
cd path/in/repo
git format-patch --signature-file=foo
will not work, as we chdir() to the toplevel before evaluating the
arguments. You can fix that either by using parse-option's OPT_FILENAME
to save the filename, followed by opening the file after all arguments
are processed; or by manually fixing up the filename.
Since parse-options already knows how to do this fixup (it does it for
OPT_FILENAME), it would be nice if it were a flag rather than a full
type, so you could specify at as an option to your callback here:
+ { OPTION_CALLBACK, 0, signature-file, signature,
N_(signature-file),
+ N_(add a signature from contents of a file),
+ PARSE_OPT_NONEG, signature_file_callback },
Noticing your OPT_NONEG, though, I wonder if you should simply use an
OPT_FILENAME. I would expect --no-signature-file to countermand any
earlier --signature-file on the command-line (or if we eventually grow a
config option, which seems sensible, it would tell git to ignore the
option). The usual ordering for that is:
1. Read config and store format.signatureFile as a string
signature_file.
2. Parse arguments. --signature-file=... sets signature_file, and
--no-signature-file sets it to NULL.
3. If signature_file is non-NULL, load it.
And I believe OPT_FILENAME will implement (2) for you.
One downside of doing it this way is that you need to specify what will
happen when both --signature (or format.signature) and
--signature-file are set. With your current code, I think
--signature=foo --signature-file=bar will take the second one. I think
it would be fine to prefer one to the other, or to just notice that both
are set and complain.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/17] contrib: remove outdated README
Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: == contrib vs. core == This is the only point relevant to contrib vs. core: - We may be painted in a hard place if remote-hg or remote-bzr take us to a position where the Git as a whole is blocked while it is incompatible with the other side. It will never happen. I already argued against it[1], multiple times. Essentially making the tests optional removes any possibility of blockage (pluse many more arguments). I already said that your It will never happen is a handwaving, and I already saw you argued against it. This is a red herring. Ignore the fact that it will never happen (which it won't), the next point remains a FACT, and you conveniently ignore it. ANSWER THIS: Essentially making the tests optional removes any possibility of blockage (pluse many more arguments). If the tests are optional, it doesn't matter if such change you are worried about happens or not (it won't). That's all there is to it. You made a wrong call. The tools *can* move into the core, and you said they couldn't. You may have been interested in contrib/core in the thread, but that does not prevent others from considering other aspects of the issue and different and possibly better solutions, which was to unbundle. That is IRRELEVANT to the fact that the tools *could* move into the core. You are using arguments (refuted) to demonstrate that the tools *might be better served* by being unbundled, in order to explain why they *could not* move into the core. That's a red herring. I was very confident back in that thread that the remote Hg bridge would not merely survive but would serve its users well as a standalone project, And you were wrong. There is a greater risk for these bridges to become unmaintained if we do not have them in my tree, and that would only hurt our users. But I did not see that particular risk at all for the remote Hg bridge. You have been very interested in maintaining it, and I don't think I ever had to prod you to respond to an issue raised on the list. It is an apples-and-oranges comparison to bring up git-svn/p4. In other words; if I had done a poorer job of maintaining these tools, they would have had a higher chance of moving into the core? If that's the case I would gladly stop any maintenance on them. Just say the word. I worked extremely hard so they could become part of the core, if being part of the core means I have to maintain them less, so be it. Besides, I want to see the git-core has the best thing among competing implementations for a specific niche subtask perception changed in the longer term so that it becomes natural for users to say something like For this particular task, there is no support in what comes with core (or there is a tool that comes with core to address similar issues but in a way different from what you want), and the go-to tool these days for that kind of task is to use this third-party plugin, Where are you saying that? Nowhere, that's where. Therefore every tool you unbundle, you are throwing to the wolves. Things like git imerge are helping us to go in that direction. I was hoping that the remote Hg bridge to be capable of becoming the first demonstration to graduate out of contrib/ that shows the users that is the case, not with just talk but with a specific example. You told me you wanted them to move to the core, you even moved them to the core. Then after years of work you change your mind, AGAINST my explicit will and with clear strong arguments AGAINST your apparently newly conceived idea. And idea you didn't explain clearly, and which you didn't give any chance of being refuted. In other words; an idea which very well COULD BE WRONG, and which very well could impact negatively many Git users. But you cannot even ponder the notion that you could possibly be wrong. After seeing these discussions, it tells me that the code is not fit for the core (also [*3*]), and I think there no longer is any reason for us to still talk only about contrib vs core. As you already said, you do not want to see them in contrib/, No, I want to see them in the core, and you said you did too. More importantly the vast majority of our users would want to see them in the core, therefore the discussion of contrib vs. core is pretty much relevant, but you don't care about them, do you? As I said, I will complain about this publicly _to our users_, which you are disregarding completely with this poorly thought decision and the subsequent ones. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep -I: do not bother to read known-binary files
Hello, On Thu, May 15, 2014 at 03:22:26PM -0400, Jeff King wrote: As the person who is proposing the patch for git.git, I would hope Stepan would follow up on such review and confirm whether or not it is still needed. well, I try to. (I verified that less -I works in msysGit before submitting the fixup back there, fox example.) But I think my role is to moderate the reconciliation. In this case, having read the discussion, I decided to to ask Dscho to drop the patch. (It is always about balancing the risks and the expenses.) Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/10] git-mergetool.sh: don't use the -a or -b option with the test command
On Thu, May 15, 2014 at 07:17:35AM -0700, Elia Pinto wrote:
Even though POSIX.1 lists -a/-o as options to test, they are
marked Obsolescent XSI. Scripts using these expressions
should be converted as follow:
test $1 -a $2
should be written as:
test $1 test $2
Likewise
test $1 -o $2
should be written as:
test $1 test $2
But note that, in test, -a has higher precedence than -o while
and || have equal precedence in the shell.
The reason for this is that the precedence rules were never well
specified, and this made many sane-looking uses of test -a/-o problematic.
For example, if $x is =, these work according to POSIX (it's not
portable, but in practice it's okay):
$ test -z $x
$ test -z $x test a = b
but this doesn't
$ test -z $x -a a = b
bash: test: too many arguments
because it groups test -n = -a and is left with a = b.
Similarly, if $x is -f, these
$ test $x
$ test $x || test c = d
correctly adds an implicit -n, but this fails:
$ test $x -o c = d
bash: test: too many arguments
Signed-off-by: Elia Pinto gitter.spi...@gmail.com
---
Inspired from this discussion
http://permalink.gmane.org/gmane.comp.version-control.git/137056
Looks good, thanks.
Acked-by: David Aguilar dav...@gmail.com
git-mergetool.sh |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 332528f..88e853f 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -205,7 +205,7 @@ checkout_staged_file () {
$(git checkout-index --temp --stage=$1 $2 2/dev/null) \
: '\([^ ]*\)')
- if test $? -eq 0 -a -n $tmpfile
+ if test $? -eq 0 test -n $tmpfile
then
mv -- $(git rev-parse --show-cdup)$tmpfile $3
else
@@ -256,7 +256,7 @@ merge_file () {
checkout_staged_file 2 $MERGED $LOCAL
checkout_staged_file 3 $MERGED $REMOTE
- if test -z $local_mode -o -z $remote_mode
+ if test -z $local_mode || test -z $remote_mode
then
echo Deleted merge conflict for '$MERGED':
describe_file $local_mode local $LOCAL
--
1.7.10.4
--
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep -I: do not bother to read known-binary files
On Fri, May 16, 2014 at 10:19:57AM +0200, Stepan Kasal wrote: On Thu, May 15, 2014 at 03:22:26PM -0400, Jeff King wrote: As the person who is proposing the patch for git.git, I would hope Stepan would follow up on such review and confirm whether or not it is still needed. well, I try to. (I verified that less -I works in msysGit before submitting the fixup back there, fox example.) But I think my role is to moderate the reconciliation. In this case, having read the discussion, I decided to to ask Dscho to drop the patch. (It is always about balancing the risks and the expenses.) Sure, I think that is reasonable, and I hope I did not sound like blame Stepan, he was screwed up. After reading Dscho's other message and knowing that he has not much time for git, I was trying to communicate that I did not expect _him_ to be dealing with it. Git.git has existed for a long time without that patch, so from our perspective, it is a new patch. And as with any new patch, I would expect a submitter who receives review of eh, don't we already do this to either look into it, or decide that the review is probably right and not bother with it. And you did the latter, and that is totally fine and reasonable. From msysgit's perspective, they may or may not want to revert the patch that they already have. That is a _separate_ issue, and I think the burden of effort goes the other way. The patch should stay unless somebody goes to the work to confirm that it is not necessary (or unless they want to accept my say-so and indication that I did not do fresh testing, but that is up to them). Sorry if that is long and/or obvious. There have been enough bad feelings on the list lately that I didn't want there to be a misunderstanding about what I meant. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Truncated patch
The patch returned by http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=462fb2af9788a82a534f8184abfde31574e1cfa0 is truncated. The page which refers to that patch, at http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=462fb2af9788a82a534f8184abfde31574e1cfa0, shows the full patch. I am not subscribed to this list so please CC me in discussion. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
On Thu, May 15, 2014 at 03:56:29PM -0700, Junio C Hamano wrote: Two announcements for their version 0.2 on the list archive are not quite enough to advertise them to their users. I do not think this README nor a mention in the release notes will get their attention either, and many people (and packagers) will continue to use the stale versions forever until those versions go away. I would much rather _replace_ them with a README in the long run, and people will notice that they are gone, and then use the README to update their install procedure. For 2.0, I am hesitant to do that, though I do not have a problem with a README like this as a heads-up to prepare packagers for the future. I say hesitant because people may have been test-packaging 2.0.0-rc3 in preparation for release, and it will be annoying to them to suddenly switch. But that being said, this is Felipe's code. While we have a legal right to distribute it in v2.0, if he would really prefer it out for v2.0, I would respect that. I would prefer to instrument the code with warnings, as that is the sort of thing a packager moving from -rc3 to -final might not notice, and shipping the warnings to end users who did not package the software in the first place will not help them. It is the attention of the packagers (and source-builders) you want to get. Of course that is all just my two cents, and is mostly predicated on there _being_ packagers of the contrib/ tools. It looks like there is a Debian package in RFP status, but I don't know if that is following the new release closely. And I don't know about other systems. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Truncated patch
On Fri, May 16, 2014 at 06:04:57PM +0930, David Newall wrote: The patch returned by http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=462fb2af9788a82a534f8184abfde31574e1cfa0 is truncated. The page which refers to that patch, at http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=462fb2af9788a82a534f8184abfde31574e1cfa0, shows the full patch. Weird. It is truncated if I fetch it with curl, but it looks fine in my browser. It's truncated at exactly 4K, and the transfer-encoding is chunked. Maybe cgit (or kernel.org's webserver) is doing something non-standard with chunking that the browser understands but curl does not? -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
On Fri, May 16, 2014 at 10:41 AM, Jeff King p...@peff.net wrote: But that being said, this is Felipe's code. While we have a legal right to distribute it in v2.0, if he would really prefer it out for v2.0, I would respect that. My understanding is that Felipe would prefer to keep it _in_ the git.git repository and eventually get it included in the core. Regards, -- Paolo -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
On Fri, May 16, 2014 at 10:55:54AM +0200, Paolo Ciarrocchi wrote: On Fri, May 16, 2014 at 10:41 AM, Jeff King p...@peff.net wrote: But that being said, this is Felipe's code. While we have a legal right to distribute it in v2.0, if he would really prefer it out for v2.0, I would respect that. My understanding is that Felipe would prefer to keep it _in_ the git.git repository and eventually get it included in the core. Yes, sorry if I was unclear. I meant ...if we are going to remove it, and are considering leaving it in v2.0, we should respect his wishes to remove it earlier if he wants to. It is not his decision whether it stays in the core at all. That is Junio's decision. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/17] contrib: remove outdated README
On Fri, May 16, 2014 at 03:08:51AM -0500, Felipe Contreras wrote: Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: == contrib vs. core == This is the only point relevant to contrib vs. core: - We may be painted in a hard place if remote-hg or remote-bzr take us to a position where the Git as a whole is blocked while it is incompatible with the other side. It will never happen. I already argued against it[1], multiple times. Essentially making the tests optional removes any possibility of blockage (pluse many more arguments). I already said that your It will never happen is a handwaving, and I already saw you argued against it. This is a red herring. Ignore the fact that it will never happen (which it won't), the next point remains a FACT, and you conveniently ignore it. It may not block git being released, but as we can see from the recent patches that were needed to enable hg 3.0 support it can break and would have to follow *both* mercurial and git upstreams, not just git's. After thinking about this for a while, I would have to agree with Junio That it's better if a bridge between to actively developed applications not be coupled to one. This does not mean that I think git-remote-hg is not of a quality to be in the git.git tree, but it is simply a fact of development and stability. If git's remote-helper stuff changes but mercurial doesn't, we're fine because, having seen the speed of your fixes, we would have a fix before the next release without a doubt. However, if mercurial changes, like it just did, then git itself would need to make a release to have it actually work with the newest release. Having the tool out of tree allows the maintainer to fix things on both ends and release independently so that both situations above can be solved without any real hassle on git or mercurial's side. This goes for bzr, too, but it looks to be changing less quickly. tl;dr: This may not block a release, but it will make releases a lot more dependent on outside forces. Thanks, -- William Giokas | KaiSforza | http://kaictl.net/ GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF pgp52E8cms84y.pgp Description: PGP signature
Re: [PATCH] remote-helpers: point at their upstream repositories
Paolo Ciarrocchi wrote: On Fri, May 16, 2014 at 10:41 AM, Jeff King p...@peff.net wrote: But that being said, this is Felipe's code. While we have a legal right to distribute it in v2.0, if he would really prefer it out for v2.0, I would respect that. My understanding is that Felipe would prefer to keep it _in_ the git.git repository and eventually get it included in the core. That is correct. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] grep -I: do not bother to read known-binary files
Hi, On Fri, May 16, 2014 at 04:29:58AM -0400, Jeff King wrote: [..] I hope I did not sound like blame Stepan, he was screwed up. no, you did not, it was ok. From msysgit's perspective, they may or may not want to revert the patch that they already have. That is a _separate_ issue, and I think the I hope Dscho will help with the decision: he can say keep it until we have evidence or at least time to do a more thorough review, for example. Stepan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Truncated patch
On Fri, May 16, 2014 at 04:51:16AM -0400, Jeff King wrote: On Fri, May 16, 2014 at 06:04:57PM +0930, David Newall wrote: The patch returned by http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/patch/?id=462fb2af9788a82a534f8184abfde31574e1cfa0 is truncated. The page which refers to that patch, at http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=462fb2af9788a82a534f8184abfde31574e1cfa0, shows the full patch. Weird. It is truncated if I fetch it with curl, but it looks fine in my browser. It's truncated at exactly 4K, and the transfer-encoding is chunked. Maybe cgit (or kernel.org's webserver) is doing something non-standard with chunking that the browser understands but curl does not? I take it back. Now it is screwed up in my browser, too. Perhaps related to hitting different kernel.org servers (though all three IPs now give me the truncated output). Looking at the raw protocol, the chunked encoding is fine; it literally just quits after 4K, and sends the 0-byte all done chunk. So I'm guessing it's either a bug in cgit, or a temporary error that erroneously ended up cached (I don't know enough about cgit's or kernel.org's caching infrastructure to say more). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote-helpers: point at their upstream repositories
Jeff King wrote: On Thu, May 15, 2014 at 03:56:29PM -0700, Junio C Hamano wrote: Two announcements for their version 0.2 on the list archive are not quite enough to advertise them to their users. I do not think this README nor a mention in the release notes will get their attention either, and many people (and packagers) will continue to use the stale versions forever until those versions go away. I would much rather _replace_ them with a README in the long run, and people will notice that they are gone, and then use the README to update their install procedure. For 2.0, I am hesitant to do that, though I do not have a problem with a README like this as a heads-up to prepare packagers for the future. I say hesitant because people may have been test-packaging 2.0.0-rc3 in preparation for release, and it will be annoying to them to suddenly switch. I agree, that's why I sent patches that: 1) Add a warning for v2.0 (which already has problems) So everything keeps working as well as in the release candidates, except a warning is introduced each time they do a fetch, push, or clone operation (use the remote-helpers) 2) Replace the tools with stubs So when they try to fetch, push, or clone, they get an error, and nothing else happens. There's an additional README for the people that want to read more, but if you don't put stubs, users might try to do what worked before: % git clone hg::http://selenic.com/repo/hello fatal: Unable to find remote helper for 'hg' It's much better to report: git-remote-hg is now maintained independently. For more information visit https://github.com/felipec/git-remote-hg But that being said, this is Felipe's code. While we have a legal right to distribute it in v2.0, if he would really prefer it out for v2.0, I would respect that. That's right, you have the legal right to distributed it. It is not my wish to disrupt v2.0, so I think adding a warning should be sufficient. Eventually I would prefer if they were not distributed, and replaced with stubs, not just because it would help the out-of-tree projects gather more visibility, but also because users would be better served by not having poorly maintained or unsynchronized code. Hopefully for v2.1. I would prefer to instrument the code with warnings, as that is the sort of thing a packager moving from -rc3 to -final might not notice, and shipping the warnings to end users who did not package the software in the first place will not help them. It is the attention of the packagers (and source-builders) you want to get. Of course that is all just my two cents, and is mostly predicated on there _being_ packagers of the contrib/ tools. It looks like there is a Debian package in RFP status, but I don't know if that is following the new release closely. And I don't know about other systems. As far as I understand most distributions don't do anything special with contrib, they just put everything under /usr/share. It is unlikely packagers will notice any change in one of the dozens tools in contrib, so they'll make no changes to the packages. So if the user did: % ln -s /usr/share/git/remote-helpers/git-remote-hg ~/bin/git-remote-hg The expectation would be that this keeps working even if the package doesn't change (it just adds a warning). Eventually it will stop working with an error, but the package still won't change. The distributions that do something special about remote-helpers (AFAIK it's only debian's git-bzr) would need to change, and sooner or later they will if there's only stubs there. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v2 12/29] guilt header: more robust header selection.
On Fri, May 16, 2014 at 12:46 AM, Jeff Sipek jef...@josefsipek.net wrote: On Tue, May 13, 2014 at 10:30:48PM +0200, Per Cederqvist wrote: If you run something like guilt header '.*' the command would crash, because the grep comand that tries to ensure that the patch exist would detect a match, but the later code expected the match to be exact. Fixed by comparing exact strings. And as a creeping feature guilt header will now try to use the supplied patch name as an unachored regexp if no exact match was found. If the regexp yields a unique match, it is used; if more than one patch matches, the names of all patches are listed and the command fails. (Exercise left to the reader: generalized this so that guilt push also accepts a unique regular expression.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-header | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/guilt-header b/guilt-header index 41e00cc..4701b31 100755 --- a/guilt-header +++ b/guilt-header @@ -45,10 +45,32 @@ esac [ -z $patch ] die No patches applied. # check that patch exists in the series -ret=`get_full_series | grep -e ^$patch\$ | wc -l` -if [ $ret -eq 0 ]; then - die Patch $patch is not in the series +full_series=`get_tmp_file series` +get_full_series $full_series +found_patch= +while read x; do + if [ $x = $patch ]; then + found_patch=$patch + break + fi +done $full_series We have to use a temp file instead of a 'get_full_series | while read x; do ...' because that'd create a subshell, correct? Yes. Also (and probably less importantly) we sometimes need to run grep on the same output (see the creation of TMP_MATCHES below) and it would be a bit wasteful to run get_full_series twice. (The assumption is that it is cheaper to create a temp file than to recompute the value. I have not measured this, though.) +if [ -z $found_patch ]; then + TMP_MATCHES=`get_tmp_file series` + grep $patch $full_series $TMP_MATCHES + nr=`wc -l $TMP_MATCHES` + if [ $nr -gt 1 ]; then + echo $patch does not uniquely identify a patch. Did you mean any of these? 2 + sed 's/^/ /' $TMP_MATCHES 2 + rm -f $TMP_MATCHES + exit 1 + elif [ $nr -eq 0 ]; then + rm -f $TMP_MATCHES + die Patch $patch is not in the series + fi + found_patch=`cat $TMP_MATCHES` + rm -f $TMP_MATCHES fi +patch=$found_patch Do we not delete $full_series? Good catch. Will fix in the next version of the series. I'll also rename the variable $TMP_FULL_SERIES to adhere to the apparent coding style. (But I will not fix guilt-patchbomb that uses $dir as a temporary variable.) /ceder # FIXME: warn if we're editing an applied patch -- 1.8.3.1 -- OpenIndiana ibdm: 8 cores, 12 GB RAM -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [git-users] worlds slowest git repo- what to do?
On Fri, May 16, 2014 at 2:06 AM, Philip Oakley philipoak...@iee.org wrote: From: John Fisher fishook2...@gmail.com I assert based on one piece of evidence ( a post from a facebook dev) that I now have the worlds biggest and slowest git repository, and I am not a happy guy. I used to have the worlds biggest CVS repository, but CVS can't handle multi-G sized files. So I moved the repo to git, because we are using that for our new projects. goal: keep 150 G of files (mostly binary) from tiny sized to over 8G in a version-control system. I think your best bet so far is git-annex (or maybe bup) for dealing with huge files. I plan on resurrecting Junio's split-blob series to make core git handle huge files better, but there's no eta on that. The problem here is about file size, not the number of files, or history depth, right? problem: git is absurdly slow, think hours, on fast hardware. Probably known issues. But some elaboration would be nice (e.g. what operation is slow, how slow, some more detail characteristics of the repo..) in case new problems pop up. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fwd: [Bug] - Processing commit message after amend
Hi, I have stumbled on a weird bug. At work, we use redmine as an issue tracker and its task are marked by a number starting with #. When I commit some work and write #1234 in the message, it works. However, later on when I remember that I forgot to add some files and amend the commit, vim appears and I cannot perform the commit because the message starts with # which is a comment in vim and thus I get an error that my commit message is empty. Steps to reproduce: 1) commit a file git commit File1.txt -m #1234 documentation added 2) amend previous commit git commit File2.txt -- amend 3) go for :wq right away 4) an error that the message is empty appears Aborting commit due to empty commit message However, if you use amend and no edit option, it works git commit --amend --no-edit We use git for Windows downloaded here: http://git-scm.com/downloads The problem appears in Windows command line. I have not tested it anywhere else. The OS is Windows Server 2008 R2 Datacenter. Cheers from cloudy Prague Michal Staša Santhos.net www.santhos.net +420 773 454 793 michal.st...@santhos.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] - Processing commit message after amend
On Fri, May 16, 2014 at 5:18 PM, Michal Stasa michal.st...@gmail.com wrote: Hi, I have stumbled on a weird bug. At work, we use redmine as an issue tracker and its task are marked by a number starting with #. When I commit some work and write #1234 in the message, it works. However, later on when I remember that I forgot to add some files and amend the commit, vim appears and I cannot perform the commit because the message starts with # which is a comment in vim and thus I get an error that my commit message is empty. A workaround would be git -c core.commentChar=@ command ... (@ could be some other character). But maybe git should detect that the current commit message has leading '#' and automatically switch to another character.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/17] contrib: remove outdated README
William Giokas wrote: On Fri, May 16, 2014 at 03:08:51AM -0500, Felipe Contreras wrote: This is a red herring. Ignore the fact that it will never happen (which it won't), the next point remains a FACT, and you conveniently ignore it. It may not block git being released, but as we can see from the recent patches that were needed to enable hg 3.0 support it can break and would have to follow *both* mercurial and git upstreams, not just git's. Indeed, it *can* happen, nobody has argued otherwise. The thing is how *likely* is it that it will happen again. As I said, in my experience developing this there has never been a single instance where such a change was required for *newer* versions of Mercurial. From what I can tell this is an exceptional situation. And it makes sense that when it happened, it happened exactly in the part where I wrote custom push() method. Mercurial has unstable API, but in unstable API's are part that are more stable than others. Generally the higher level the API is, the more stable it tends to be. The Linux kernels makes an even weaker promise of API stability than Mercurial does, but even then, the higher the API you use, the less likelihood you have of breakage. I've seen this in practice many times. And it does makes sense that for practical purses Mercurial would try to avoid changes in the higher level API, which require changes in many places, and not so much in lower level APIs, which might require a few changes here and there. The problem is that in order to replace the higher level API push(), I had to use the lower level API, and that's where the breakage happened, it was not unlikely. It is unlikely that another change in this particular API will happen soon, but not extremely so. As I already explained, this can be mitigated by contacting the Mercurial developers and figure out if their high-level API can accommodate the kind of functionality git-remote-hg needs. Maybe by adding a new option, or maybe by adding another high-level helper function. Either way, I doubt Junio is qualified at all to discern between the likelihood of future Mercurial API changes that would break git-remote-hg. He has seen one, and he is wrongly assuming there are more to come. After thinking about this for a while, I would have to agree with Junio That it's better if a bridge between to actively developed applications not be coupled to one. How exactly would it be better? If you concede that the Git release wouldn't be affected, then assuming a hypothetical future where git-remote-hg is bundled, and we have a Mercurial API breakage, we would have: Git v2.5 fail, Git = 2.5 get the fix If we unbundle, we have: git-remote-hg v0.5 fail, git-remote-hg = v0.5 get the fix What is the big difference? I presume you would say that git-remote-hg v0.5 could be released earlier than Git v2.5, so the users would get the fix faster. However, that is not the case if the breakage is detected *before* the Mercurial release happened, in which case both Git v2.4 and git-remote-hg v0.4 would already contain the fix, and it doesn't matter much which was released first. The problem is that I wasn't doing the continuous integration with the development version of Mercurial, which I am now, so these kind of exceptional issues would be detected earlier. Moreover, it is likely that the distribution package for git-remote-hg would not be maintained as rigorously as the Git package. It might not even be part of the official packages (e.g. ppa or AUR). Therefore, even if the git-remote-hg v0.4 happens earlier, it might reach the users later. Furthermore, we are talking about a single script that can be installed by hand easily. The users can simply override their distribution's script and install by hand the latest version, as many have been doing already when they report errors and want the latest fix. Even more. git-remote-hg will not have maintenance releases, if an exceptional issue like this happens, it can be back-ported to Git v2.3.x, Git v2.2.x, and so on. It seems like a very feeble argument in favor of unbundling *at best*. This does not mean that I think git-remote-hg is not of a quality to be in the git.git tree, but it is simply a fact of development and stability. If git's remote-helper stuff changes but mercurial doesn't, we're fine because, having seen the speed of your fixes, we would have a fix before the next release without a doubt. However, if mercurial changes, like it just did, then git itself would need to make a release to have it actually work with the newest release. That is not true. In this particular case, because I didn't build against the development version of Mercurial, yes, but not for the future. If I had the tests I have in place now, we would have detected the change earlier, and Git v1.9.2 would *already* contain the fix, and when Mercurial v3.0 got released we wouldn't need to make a Git release in response (same goes for unbundled
Re: Fwd: [Bug] - Processing commit message after amend
Michal Stasa michal.st...@gmail.com writes: I have stumbled on a weird bug. At work, we use redmine as an issue tracker and its task are marked by a number starting with #. When I commit some work and write #1234 in the message, it works. However, later on when I remember that I forgot to add some files and amend the commit, vim appears and I cannot perform the commit because the message starts with # which is a comment in vim and thus I get an error that my commit message is empty. Steps to reproduce: 1) commit a file git commit File1.txt -m #1234 documentation added 2) amend previous commit git commit File2.txt -- amend 3) go for :wq right away git commit --amend -C HEAD File2.txt should do the trick without starting the editor. However, if you use amend and no edit option, it works git commit --amend --no-edit Ah, so you got your solution. It's not like you could add that sort of commit message interactively to start with, so it's not all that surprising that you won't be able to amend it interactively. -- David Kastrup -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] - Processing commit message after amend
It sounds like a good workaround but I think there could be a problem. When vim opens there is the message on the first line and two lines below is a commented text which uses # as comment char. Does the char change when you change the comment char? Michal Staša Santhos.net www.santhos.net +420 773 454 793 michal.st...@santhos.net On 16 May 2014 12:28, Duy Nguyen pclo...@gmail.com wrote: On Fri, May 16, 2014 at 5:18 PM, Michal Stasa michal.st...@gmail.com wrote: Hi, I have stumbled on a weird bug. At work, we use redmine as an issue tracker and its task are marked by a number starting with #. When I commit some work and write #1234 in the message, it works. However, later on when I remember that I forgot to add some files and amend the commit, vim appears and I cannot perform the commit because the message starts with # which is a comment in vim and thus I get an error that my commit message is empty. A workaround would be git -c core.commentChar=@ command ... (@ could be some other character). But maybe git should detect that the current commit message has leading '#' and automatically switch to another character.. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] commit: switch core.commentChar if it's found in existing commit
If we need to use core.commentChar and it's already in the prepared
message, find another char among a small subset. This should stop
surprises because git strips some lines unexpectedly. Of course if
candicate characters happen to be all out, this change does not help.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
On Fri, May 16, 2014 at 5:28 PM, Duy Nguyen pclo...@gmail.com wrote:
But maybe git should detect that the
current commit message has leading '#' and automatically switch to
another character..
Something like this. Lightly tested.. I know there's a small bug..
builtin/commit.c | 29 +
1 file changed, 29 insertions(+)
diff --git a/builtin/commit.c b/builtin/commit.c
index 6ab4605..70ceb61 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -593,6 +593,32 @@ static char *cut_ident_timestamp_part(char *string)
return ket;
}
+static void adjust_comment_line_char(const struct strbuf *sb)
+{
+ char candidates[] = !@#$%^|:;~;
+ char *candidate;
+ const char *p;
+ if (!sb-len)
+ return;
+ candidates[0] = comment_line_char;
+ p = sb-buf;
+ do {
+ candidate = strchr(candidates, *p);
+ if (candidate)
+ *candidate = ' ';
+ p = strchrnul(p, '\n');
+ if (*p)
+ p++;
+ } while (*p);
+ if (strchr(candidates, comment_line_char)) {
+ p = candidates;
+ while (*p *p == ' ')
+ p++;
+ if (*p)
+ comment_line_char = *p;
+ }
+}
+
static int prepare_to_commit(const char *index_file, const char *prefix,
struct commit *current_head,
struct wt_status *s,
@@ -748,6 +774,9 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
if (fwrite(sb.buf, 1, sb.len, s-fp) sb.len)
die_errno(_(could not write commit template));
+ if (use_editor include_status)
+ adjust_comment_line_char(sb);
+
strbuf_release(sb);
/* This checks if committer ident is explicitly given */
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 01/17] contrib: remove outdated README
On Fri, May 16, 2014 at 05:21:36AM -0500, Felipe Contreras wrote: How exactly would it be better? If you concede that the Git release wouldn't be affected, then assuming a hypothetical future where git-remote-hg is bundled, and we have a Mercurial API breakage, we would have: Git v2.5 fail, Git = 2.5 get the fix If we unbundle, we have: git-remote-hg v0.5 fail, git-remote-hg = v0.5 get the fix What is the big difference? It's a matter of scope and where the releases happen, that is all. Thank you, -- William Giokas | KaiSforza | http://kaictl.net/ GnuPG Key: 0x73CD09CF Fingerprint: F73F 50EF BBE2 9846 8306 E6B8 6902 06D8 73CD 09CF pgphjvVkMmMZ4.pgp Description: PGP signature
Re: [PATCH v2 01/17] contrib: remove outdated README
William Giokas wrote: On Fri, May 16, 2014 at 05:21:36AM -0500, Felipe Contreras wrote: How exactly would it be better? If you concede that the Git release wouldn't be affected, then assuming a hypothetical future where git-remote-hg is bundled, and we have a Mercurial API breakage, we would have: Git v2.5 fail, Git = 2.5 get the fix If we unbundle, we have: git-remote-hg v0.5 fail, git-remote-hg = v0.5 get the fix What is the big difference? It's a matter of scope and where the releases happen, that is all. Of course the core vs. out-of-tree question is a matter of where the releases happen. The question here was: in which way is out-of-tree a better place? If it's a matter of scope, that is; should a foreign vcs interface tool be bundled in the Git core? Then that question applies not only to git-remote-hg/bzr, but also git-p4, git-cvs, git-svn, and others. The answer to the first question seems to be; it's not at all clear (in fact there doesn't seem to be any valid argument in favour of out-of-tree). The answer to the second question is; we are not asking that question right now (for the moment foreign vcs tools should remain part of the Git core). I started the graduation series by saying there doesn't seem to be any good reason not to, and Junio agreed. Now Junio doesn't agree, but it's till the case there's no good reason not to. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v2 12/29] guilt header: more robust header selection.
On Fri, May 16, 2014 at 11:51:37AM +0200, Per Cederqvist wrote: On Fri, May 16, 2014 at 12:46 AM, Jeff Sipek jef...@josefsipek.net wrote: On Tue, May 13, 2014 at 10:30:48PM +0200, Per Cederqvist wrote: If you run something like guilt header '.*' the command would crash, because the grep comand that tries to ensure that the patch exist would detect a match, but the later code expected the match to be exact. Fixed by comparing exact strings. And as a creeping feature guilt header will now try to use the supplied patch name as an unachored regexp if no exact match was found. If the regexp yields a unique match, it is used; if more than one patch matches, the names of all patches are listed and the command fails. (Exercise left to the reader: generalized this so that guilt push also accepts a unique regular expression.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-header | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/guilt-header b/guilt-header index 41e00cc..4701b31 100755 --- a/guilt-header +++ b/guilt-header @@ -45,10 +45,32 @@ esac [ -z $patch ] die No patches applied. # check that patch exists in the series -ret=`get_full_series | grep -e ^$patch\$ | wc -l` -if [ $ret -eq 0 ]; then - die Patch $patch is not in the series +full_series=`get_tmp_file series` +get_full_series $full_series +found_patch= +while read x; do + if [ $x = $patch ]; then + found_patch=$patch + break + fi +done $full_series We have to use a temp file instead of a 'get_full_series | while read x; do ...' because that'd create a subshell, correct? Yes. Also (and probably less importantly) we sometimes need to run grep on the same output (see the creation of TMP_MATCHES below) and it would be a bit wasteful to run get_full_series twice. (The assumption is that it is cheaper to create a temp file than to recompute the value. I have not measured this, though.) I think this is a fair assumption. +if [ -z $found_patch ]; then + TMP_MATCHES=`get_tmp_file series` + grep $patch $full_series $TMP_MATCHES + nr=`wc -l $TMP_MATCHES` + if [ $nr -gt 1 ]; then + echo $patch does not uniquely identify a patch. Did you mean any of these? 2 + sed 's/^/ /' $TMP_MATCHES 2 + rm -f $TMP_MATCHES + exit 1 + elif [ $nr -eq 0 ]; then + rm -f $TMP_MATCHES + die Patch $patch is not in the series + fi + found_patch=`cat $TMP_MATCHES` + rm -f $TMP_MATCHES fi +patch=$found_patch Do we not delete $full_series? Good catch. Will fix in the next version of the series. I'll also rename the variable $TMP_FULL_SERIES to adhere to the apparent coding style. (But I will not fix guilt-patchbomb that uses $dir as a temporary variable.) Ok. Thanks, Jeff. -- *NOTE: This message is ROT-13 encrypted twice for extra protection* -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Pretty print truncate does not work
Pretty format string %(N,[ml]trunc)%s truncates subject to a given length with an appropriate padding. This works for non-ASCII texts when i18n.logOutputEncoding is UTF-8 only (independently of a printed commit message encoding) but does not work when i18n.logOutputEncoding is NOT UTF-8. Following patches are: 1. failing tests 2. Fix patch Alexey Shumkin (2): t4205 (log-pretty-formats): Add failing tests for the case when i18n.logOutputEncoding is set pretty.c: format string with truncate respects logOutputEncoding pretty.c | 7 +- t/t4205-log-pretty-formats.sh | 169 ++ 2 files changed, 174 insertions(+), 2 deletions(-) -- 1.9.2-15 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t4205 (log-pretty-formats): Add failing tests for the case when i18n.logOutputEncoding is set
Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.
There were no breakages as far as were no tests for the case
when both a commit message and logOutputEncoding are not UTF-8.
Add failing tests for that which will be fixed in the next patch.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
t/t4205-log-pretty-formats.sh | 169 ++
1 file changed, 169 insertions(+)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 2a6278b..6791e0d 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -153,6 +153,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting. i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(40)%s
actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+message twoZ
+message oneZ
+add barZ
+$(commit_msg)Z
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting at the nth column' '
git log --pretty=format:%h %|(40)%s actual
# complete the incomplete line at the end
@@ -166,6 +179,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting at the nth column.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%h
%|(40)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+$head1 message twoZ
+$head2 message oneZ
+$head3 add barZ
+$head4 $(commit_msg)Z
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with no padding' '
git log --pretty=format:%(1)%s actual
# complete the incomplete line at the end
@@ -179,6 +205,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting with no padding.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(1)%s
actual
+ # complete the incomplete line at the end
+ echo actual
+ cat EOF | iconv -f utf-8 -t iso8859-1 expected
+message two
+message one
+add bar
+$(commit_msg)
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with trunc' '
git log --pretty=format:%(10,trunc)%s actual
# complete the incomplete line at the end
@@ -192,6 +231,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+message ..
+message ..
+add bar Z
+initial...
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with ltrunc' '
git log --pretty=format:%(10,ltrunc)%s actual
# complete the incomplete line at the end
@@ -205,6 +257,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,ltrunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+..sage two
+..sage one
+add bar Z
+..${sample_utf8_part}lich
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with mtrunc' '
git log --pretty=format:%(10,mtrunc)%s actual
# complete the incomplete line at the end
@@ -218,6 +283,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,mtrunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+mess.. two
+mess.. one
+add bar Z
+init..lich
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'right alignment formatting' '
git log --pretty=format:%(40)%s actual
# complete the incomplete line at the end
@@ -231,6 +309,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'right alignment formatting.
[PATCH 2/2] pretty.c: format string with truncate respects logOutputEncoding
Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.
In 7e77df3 (pretty: two phase conversion for non utf-8 commits, 2013-04-19)
'format_commit_item' function assumes commit message to be in UTF-8.
And that was so until ecaee80 (pretty: --format output should honor
logOutputEncoding, 2013-06-26) where conversion to logOutputEncoding was
added before calling 'format_commit_message'.
Correct this by converting a commit message to UTF-8 first (as it
assumed in 7e77df3 (pretty: two phase conversion for non utf-8 commits,
2013-04-19)). Only after that set 'output_enc' variable to an actual
logOutputEncoding.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
---
pretty.c | 7 +--
t/t4205-log-pretty-formats.sh | 8
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/pretty.c b/pretty.c
index 6e266dd..7eb43c1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1500,16 +1500,19 @@ void format_commit_message(const struct commit *commit,
const struct pretty_print_context *pretty_ctx)
{
struct format_commit_context context;
- const char *output_enc = pretty_ctx-output_encoding;
const char *utf8 = UTF-8;
memset(context, 0, sizeof(context));
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
+ // convert a commit message to UTF-8 first
+ // as far as 'format_commit_item' assumes it in UTF-8
context.message = logmsg_reencode(commit,
context.commit_encoding,
- output_enc);
+ utf8);
+ // then convert to an actual output encoding
+ const char *output_enc = pretty_ctx-output_encoding;
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 6791e0d..7426fe2 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -231,7 +231,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -257,7 +257,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,ltrunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -283,7 +283,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,mtrunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -465,7 +465,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left/right alignment formatting with stealing.
i18n.logOutputEncoding' '
+test_expect_success 'left/right alignment formatting with stealing.
i18n.logOutputEncoding' '
git commit --amend -m short --author long long long l...@me.com
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual
# complete the incomplete line at the end
--
1.9.2-15
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC] Always auto-gc after calling a fast-import transport
Stepan Kasal wrote: From: Johannes Schindelin johannes.schinde...@gmx.de Date: Mon, 9 Apr 2012 13:04:35 -0500 After importing anything with fast-import, we should always let the garbage collector do its job, since the objects are written to disk inefficiently. This brings down an initial import of http://selenic.com/hg from about 230 megabytes to about 14. In the future, we may want to make this configurable on a per-remote basis, or maybe teach fast-import about it in the first place. Actually I tested this patch and it makes no difference. Before and after the patch the repository size is exactly the same (and so is the run-time). That is because after the initial import there are no loose objects, every thing is into one big pack, so `git gc --auto` does nothing. Could anyone on the list try to reproduce the performance problem that triggered this? I'd say this patch is not doing anything in recent versions of Git. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] [PATCH] transport-helper: add trailing --
Johannes Schindelin wrote: Hi, On Thu, 15 May 2014, Stepan Kasal wrote: From: Sverre Rabbelier srabbel...@gmail.com Date: Sat, 28 Aug 2010 20:49:01 -0500 [PT: ensure we add an additional element to the argv array] IIRC we had problems with refs vs file names. Actually I tried to push a file named refs/heads/master and I saw the issue. I'd say it's a very rare issue, and I don't see how it could be triggered with normal files since all the ref names have the full name. That being said I don't think it would hurt. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] pretty.c: format string with truncate respects logOutputEncoding
And I thought I was the only one using this :)
diff --git a/pretty.c b/pretty.c
index 6e266dd..7eb43c1 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1500,16 +1500,19 @@ void format_commit_message(const struct commit
*commit,
const struct pretty_print_context *pretty_ctx)
{
struct format_commit_context context;
- const char *output_enc = pretty_ctx-output_encoding;
const char *utf8 = UTF-8;
memset(context, 0, sizeof(context));
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
+ // convert a commit message to UTF-8 first
+ // as far as 'format_commit_item' assumes it in UTF-8
context.message = logmsg_reencode(commit,
context.commit_encoding,
- output_enc);
+ utf8);
+ // then convert to an actual output encoding
+ const char *output_enc = pretty_ctx-output_encoding;
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
It looks ok except minor issues, use C comment syntax, not C++ and
variable declaration not in the middle of the body.
--
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] commit: allow core.commentChar=auto for character auto selection
core.commentChar starts with '#' as in default but if it's already in
the prepared message, find another one among a small subset. This
should stop surprises because git strips some lines unexpectedly.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
Documentation/config.txt | 3 +++
builtin/commit.c | 36
cache.h | 1 +
config.c | 2 ++
environment.c| 1 +
t/t7502-commit.sh| 25 +
6 files changed, 68 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1932e9b..d5bf4d0 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -544,6 +544,9 @@ core.commentchar::
messages consider a line that begins with this character
commented, and removes them after the editor returns
(default '#').
++
+If set to auto, `git-commit` would select a character that is not
+the beginning character of any line of existing commit messages.
sequence.editor::
Text editor used by `git rebase -i` for editing the rebase instruction
file.
diff --git a/builtin/commit.c b/builtin/commit.c
index 9cfef6c..039b426 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -594,6 +594,40 @@ static char *cut_ident_timestamp_part(char *string)
return ket;
}
+static void adjust_comment_line_char(const struct strbuf *sb)
+{
+ char candidates[] = @!#$%^|:;~;
+ char *candidate;
+ const char *p;
+
+ if (!sb-len)
+ return;
+
+ if (!strchr(candidates, comment_line_char))
+ candidates[0] = comment_line_char;
+ p = sb-buf;
+ candidate = strchr(candidates, *p);
+ if (candidate)
+ *candidate = ' ';
+ for (p = sb-buf; *p; p++) {
+ if ((p[0] == '\n' || p[0] == '\r') p[1]) {
+ candidate = strchr(candidates, p[1]);
+ if (candidate)
+ *candidate = ' ';
+ }
+ }
+
+ if (candidates[0] == comment_line_char)
+ return;
+ for (p = candidates; *p == ' '; p++)
+ ;
+ if (!*p)
+ die(_(the comment character '%c' exists in the commit
message\n
+ Please choose another character for core.commentChar),
+ comment_line_char);
+ comment_line_char = *p;
+}
+
static int prepare_to_commit(const char *index_file, const char *prefix,
struct commit *current_head,
struct wt_status *s,
@@ -748,6 +782,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
if (fwrite(sb.buf, 1, sb.len, s-fp) sb.len)
die_errno(_(could not write commit template));
+ if (auto_comment_line_char)
+ adjust_comment_line_char(sb);
strbuf_release(sb);
/* This checks if committer ident is explicitly given */
diff --git a/cache.h b/cache.h
index 107ac61..646fb81 100644
--- a/cache.h
+++ b/cache.h
@@ -602,6 +602,7 @@ extern int precomposed_unicode;
* that is subject to stripspace.
*/
extern char comment_line_char;
+extern int auto_comment_line_char;
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
diff --git a/config.c b/config.c
index 05d909b..5ec3520 100644
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_core_config(const char *var, const
char *value)
if (!ret) {
if (comment[0] !comment[1])
comment_line_char = comment[0];
+ else if (!strcasecmp(comment, auto))
+ auto_comment_line_char = 1;
else
return error(core.commentChar should only be
one character);
}
diff --git a/environment.c b/environment.c
index 5c4815d..f2de1ee 100644
--- a/environment.c
+++ b/environment.c
@@ -69,6 +69,7 @@ unsigned long pack_size_limit_cfg;
* that is subject to stripspace.
*/
char comment_line_char = '#';
+int auto_comment_line_char;
/* Parallel index stat data preload? */
int core_preload_index = 0;
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 9a3f3a1..5cff300 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -563,4 +563,29 @@ test_expect_success 'commit --status with custom comment
character' '
test_i18ngrep ^; Changes to be committed: .git/COMMIT_EDITMSG
'
+test_expect_success 'switch core.commentchar' '
+ test_commit #foo foo
+ GIT_EDITOR=.git/FAKE_EDITOR git -c core.commentChar=auto commit --amend
+ test_i18ngrep ^@ Changes to be committed: .git/COMMIT_EDITMSG
+'
+
+test_expect_success 'switch core.commentchar but out of options' '
+ cat text \EOF
+# 1
+@ 2
+! 3
+$ 4
+% 5
+^ 6
+ 7
+| 8
+: 9
+; 10
+~ 11
+EOF
+ git commit --amend -F text
+
[PATCH 1/2] config: be strict on core.commentChar
We don't support comment _strings_ (at least not yet). And multi-byte
character encoding could also be misinterpreted.
The test with two commas is deleted because it violates this. It's
added with the patch that introduces core.commentChar in eff80a9
(Allow custom comment char - 2013-01-16). It's not clear to me _why_
that behavior is wanted.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
config.c | 8 ++--
t/t7508-status.sh | 6 --
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/config.c b/config.c
index a30cb5c..05d909b 100644
--- a/config.c
+++ b/config.c
@@ -826,8 +826,12 @@ static int git_default_core_config(const char *var, const
char *value)
if (!strcmp(var, core.commentchar)) {
const char *comment;
int ret = git_config_string(comment, var, value);
- if (!ret)
- comment_line_char = comment[0];
+ if (!ret) {
+ if (comment[0] !comment[1])
+ comment_line_char = comment[0];
+ else
+ return error(core.commentChar should only be
one character);
+ }
return ret;
}
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index c987b5e..98a9990 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1348,12 +1348,6 @@ test_expect_success status (core.commentchar with
submodule summary) '
test_i18ncmp expect output
'
-test_expect_success status (core.commentchar with two chars with submodule
summary) '
- test_config core.commentchar ;;
- git -c status.displayCommentPrefix=true status output
- test_i18ncmp expect output
-'
-
test_expect_success --ignore-submodules=all suppresses submodule summary '
cat expect EOF
On branch master
--
1.9.1.346.ga2b5940
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 01/31] The tests should not fail if guilt.diffstat is set.
Explicitly set guilt.diffstat to its default value. Without this, the 027 test (and possibly others) fail if guilt.diffstat is set to true in ~/.gitconfig. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/scaffold | 1 + 1 file changed, 1 insertion(+) diff --git a/regression/scaffold b/regression/scaffold index 546d8c6..5c8b73e 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -87,6 +87,7 @@ function setup_git_repo # Explicitly set config that the tests rely on. git config log.date default git config log.decorate no + git config guilt.diffstat false } function setup_guilt_repo -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 02/31] Allow guilt delete -f to run from a dir which contains spaces.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-delete | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-delete b/guilt-delete index 3e394f8..967ac10 100755 --- a/guilt-delete +++ b/guilt-delete @@ -49,7 +49,7 @@ series_remove_patch $patch guilt_hook delete $patch -[ ! -z $force ] rm -f $GUILT_DIR/$branch/$patch +[ ! -z $force ] rm -f $GUILT_DIR/$branch/$patch exit 0 -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 00/31] Teach guilt import-commit how to create legal patch names, and more
This is version 3 of the patch series I sent back on 21 Mar 2014. I have addressed all feedback to date, updated the coding style, and added signed-off-by lines from Jeff Sipek for those commits that I have received an explicit approval from him (but only if I have not made any other change to that particular commit). I have added two new patches: * Patch 28/31 fixes coding style issues in t-061.sh. I inserted it before the new patch 29/31 since 29/31 copies t-061.sh as t-062.sh, and by fixing the style issues in the origin it is easier to track the changes. This means that v2 patches 28 and 29 are now numbered 29 and 30. Sorry about that. * Patch 31/31 removes all use of git log -p in the test suite. To see how the patches have evolved, you might find http://www.lysator.liu.se/~ceder/guilt-oslo-2014-v3/ useful. It displays diffs of all the patches between v2 and v3, in pdiffdiff output format. Here is the original blurb for the series, slightly edited: I recently found myself sitting on a train with a computer in front of me. I tried to use guilt import-commit, which seemed to work, but when I tried to guilt push the commits I had just imported I got some errors. It turned out that guilt import-commit had generated invalid patch names. I decided to fix the issue, and write a test case that demonstrated the problem. One thing led to another, and here I am, a few late nights at a hotel and a return trip on the train later, submitting a patch series in 28 parts. Sorry about the number of patches, but this is what happens when you uncover a bug while writing a test case for the bug you uncovered while writing a test case for your original problem. The patch series consists of: - A number of fixes to the test suite. - A number of bug fixes which I hope are non-controversial. Most of the fixes have test cases. - Changed behavior: guilt push when there is nothing more to push now uses exit status 1. This makes it possible to write shell loops such as while guilt push; do make test||break; done. Also, guilt pop when no patches are applied also uses exit status 1. (This aligns guilt push and guilt pop with how hg qpush and hg qpop has worked for several years.) - Changed behavior: by default, guilt no longer changes branch when you push a patch. You need to do git config guilt.reusebranch false to re-enable that. This patch sets the default value of guilt.reusebranch to true; it should in my opinion change to false a year or two after the next release. - The humble beginnings of a coding style guide. A more detailed overview of the patches: 1. Some tests fail if git config guilt.diffstat true is in effect. 2-4. Some commands fail if run from a directory with spaces in its name. 5. guilt new had an overly restrictive argument parser. 6-8. guilt.diffstat could break guilt fold and guilt new. 9-10. The test suite did not test exit status properly. 11. Remove pointless redirections in the test suite. 12-13. guilt header tried to check if a patch existed, but the check was broken. 14-16. Various parts of guilt tried to ensure that patch names were legal git branch names, but failed. 17-20. guilt graph failed when no patch was applied, and when a branch name contained a comma or a quote. 21-23. git config log.decorate short caused guilt import-commit, guilt patchbomb and guilt rebase to fail in various ways. 24. Patches may contain backslashes, but various informative messages from guilt did backslash processing. 25-26. guilt push and guilt pop should fail when there is nothing to do. 28. Fix coding style problems in a test case. 27, 29. These two commits were things I intended to contribute a while back, when contributing the Change git branch when patches are applied change (commit 67d3af63f422). These commits makes that behavior optional, and it defaults to being disabled, so that you can use both Guilt v0.35 (and earlier) and the current Guilt code against the same repo. These commits add some code complexity, and you might want to skip them if you think the current behavior is better. 30. A coding style guide, with Emacs support. 31. Avoid using git log -p. This patch series is also available on http://repo.or.cz/w/guilt/ceder.git in the oslo-2014-v3 branch. If you already have a copy of guilt, you should be able to fetch that branch with something like: git remote add ceder http://repo.or.cz/r/guilt/ceder.git git fetch ceder refs/heads/oslo-2014-v3:refs/remotes/ceder/oslo-2014-v3 A few of the regression/t-*.out files contain non-ASCII characters. I hope they survive the mail transfer; if not, please use the repo above to fetch the commits. Per Cederqvist (31): The tests should not fail if guilt.diffstat is set. Allow guilt delete -f to run from a dir which contains spaces. Added test case for guilt delete -f. Allow
[GUILT v3 04/31] Allow guilt import-commit to run from a dir which contains spaces.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-import-commit | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/guilt-import-commit b/guilt-import-commit index 20dcee2..f14647c 100755 --- a/guilt-import-commit +++ b/guilt-import-commit @@ -23,7 +23,7 @@ if ! must_commit_first; then fi disp About to begin conversion... 2 -disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2 +disp Current head: `git rev-parse \`git_branch\`` 2 for rev in `git rev-list $rhash`; do s=`git log --pretty=oneline -1 $rev | cut -c 42-` @@ -46,7 +46,7 @@ for rev in `git rev-list $rhash`; do do_make_header $rev echo git diff --binary $rev^..$rev - ) $GUILT_DIR/$branch/$fname + ) $GUILT_DIR/$branch/$fname # FIXME: grab the GIT_AUTHOR_DATE from the commit object and set the # timestamp on the patch @@ -68,6 +68,6 @@ for rev in `git rev-list $rhash`; do done disp Done. 2 -disp Current head: `cat $GIT_DIR/refs/heads/\`git_branch\`` 2 +disp Current head: `git rev-parse \`git_branch\`` 2 } -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 06/31] Fix the do_get_patch function.
A patch file consists of:
(1) the description
(2) optional diffstat
(3) the patches
When extracting the patch, we only want part 3. The do_get_patch used
to give us part 2 and part 3. That made for instance this series of
operations fail if guilt.diffstat is true:
$ guilt new empty-1
$ guilt new empty-2
$ guilt pop
Now at empty-1
$ guilt fold empty-2
$ guilt pop
All patches popped.
$ guilt push
Applying patch..empty-1
fatal: unrecognized input
To force apply this patch, use 'guilt push -f'
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt b/guilt
index 8701481..3fc524e 100755
--- a/guilt
+++ b/guilt
@@ -334,7 +334,7 @@ do_get_patch()
{
awk '
BEGIN{}
-/^(diff |---$|--- )/ {patch = 1}
+/^(diff |--- )/ {patch = 1}
patch == 1 {print $0}
END{}
' $1
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 03/31] Added test case for guilt delete -f.
Ensure that the file really is deleted. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/t-026.out | 15 +++ regression/t-026.sh | 5 - 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/regression/t-026.out b/regression/t-026.out index 3b9fb14..be50b48 100644 --- a/regression/t-026.out +++ b/regression/t-026.out @@ -29,3 +29,18 @@ f 7848194fd2e9ee0eb6589482900687d799d60a12 .git/patches/master/series f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +% guilt new delete-me +% guilt pop +All patches popped. +% guilt delete -f delete-me +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7848194fd2e9ee0eb6589482900687d799d60a12 .git/patches/master/series +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status diff --git a/regression/t-026.sh b/regression/t-026.sh index 0ccdf85..e25d0df 100755 --- a/regression/t-026.sh +++ b/regression/t-026.sh @@ -20,4 +20,7 @@ cmd guilt delete add cmd list_files -# FIXME: test delete -f +cmd guilt new delete-me +cmd guilt pop +cmd guilt delete -f delete-me +cmd list_files -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 05/31] guilt new: Accept more than 4 arguments.
The argument parser arbitrarily refused to accept more than 4
arguments. That made it impossible to run guilt new -f -s -m msg
patch. Removed the checks for the number of arguments from the
guilt new parser -- the other checks that are already there are
enough to catch all errors.
Give a better error message if -m isn't followed by a message
argument.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-new | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/guilt-new b/guilt-new
index bb68924..9528438 100755
--- a/guilt-new
+++ b/guilt-new
@@ -11,10 +11,6 @@ fi
_main() {
-if [ $# -lt 1 ] || [ $# -gt 4 ]; then
- usage
-fi
-
while [ $# -gt 0 ] ; do
case $1 in
-f)
@@ -31,6 +27,9 @@ while [ $# -gt 0 ] ; do
fi
;;
-m)
+ if [ $# -eq 1 ]; then
+ usage
+ fi
msg=$2
shift
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 07/31] Added test cases for guilt fold.
Test that we can combine any combination of patches with empty and non-empty messages, both with and without guilt.diffstat. (All patches are empty.) Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/t-035.out | 467 +++ regression/t-035.sh | 61 +++ 2 files changed, 528 insertions(+) create mode 100644 regression/t-035.out create mode 100755 regression/t-035.sh diff --git a/regression/t-035.out b/regression/t-035.out new file mode 100644 index 000..cc16fb4 --- /dev/null +++ b/regression/t-035.out @@ -0,0 +1,467 @@ +% setup_repo +% git config guilt.diffstat true +%% empty + empty (diffstat=true) +% guilt new empty-1 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% guilt new empty-2 +% guilt pop +Now at empty-1. +% guilt push +Applying patch..empty-2 +Patch applied. +% guilt pop +Now at empty-1. +% guilt fold empty-2 +% guilt pop +All patches popped. +% guilt push +Applying patch..empty-1 +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 4ea806e306f0228a8ef41f186035e7b04097f1f2 .git/patches/master/status +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty-1 +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d28d87b88c1e24d637e390dc3603cfa7c1715711 .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r bde3d337af70f36836ad606c800d194006f883b3 .git/refs/patches/master/empty-1 +% guilt pop +All patches popped. +% guilt delete -f empty-1 +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +%% empty + nonempty (diffstat=true) +% guilt new empty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% guilt new -f -s -m A commit message. nonempty +% guilt pop +Now at empty. +% guilt push +Applying patch..nonempty +Patch applied. +% guilt pop +Now at empty. +% guilt fold nonempty +% guilt pop +All patches popped. +% guilt push +Applying patch..empty +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 15aab0fd8b937eb3bb01841693f35dcb75da2faf .git/patches/master/status +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/empty~ +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/nonempty~ +f 683678040eef9334d6329e00d5b9babda3e65b57 .git/patches/master/empty +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f a26a22287b500a2a372e42c2bab03599bbe37cdf .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +r 4eedaa32894fc07af3298d8c1178052942a3ca6a .git/refs/patches/master/empty +% guilt pop +All patches popped. +% guilt delete -f empty +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/empty~ +f 51fcfcf16db2903f19ab4a4a3caacd297ea9f6cd .git/patches/master/nonempty~ +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-1~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty-2~ +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +%% nonempty + empty (diffstat=true) +% guilt new -f -s -m A commit
[GUILT v3 08/31] Added more test cases for guilt new: empty patches.
Test that empty patches are handled correctly, both with and without the guilt.diffstat configuration option. Signed-off-by: Per Cederqvist ced...@opera.com --- regression/t-020.out | 269 +++ regression/t-020.sh | 60 2 files changed, 329 insertions(+) diff --git a/regression/t-020.out b/regression/t-020.out index af45734..42433dc 100644 --- a/regression/t-020.out +++ b/regression/t-020.out @@ -1128,3 +1128,272 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +% guilt new empty.patch +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat true +% guilt refresh +Patch empty.patch refreshed +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat false +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7ad87a0bdb8cf0a57cfc384633edabbb9c2bfa1b .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r 8ed27228b117c0c88abf3d586bcc43c68e975cea .git/refs/patches/master/empty.patch +% git log -p +commit 8ed27228b117c0c88abf3d586bcc43c68e975cea +Author: Per Cederqvist ce...@lysator.liu.se +Date: Mon Jan 1 00:00:00 2007 + + +Fix a bug. + +This commit fixes a serious bug. + +FIXME: +- add a test case +- track down the bug +- actually fix it + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config
[GUILT v3 09/31] Test suite: properly check the exit status of commands.
The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)
Updated t-032.sh, which used shouldfail instead of cmd in one
place. (The comment in the script makes it clear that the command is
expected to succeed.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/scaffold | 17 +++--
regression/t-032.sh | 2 +-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
function cmd
{
echo % $@
- $@ 21 | replace_path return 0
- return 1
+ (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+ exit $rv
+ )
+ return $?
}
# usage: shouldfail cmd..
function shouldfail
{
echo % $@
- (
- $@ 21 || return 0
- return 1
- ) | replace_path
+ ! (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+ exit $rv
+ )
return $?
}
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
cmd guilt import -P foo2 foo
# ok
-shouldfail guilt import foo
+cmd guilt import foo
# duplicate patch name (implicit)
shouldfail guilt import foo
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 11/31] test suite: remove pointless redirection.
The shouldfail function already redirects stderr to stdout, so there is no need to do the same in t-028.sh and t-021.sh. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/t-021.sh | 2 +- regression/t-025.sh | 2 +- regression/t-028.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/regression/t-021.sh b/regression/t-021.sh index 6337d7b..614e870 100755 --- a/regression/t-021.sh +++ b/regression/t-021.sh @@ -61,7 +61,7 @@ for n in `_seq -2 $npatches`; do if [ $n -gt 0 ]; then cmd guilt pop -n $n else - shouldfail guilt pop -n $n 21 + shouldfail guilt pop -n $n fi cmd list_files diff --git a/regression/t-025.sh b/regression/t-025.sh index 3824608..985fed4 100755 --- a/regression/t-025.sh +++ b/regression/t-025.sh @@ -44,7 +44,7 @@ shouldfail guilt new white space cmd list_files for pname in prepend mode /abc ./blah ../blah abc/./blah abc/../blah abc/. abc/.. abc/ ; do - shouldfail guilt new $pname 21 + shouldfail guilt new $pname cmd list_files done diff --git a/regression/t-028.sh b/regression/t-028.sh index 8480100..88e9adb 100755 --- a/regression/t-028.sh +++ b/regression/t-028.sh @@ -29,6 +29,6 @@ guilt series | while read n; do cmd guilt header $n done -shouldfail guilt header non-existant 21 +shouldfail guilt header non-existant # FIXME: how do we check that -e works? -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 10/31] Run test_failed if the exit status of a test script is bad.
There were two problems with the old code: - Since set -e is in effect (that is set in scaffold) the run-test script exited immediately if a t-*.sh script failed. This is not nice, as we want the error report that test_failed prints. - The code ran cd - between running the t-*.sh script and checking the exit status, so the exit status was lost. (Actually, the exit status was saved in $ERR, but nothing ever looked at $ERR.) Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/run-tests | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/regression/run-tests b/regression/run-tests index a10e796..8e0af9f 100755 --- a/regression/run-tests +++ b/regression/run-tests @@ -55,11 +55,15 @@ function run_test # run the test cd $REPODIR /dev/null - $REG_DIR/t-$1.sh 21 $LOGFILE - ERR=$? + if $REG_DIR/t-$1.sh 21 $LOGFILE; then + ERR=false + else + ERR=true + fi + cd - /dev/null - [ $? -ne 0 ] test_failed + $ERR test_failed diff -u t-$1.out $LOGFILE || test_failed echo done. -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/2] config: be strict on core.commentChar
Nguyễn Thái Ngọc Duy wrote:
We don't support comment _strings_ (at least not yet). And multi-byte
character encoding could also be misinterpreted.
The test with two commas is deleted because it violates this. It's
added with the patch that introduces core.commentChar in eff80a9
(Allow custom comment char - 2013-01-16). It's not clear to me _why_
that behavior is wanted.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
config.c | 8 ++--
t/t7508-status.sh | 6 --
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/config.c b/config.c
index a30cb5c..05d909b 100644
--- a/config.c
+++ b/config.c
@@ -826,8 +826,12 @@ static int git_default_core_config(const char *var,
const char *value)
if (!strcmp(var, core.commentchar)) {
const char *comment;
int ret = git_config_string(comment, var, value);
- if (!ret)
- comment_line_char = comment[0];
+ if (!ret) {
+ if (comment[0] !comment[1])
+ comment_line_char = comment[0];
+ else
+ return error(core.commentChar should only be
one character);
+ }
Small nit:
if (ret)
return ret;
if (comment[0] !comment[1])
comment_line_char = comment[0];
else
return error(core.commentChar should only be one character);
--
Felipe Contreras--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 14/31] Use git check-ref-format to validate patch names.
The valid_patchname now lets git check-ref-format do its job instead
of trying (and failing) to implement the same rules. See
git-check-ref-format(1) for a list of the rules.
Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.
Added testcases that breaks most of the rules in that man-page.
Git version 1.8.5 no longer allows the single character @ as a
branch name. Guilt always rejects that name, for increased
compatibility.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt| 21 ++-
guilt-fork | 2 +-
guilt-import | 2 +-
guilt-new| 2 +-
regression/t-025.out | 426 +--
regression/t-025.sh | 12 +-
regression/t-032.out | 4 +-
7 files changed, 446 insertions(+), 23 deletions(-)
diff --git a/guilt b/guilt
index 3fc524e..23cc2da 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,19 @@ fi
# usage: valid_patchname patchname
valid_patchname()
{
- case $1 in
- /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
- return 1;;
- *:*)
- return 1;;
- *)
- return 0;;
- esac
+ if git check-ref-format --allow-onelevel $1; then
+ # Starting with Git version 1.8.5, a branch cannot be
+ # the single character @. Make sure guilt rejects
+ # that name even if we are currently using an older
+ # version of Git. This ensures that the test suite
+ # runs fine using any version of Git.
+ if [ $1 = @ ]; then
+ return 1
+ fi
+ return 0
+ else
+ return 1
+ fi
}
get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
fi
if ! valid_patchname $newpatch; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
if [ -e $GUILT_DIR/$branch/$newpatch ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e $GUILT_DIR/$branch/$newname ]; then
fi
if ! valid_patchname $newname; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
if ! valid_patchname $patch; then
disp Patchname is invalid. 2
- die it cannot begin with '/', './' or '../', or contain /./, /../, or
whitespace
+ die It must follow the rules in git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new white space
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new /abc
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new ./blah
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new ../blah
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -283,7 +283,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8
[GUILT v3 12/31] guilt header: more robust header selection.
If you run something like guilt header '.*' the command would crash, because the grep comand that tries to ensure that the patch exist would detect a match, but the later code expected the match to be exact. Fixed by comparing exact strings. And as a creeping feature guilt header will now try to use the supplied patch name as an unachored regexp if no exact match was found. If the regexp yields a unique match, it is used; if more than one patch matches, the names of all patches are listed and the command fails. (Exercise left to the reader: generalized this so that guilt push also accepts a unique regular expression.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-header | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/guilt-header b/guilt-header index 41e00cc..c3d24f9 100755 --- a/guilt-header +++ b/guilt-header @@ -45,10 +45,33 @@ esac [ -z $patch ] die No patches applied. # check that patch exists in the series -ret=`get_full_series | grep -e ^$patch\$ | wc -l` -if [ $ret -eq 0 ]; then - die Patch $patch is not in the series +TMP_FULL_SERIES=`get_tmp_file series` +get_full_series $TMP_FULL_SERIES +found_patch= +while read x; do + if [ $x = $patch ]; then + found_patch=$patch + break + fi +done $TMP_FULL_SERIES +if [ -z $found_patch ]; then + TMP_MATCHES=`get_tmp_file series` + grep $patch $TMP_FULL_SERIES $TMP_MATCHES + nr=`wc -l $TMP_MATCHES` + if [ $nr -gt 1 ]; then + echo $patch does not uniquely identify a patch. Did you mean any of these? 2 + sed 's/^/ /' $TMP_MATCHES 2 + rm -f $TMP_MATCHES $TMP_FULL_SERIES + exit 1 + elif [ $nr -eq 0 ]; then + rm -f $TMP_MATCHES $TMP_FULL_SERIES + die Patch $patch is not in the series + fi + found_patch=`cat $TMP_MATCHES` + rm -f $TMP_MATCHES fi +rm -f $TMP_FULL_SERIES +patch=$found_patch # FIXME: warn if we're editing an applied patch -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 15/31] Produce legal patch names in guilt-import-commit.
Try harder to create patch names that adhere to the rules in
git-check-ref-format(1) when deriving a patch name from the commit
message. Verify that the derived name using git check-ref-format,
and as a final fallback simply use the patch name x (to ensure that
the code is future-proof in case new rules are added in the future).
Always append a .patch suffix to the patch name.
Added test cases.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-import-commit | 20 +-
regression/t-034.out | 567 +++
regression/t-034.sh | 71 +++
3 files changed, 656 insertions(+), 2 deletions(-)
create mode 100644 regression/t-034.out
create mode 100755 regression/t-034.sh
diff --git a/guilt-import-commit b/guilt-import-commit
index f14647c..6260c56 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -28,19 +28,35 @@ disp Current head: `git rev-parse \`git_branch\`` 2
for rev in `git rev-list $rhash`; do
s=`git log --pretty=oneline -1 $rev | cut -c 42-`
+ # Try to convert the first line of the commit message to a
+ # valid patch name.
fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
- -e 's/\?/-/g' | tr A-Z a-z`
+ -e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
+ -e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
+
+ if ! valid_patchname $fname; then
+ # Try harder to make it a legal commit name by
+ # removing all but a few safe characters.
+ fname=`echo $fname|tr -d -c _a-zA-Z0-9---/\\n`
+ fi
+ if ! valid_patchname $fname; then
+ # If we failed to derive a legal patch name, use the
+ # name x. (If this happens, we likely have to
+ # append a suffix to make the name unique.)
+ fname=x
+ fi
disp Converting `echo $rev | cut -c 1-8` as $fname
mangle_prefix=1
fname_base=$fname
- while [ -f $GUILT_DIR/$branch/$fname ]; do
+ while [ -f $GUILT_DIR/$branch/$fname.patch ]; do
fname=$fname_base-$mangle_prefix
mangle_prefix=`expr $mangle_prefix + 1`
disp Patch under that name exists...trying '$fname'
done
+ fname=$fname.patch
(
do_make_header $rev
diff --git a/regression/t-034.out b/regression/t-034.out
new file mode 100644
index 000..7bc9459
--- /dev/null
+++ b/regression/t-034.out
@@ -0,0 +1,567 @@
+% setup_git_repo
+% git tag base
+% create_commit a The sequence /. is forbidden.
+[master eebb76e] The sequence /. is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+ create mode 100644 a
+% create_commit a The sequence .lock/ is forbidden.
+[master 45e81b5] The sequence .lock/ is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a A/component/may/not/end/in/foo.lock
+[master bbf3f59] A/component/may/not/end/in/foo.lock
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Two consecutive dots (..) is forbidden.
+[master 1535e67] Two consecutive dots (..) is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Check/multiple/../dots/./foo..patch
+[master 48eb60c] Check/multiple/../dots/./foo..patch
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Space is forbidden.
+[master 10dea83] Space is forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Tilde~is~forbidden.
+[master 70a83b7] Tilde~is~forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Caret^is^forbidden.
+[master ee6ef2c] Caret^is^forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Colon:is:forbidden.
+[master c077fe2] Colon:is:forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Delisforbidden.
+[master 589ee30] Delisforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% git branch some-branch
+% git tag some-tag
+% create_commit a Ctrlisforbidden.
+[master e63cdde] Ctrlisforbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a CR
is
also
forbidden.
+[master 21ad093] CR
is
also
forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Question-mark?is?forbidden.
+[master be2fa9b] Question-mark?is?forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+% create_commit a Asterisk*is*forbidden.
+[master af7b50f] Asterisk*is*forbidden.
+ Author: Author Name author@email
+ 1 file changed, 1 insertion(+)
+%
[GUILT v3 16/31] Fix backslash handling when creating names of imported patches.
The 'echo $s' construct sometimes processes escape sequences. (This
happens, for instance, under Ubuntu 14.04 when /bin/sh is actually
dash.) We don't want that to happen when we are importing commits, so
use 'printf %s $s' instead.
(The -E option of bash that explicitly disables backslash expansion is
not portable; it is not supported by dash.)
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-import-commit | 2 +-
regression/t-034.out | 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/guilt-import-commit b/guilt-import-commit
index 6260c56..45f2404 100755
--- a/guilt-import-commit
+++ b/guilt-import-commit
@@ -30,7 +30,7 @@ for rev in `git rev-list $rhash`; do
# Try to convert the first line of the commit message to a
# valid patch name.
- fname=`echo $s | sed -e s//and/g -e s/[ :]/_/g -e s,[/\\],-,g \
+ fname=`printf %s $s | sed -e s//and/g -e s/[ :]/_/g -e
s,[/\\],-,g \
-e s/['\\[{}]//g -e 's/]//g' -e 's/\*/-/g' \
-e 's/\?/-/g' -e 's/\.\.\.*/./g' -e 's/^\.//' \
-e 's/\.patch$//' -e 's/\.$//' | tr A-Z a-z`
diff --git a/regression/t-034.out b/regression/t-034.out
index 7bc9459..bda4399 100644
--- a/regression/t-034.out
+++ b/regression/t-034.out
@@ -236,7 +236,7 @@ Date: Mon Jan 1 00:00:00 2007 +
About to begin conversion...
Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541
Converting 2a8b1889 as can-have-embedded-single-slashes
-Converting 0a46f8fa as backslash-isorbidden
+Converting 0a46f8fa as backslash-is-forbidden
Converting aedb74fd as x
Converting 30187ed0 as cannot@have@the@sequence@at-brace
Converting 106e8e5a as cannot_end_in_
@@ -300,7 +300,7 @@ Applying patch..cannot@have@the@sequence@at-brace.patch
Patch applied.
Applying patch..x.patch
Patch applied.
-Applying patch..backslash-isorbidden.patch
+Applying patch..backslash-is-forbidden.patch
Patch applied.
Applying patch..can-have-embedded-single-slashes.patch
Patch applied.
@@ -311,7 +311,7 @@ Date: Mon Jan 1 00:00:00 2007 +
Can/have/embedded/single/slashes
-commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-isorbidden.patch)
+commit 7c3ffa4f940c862e9f11f5d4a5ae421f7a8d3141
(refs/patches/master/backslash-is-forbidden.patch)
Author: Author Name author@email
Date: Mon Jan 1 00:00:00 2007 +
@@ -518,8 +518,6 @@ d .git/patches/master
d .git/refs/patches
d .git/refs/patches/master
f 06beca7069b9e576bd431f65d13862ed5d3e2a0f
.git/patches/master/ctrlisforbidden.patch
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/series
-f 08267ec6783ea9d1adae55b275198f7594764ed0 .git/patches/master/status
f 09b7e9be44ae5ec3a4bb30f5ee9d4ebc2c042f64
.git/patches/master/two_consecutive_dots_(.)_is_forbidden.patch
f 0b971c9a17aeca2319c93d700ffd98acc2a93451
.git/patches/master/question-mark-is-forbidden.patch
f 2b8392f63d61efc12add554555adae30883993cc
.git/patches/master/cannot-end-in-slash-.patch
@@ -529,7 +527,7 @@ f 34e07c584032df137f19bdb66d93f316f00a5ac8
.git/patches/master/tildeisforbidden
f 49bab499826b63deb2bd704629d60c7268c57aee
.git/patches/master/the_sequence_-._is_forbidden.patch
f 5bcddb8ccb6e6e5e8a61e9e56cb2e0f70cbab2f5
.git/patches/master/cannot@have@the@sequence@at-brace.patch
f 637b982fe14a240de181ae63226b27e0c406b3dc
.git/patches/master/asterisk-is-forbidden.patch
-f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-isorbidden.patch
+f 698f8a7d41a64e3b6be1a3eba86574078b22a5f3
.git/patches/master/backslash-is-forbidden.patch
f 7b103c3c7ae298cd2334f6f49da48bae1424f77b
.git/patches/master/crisalsoforbidden.patch
f 9b810b8c63779c51d2e7f61ab59cd49835041563 .git/patches/master/x.patch
f a22958d9ae9976fd7b2b5a9d0bcd44bf7ad9b08a
.git/patches/master/caretisforbidden.patch
@@ -537,6 +535,8 @@ f ab325bf5a432937fc6f231d3e8a773a62d53952b
.git/patches/master/multiple-slashes
f cb9cffbd4465bddee266c20ccebd14eb687eaa89
.git/patches/master/delisforbidden.patch
f d0885a1a1fdee0fd1e4fedce3f7acd3100540bc4
.git/patches/master/openbracketisforbidden.patch
f d2903523fb66a346596eabbdd1bda4e52b266440
.git/patches/master/check-multiple-.-dots-.-foo.patch
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/series
+f da90de1c84138194524994e0bc3bc4ca8189c999 .git/patches/master/status
f dfc11f76394059909671af036598c5fbe33001ba
.git/patches/master/space_is_forbidden.patch
f e47474c52d6c893f36d0457f885a6dd1267742bb
.git/patches/master/colon_is_forbidden.patch
f e7a5f8912592d9891e6159f5827c8b4f372cc406
.git/patches/master/the_sequence_.lock-_is_forbidden.patch
@@ -548,7 +548,7 @@ r 1626a11d979a1e9e775c766484172212277153df
.git/refs/patches/master/asterisk-is
r 3a0d5ccef0359004fcaa9cee98fbd6a2c4432e74
.git/refs/patches/master/tildeisforbidden.patch
r 434e07cacdd8e7eb4723e67cb2d100b3a4121a3a
[GUILT v3 18/31] guilt-graph: Handle commas in branch names.
This fix relies on the fact that git branch names can not contain :.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-graph | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guilt-graph b/guilt-graph
index 56d0e77..924a63e 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -51,7 +51,7 @@ safebranch=`echo $branch|sed 's%/%/%g'`
while [ $current != $base ]; do
pname=`git show-ref | sed -n -e
/^$current refs\/patches\/$safebranch/ {
- s,^$current refs/patches/$branch/,,
+ s:^$current refs/patches/$branch/::
p
q
}`
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 17/31] guilt graph no longer loops when no patches are applied.
Give an error message if no patches are applied. Added a test case
that never terminates unless this fix is applied.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-graph | 9 +++--
regression/t-033.out | 3 +++
regression/t-033.sh | 13 +
3 files changed, 23 insertions(+), 2 deletions(-)
create mode 100644 regression/t-033.out
create mode 100755 regression/t-033.sh
diff --git a/guilt-graph b/guilt-graph
index b3469dc..56d0e77 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -17,8 +17,13 @@ fi
patchname=$1
-bottom=`git rev-parse refs/patches/$branch/$(head_n 1 $applied)`
-base=`git rev-parse $bottom^`
+bottompatch=$(head_n 1 $applied)
+if [ -z $bottompatch ]; then
+ echo No patch applied. 2
+ exit 1
+fi
+
+base=`git rev-parse refs/patches/${branch}/${bottompatch}^`
if [ -z $patchname ]; then
top=`git rev-parse HEAD`
diff --git a/regression/t-033.out b/regression/t-033.out
new file mode 100644
index 000..76613f9
--- /dev/null
+++ b/regression/t-033.out
@@ -0,0 +1,3 @@
+% setup_repo
+% guilt graph
+No patch applied.
diff --git a/regression/t-033.sh b/regression/t-033.sh
new file mode 100755
index 000..a3a8981
--- /dev/null
+++ b/regression/t-033.sh
@@ -0,0 +1,13 @@
+#!/bin/bash
+#
+# Test the graph code
+#
+
+source $REG_DIR/scaffold
+
+cmd setup_repo
+
+# Check that guilt graph gives a proper No patch applied error
+# message when no patches are applied. (An older version of guilt
+# used to enter an endless loop in this situation.)
+shouldfail guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 19/31] Check that guilt graph works when working on a branch with a comma.
git branch names can contain commas. Check that guilt graph works
even in that case.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
regression/t-033.out | 65
regression/t-033.sh | 39 +++
2 files changed, 104 insertions(+)
diff --git a/regression/t-033.out b/regression/t-033.out
index 76613f9..3d1c61f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -1,3 +1,68 @@
% setup_repo
% guilt graph
No patch applied.
+%% Testing branch a,graph
+% git checkout -b a,graph master
+Switched to a new branch 'a,graph'
+% guilt init
+% guilt new a.patch
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 95275d7c05c6a6176d3941374115b91272877f6c
+ 95275d7c05c6a6176d3941374115b91272877f6c [label=a.patch]
+}
+% git add file.txt
+% guilt refresh
+Patch a.patch refreshed
+% guilt pop
+All patches popped.
+% guilt push
+Applying patch..a.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+%% Adding an unrelated file in a new patch. No deps.
+% guilt new b.patch
+% git add file2.txt
+% guilt refresh
+Patch b.patch refreshed
+% guilt pop
+Now at a.patch.
+% guilt push
+Applying patch..b.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+}
+%% Changing a file already changed in the first patch adds a dependency.
+% guilt new c.patch
+% git add file.txt
+% guilt refresh
+Patch c.patch refreshed
+% guilt pop
+Now at b.patch.
+% guilt push
+Applying patch..c.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ 891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+ 891bc14b5603474c9743fd04f3da888644413dc5 -
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index a3a8981..fac081e 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -3,6 +3,13 @@
# Test the graph code
#
+function fixup_time_info
+{
+ cmd guilt pop
+ touch -a -m -t $TOUCH_DATE .git/patches/a,graph/$1
+ cmd guilt push
+}
+
source $REG_DIR/scaffold
cmd setup_repo
@@ -11,3 +18,35 @@ cmd setup_repo
# message when no patches are applied. (An older version of guilt
# used to enter an endless loop in this situation.)
shouldfail guilt graph
+
+echo %% Testing branch a,graph
+cmd git checkout -b a,graph master
+
+cmd guilt init
+
+cmd guilt new a.patch
+
+fixup_time_info a.patch
+cmd guilt graph
+
+cmd echo a file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a.patch
+cmd guilt graph
+
+echo %% Adding an unrelated file in a new patch. No deps.
+cmd guilt new b.patch
+cmd echo b file2.txt
+cmd git add file2.txt
+cmd guilt refresh
+fixup_time_info b.patch
+cmd guilt graph
+
+echo %% Changing a file already changed in the first patch adds a dependency.
+cmd guilt new c.patch
+cmd echo c file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info c.patch
+cmd guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 21/31] The log.decorate setting should not influence import-commit.
Use --no-decorate in the call to git log that tries to read the commit message to produce patch names. Otherwise, if the user has set log.decorate to short or full, the patch name will be less useful. Modify the t-034.sh test case to demonstrate that this is needed. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-import-commit | 2 +- regression/t-034.out | 2 ++ regression/t-034.sh | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/guilt-import-commit b/guilt-import-commit index 45f2404..1da7c8e 100755 --- a/guilt-import-commit +++ b/guilt-import-commit @@ -26,7 +26,7 @@ disp About to begin conversion... 2 disp Current head: `git rev-parse \`git_branch\`` 2 for rev in `git rev-list $rhash`; do - s=`git log --pretty=oneline -1 $rev | cut -c 42-` + s=`git log --no-decorate --pretty=oneline -1 $rev | cut -c 42-` # Try to convert the first line of the commit message to a # valid patch name. diff --git a/regression/t-034.out b/regression/t-034.out index bda4399..5d81bd4 100644 --- a/regression/t-034.out +++ b/regression/t-034.out @@ -232,6 +232,7 @@ Date: Mon Jan 1 00:00:00 2007 + Signed-off-by: Commiter Name commiter@email % guilt init +% git config log.decorate short % guilt import-commit base..HEAD About to begin conversion... Current head: 2a8b1889aa5066193bac978e6bf5073ffcfa6541 @@ -259,6 +260,7 @@ Converting 45e81b51 as the_sequence_.lock-_is_forbidden Converting eebb76e9 as the_sequence_-._is_forbidden Done. Current head: d4850419ccc1146c7169f500725ce504b9774ed0 +% git config log.decorate no % guilt push -a Applying patch..the_sequence_-._is_forbidden.patch Patch applied. diff --git a/regression/t-034.sh b/regression/t-034.sh index f41f958..648d009 100755 --- a/regression/t-034.sh +++ b/regression/t-034.sh @@ -57,7 +57,9 @@ cmd git log # Import all the commits to guilt. cmd guilt init +cmd git config log.decorate short cmd guilt import-commit base..HEAD +cmd git config log.decorate no for patch in .git/patches/master/*.patch; do touch -a -m -t $TOUCH_DATE $patch -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 20/31] guilt graph: Handle patch names containing quotes.
Quote quotes with a backslash in the guilt graph output. Otherwise,
the dot file could contain syntax errors.
Added a test case.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-graph | 2 ++
regression/t-033.out | 22 ++
regression/t-033.sh | 9 +
3 files changed, 33 insertions(+)
diff --git a/guilt-graph b/guilt-graph
index 924a63e..0857e0d 100755
--- a/guilt-graph
+++ b/guilt-graph
@@ -57,6 +57,8 @@ while [ $current != $base ]; do
}`
[ -z $pname ] pname=?
+ pname=`printf \%s\ \$pname\ | sed 's/\/\/g'`
+
disp # checking rev $current
disp \$current\ [label=\$pname\]
diff --git a/regression/t-033.out b/regression/t-033.out
index 3d1c61f..c120d4f 100644
--- a/regression/t-033.out
+++ b/regression/t-033.out
@@ -66,3 +66,25 @@ digraph G {
ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
891bc14b5603474c9743fd04f3da888644413dc5 -
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
}
+% guilt new a-betterquicker'-patch.patch
+% git add file.txt
+% guilt refresh
+Patch a-betterquicker'-patch.patch refreshed
+% guilt pop
+Now at c.patch.
+% guilt push
+Applying patch..a-betterquicker'-patch.patch
+Patch applied.
+% guilt graph
+digraph G {
+# checking rev bc7df666a646739eaf559af23cab72f2bfd01f0e
+ bc7df666a646739eaf559af23cab72f2bfd01f0e
[label=a-\betterquicker'-patch.patch]
+# checking rev 891bc14b5603474c9743fd04f3da888644413dc5
+ 891bc14b5603474c9743fd04f3da888644413dc5 [label=c.patch]
+ bc7df666a646739eaf559af23cab72f2bfd01f0e -
891bc14b5603474c9743fd04f3da888644413dc5; // ?
+# checking rev c7014443c33d2b0237293687ceb9cbd38313df65
+ c7014443c33d2b0237293687ceb9cbd38313df65 [label=b.patch]
+# checking rev ff2775f8d1dc753f635830adcc3a067e0b681e2d
+ ff2775f8d1dc753f635830adcc3a067e0b681e2d [label=a.patch]
+ 891bc14b5603474c9743fd04f3da888644413dc5 -
ff2775f8d1dc753f635830adcc3a067e0b681e2d; // ?
+}
diff --git a/regression/t-033.sh b/regression/t-033.sh
index fac081e..9fe1827 100755
--- a/regression/t-033.sh
+++ b/regression/t-033.sh
@@ -50,3 +50,12 @@ cmd git add file.txt
cmd guilt refresh
fixup_time_info c.patch
cmd guilt graph
+
+# A patch name that contains funky characters, including unbalanced
+# quotes.
+cmd guilt new a-\betterquicker'-patch.patch
+cmd echo d file.txt
+cmd git add file.txt
+cmd guilt refresh
+fixup_time_info a-\betterquicker'-patch.patch
+cmd guilt graph
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 24/31] disp no longer processes backslashes.
Only one invocation of disp or _disp actually needed backslash
processing. In quite a few instances, it was wrong to do backslash
processing, as the message contained data derived from the user.
Created the new function disp_e that should be used when backslash
processing is required, and changed disp and _disp to use printf
code %s instead of %b.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/guilt b/guilt
index 23cc2da..9947acc 100755
--- a/guilt
+++ b/guilt
@@ -36,15 +36,24 @@ usage()
exit 1
}
-# echo -n is a bashism, use printf instead
+# Print arguments, but no trailing newline.
+# (echo -n is a bashism, use printf instead)
_disp()
{
- printf %b $*
+ printf %s $*
}
-# echo -e is a bashism, use printf instead
+# Print arguments.
+# (echo -E is a bashism, use printf instead)
disp()
{
+ printf %s\n $*
+}
+
+# Print arguments, processing backslash sequences.
+# (echo -e is a bashism, use printf instead)
+disp_e()
+{
printf %b\n $*
}
@@ -117,7 +126,7 @@ else
disp
disp Example:
- disp \tguilt push
+ disp_e \tguilt push
# now, let's exit
exit 1
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 23/31] The log.decorate setting should not influence guilt rebase.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-rebase | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-rebase b/guilt-rebase index fd28e48..a1714a0 100755 --- a/guilt-rebase +++ b/guilt-rebase @@ -66,7 +66,7 @@ pop_all_patches git merge --no-commit $upstream /dev/null 2 /dev/null disp -log=`git log -1 --pretty=oneline` +log=`git log -1 --no-decorate --pretty=oneline` disp HEAD is now at `echo $log | cut -c 1-7`... `echo $log | cut -c 41-` # -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 22/31] The log.decorate setting should not influence patchbomb.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-patchbomb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guilt-patchbomb b/guilt-patchbomb index 1231418..164b10c 100755 --- a/guilt-patchbomb +++ b/guilt-patchbomb @@ -47,7 +47,7 @@ if [ $? -ne 0 ]; then fi # display the list of commits to be sent as patches -git log --pretty=oneline $r | cut -c 1-8,41- | $pager +git log --no-decorate --pretty=oneline $r | cut -c 1-8,41- | $pager _disp Are these what you want to send? [Y/n] read n -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 25/31] guilt push now fails when there are no more patches to push.
This makes it easier to script operations on the entire queue, for
example run the test suite on each patch in the queue:
guilt pop -a;while guilt push; do make test||break; done
This brings guilt push in line with the push operation in Mercurial
Queues (hg qpush), which fails when there are no patches to apply.
Updated the test suite.
guilt push -a still does not fail. (It successfully manages to
ensure that all patches are pushed, even if it did not have to do
anything to make it so.)
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt-push | 19 ++-
regression/t-020.out | 89
regression/t-020.sh | 13 +++-
3 files changed, 113 insertions(+), 8 deletions(-)
diff --git a/guilt-push b/guilt-push
index 67687e7..39c125e 100755
--- a/guilt-push
+++ b/guilt-push
@@ -56,6 +56,12 @@ fi
patch=$1
[ ! -z $all ] patch=-a
+# Treat guilt push as guilt push -n 1.
+if [ -z $patch ]; then
+ patch=1
+ num=t
+fi
+
if [ $patch = -a ]; then
# we are supposed to push all patches, get the last one out of
# series
@@ -65,7 +71,7 @@ if [ $patch = -a ]; then
die There are no patches to push.
fi
elif [ ! -z $num ]; then
- # we are supposed to pop a set number of patches
+ # we are supposed to push a set number of patches
[ $patch -lt 0 ] die Invalid number of patches to push.
@@ -78,11 +84,6 @@ elif [ ! -z $num ]; then
# clamp to minimum
[ $tidx -lt $eidx ] eidx=$tidx
-elif [ -z $patch ]; then
- # we are supposed to push only the next patch onto the stack
-
- eidx=`wc -l $applied`
- eidx=`expr $eidx + 1`
else
# we're supposed to push only up to a patch, make sure the patch is
# in the series
@@ -109,7 +110,11 @@ if [ $sidx -gt $eidx ]; then
else
disp File series fully applied, ends at patch `get_series |
tail -n 1`
fi
- exit 0
+ if [ -n $all ]; then
+ exit 0
+ else
+ exit 1
+ fi
fi
get_series | sed -n -e ${sidx},${eidx}p | while read p
diff --git a/regression/t-020.out b/regression/t-020.out
index 42433dc..bcb8797 100644
--- a/regression/t-020.out
+++ b/regression/t-020.out
@@ -270,6 +270,95 @@ index 000..8baef1b
+++ b/def
@@ -0,0 +1 @@
+abc
+% guilt push
+File series fully applied, ends at patch mode
+% guilt push -a
+File series fully applied, ends at patch mode
+% list_files
+d .git/patches
+d .git/patches/master
+d .git/refs/patches
+d .git/refs/patches/master
+f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode
+f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove
+f 71596bf71b72c2717e1aee378aabefbfa19ab7c8 .git/patches/master/status
+f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add
+f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series
+f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify
+r 33633e7a1aa31972f125878baf7807be57b1672d .git/refs/patches/master/modify
+r 37d588cc39848368810e88332bd03b083f2ce3ac .git/refs/patches/master/add
+r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba .git/refs/patches/master/mode
+r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 .git/refs/patches/master/remove
+% git log -p
+commit ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch mode
+
+diff --git a/def b/def
+old mode 100644
+new mode 100755
+
+commit ffb7faa126a6d91bcdd44a494f76b96dd860b8b9
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch remove
+
+diff --git a/abd b/abd
+deleted file mode 100644
+index fd3896d..000
+--- a/abd
/dev/null
+@@ -1 +0,0 @@
+-öuؽáZâñeÏÈE£ WÀV¼/U?Ú|¢@6¤8'H¸1G_ͧ*·ðRÒ¤
ªÂ~·
+\ No newline at end of file
+
+commit 37d588cc39848368810e88332bd03b083f2ce3ac
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch add
+
+diff --git a/abd b/abd
+new file mode 100644
+index 000..fd3896d
+--- /dev/null
b/abd
+@@ -0,0 +1 @@
++öuؽáZâñeÏÈE£ WÀV¼/U?Ú|¢@6¤8'H¸1G_ͧ*·ðRÒ¤
ªÂ~·
+\ No newline at end of file
+
+commit 33633e7a1aa31972f125878baf7807be57b1672d
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+patch modify
+
+diff --git a/def b/def
+index 8baef1b..7d69c2f 100644
+--- a/def
b/def
+@@ -1 +1,2 @@
+ abc
++asjhfksad
+
+commit d4850419ccc1146c7169f500725ce504b9774ed0
+Author: Author Name author@email
+Date: Mon Jan 1 00:00:00 2007 +
+
+initial
+
+Signed-off-by: Commiter Name commiter@email
+
+diff --git a/def b/def
+new file mode 100644
+index 000..8baef1b
+--- /dev/null
b/def
+@@ -0,0 +1 @@
++abc
% guilt pop --all
All patches popped.
% guilt push
diff --git a/regression/t-020.sh b/regression/t-020.sh
index
[GUILT v3 27/31] Minor testsuite fix.
Fix remove_topic() in t-061.sh so that it doesn't print a git hash.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
regression/t-061.out | 1 -
regression/t-061.sh | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/regression/t-061.out b/regression/t-061.out
index ef0f335..60ad56d 100644
--- a/regression/t-061.out
+++ b/regression/t-061.out
@@ -381,7 +381,6 @@ ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit
refs/patches/master/mode
ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit
refs/patches/master/remove
% guilt pop -a
No patches applied.
-ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
% git checkout guilt/master
Switched to branch guilt/master
% guilt pop -a
diff --git a/regression/t-061.sh b/regression/t-061.sh
index 6192f1b..db26e12 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -15,7 +15,7 @@ old_style_branch() {
remove_topic() {
cmd guilt pop -a
- if git rev-parse --verify --quiet guilt/master
+ if git rev-parse --verify --quiet guilt/master /dev/null
then
cmd git checkout guilt/master
else
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 28/31] Fix coding style errors in t-061.sh.
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/t-061.sh | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/regression/t-061.sh b/regression/t-061.sh
index db26e12..bda50c7 100755
--- a/regression/t-061.sh
+++ b/regression/t-061.sh
@@ -15,8 +15,7 @@ old_style_branch() {
remove_topic() {
cmd guilt pop -a
- if git rev-parse --verify --quiet guilt/master /dev/null
- then
+ if git rev-parse --verify --quiet guilt/master /dev/null; then
cmd git checkout guilt/master
else
cmd git checkout master
@@ -46,8 +45,7 @@ cmd git for-each-ref
cmd list_files
-for i in `seq 5`
-do
+for i in `seq 5`; do
if [ $i -ge 5 ]; then
shouldfail guilt pop
else
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 26/31] guilt pop now fails when there are no more patches to pop.
This is analogous to how guilt push now fails when there are no more patches to push. Like push, the --all argument still succeeds even if there was no need to pop anything. Updated the test suite. Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- guilt-pop| 17 +++-- regression/t-021.out | 2 ++ regression/t-021.sh | 6 ++ regression/t-061.sh | 6 +- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/guilt-pop b/guilt-pop index f0e647f..191313e 100755 --- a/guilt-pop +++ b/guilt-pop @@ -49,9 +49,19 @@ fi patch=$1 [ ! -z $all ] patch=-a +# Treat guilt pop as guilt pop -n 1. +if [ -z $patch ]; then + patch=1 + num=t +fi + if [ ! -s $applied ]; then disp No patches applied. - exit 0 + if [ $patch = -a ]; then + exit 0 + else + exit 1 + fi elif [ $patch = -a ]; then # we are supposed to pop all patches @@ -68,11 +78,6 @@ elif [ ! -z $num ]; then # catch underflow [ $eidx -lt 0 ] eidx=0 [ $eidx -eq $sidx ] die No patches requested to be removed. -elif [ -z $patch ]; then - # we are supposed to pop only the current patch on the stack - - sidx=`wc -l $applied` - eidx=`expr $sidx - 1` else # we're supposed to pop only up to a patch, make sure the patch is # in the series diff --git a/regression/t-021.out b/regression/t-021.out index 9b42d9c..58be12f 100644 --- a/regression/t-021.out +++ b/regression/t-021.out @@ -287,6 +287,8 @@ index 000..8baef1b +++ b/def @@ -0,0 +1 @@ +abc +% guilt pop +No patches applied. % guilt push --all Applying patch..modify Patch applied. diff --git a/regression/t-021.sh b/regression/t-021.sh index 614e870..e0d2dc1 100755 --- a/regression/t-021.sh +++ b/regression/t-021.sh @@ -23,6 +23,12 @@ guilt series | _tac | while read n ; do done # +# pop when there is nothing to pop +# + +shouldfail guilt pop + +# # push all # cmd guilt push --all diff --git a/regression/t-061.sh b/regression/t-061.sh index 1411baa..6192f1b 100755 --- a/regression/t-061.sh +++ b/regression/t-061.sh @@ -48,7 +48,11 @@ cmd list_files for i in `seq 5` do - cmd guilt pop + if [ $i -ge 5 ]; then + shouldfail guilt pop + else + cmd guilt pop + fi cmd git for-each-ref cmd guilt push cmd git for-each-ref -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GUILT v3 29/31] Added guilt.reusebranch configuration option.
When the option is true, Guilt does not create a new Git branch when patches are applied. This way, you can switch between Guilt 0.35 and the current version of Guilt with no issues. By default, the option is false, so that all users will immediately get the added protection against pushing a branch upstream with a patch applied. While this might break guilt if a user is running both version 0.35 and the current version against the same local repository, it will not lead to data loss, and that situation is probably rare. Signed-off-by: Per Cederqvist ced...@opera.com --- guilt| 24 ++- regression/scaffold | 1 + regression/t-062.out | 420 +++ regression/t-062.sh | 130 4 files changed, 569 insertions(+), 6 deletions(-) create mode 100644 regression/t-062.out create mode 100755 regression/t-062.sh diff --git a/guilt b/guilt index 9947acc..ac7d046 100755 --- a/guilt +++ b/guilt @@ -853,6 +853,9 @@ guilt_push_diff_context=1 # default diffstat value: true or false DIFFSTAT_DEFAULT=false +# default guilt.reusebranch value: true or false +REUSE_BRANCH_DEFAULT=false + # Prefix for guilt branches. GUILT_PREFIX=guilt/ @@ -864,6 +867,10 @@ GUILT_PREFIX=guilt/ diffstat=`git config --bool guilt.diffstat` [ -z $diffstat ] diffstat=$DIFFSTAT_DEFAULT +# reuse Git branch? +reuse_branch=`git config --bool guilt.reusebranch` +[ -z $reuse_branch ] reuse_branch=$REUSE_BRANCH_DEFAULT + # # The following gets run every time this file is source'd # @@ -928,13 +935,18 @@ else die Unsupported operating system: $UNAME_S fi -if [ $branch = $raw_git_branch ] [ -n `get_top 2/dev/null` ] -then -# This is for compat with old repositories that still have a -# pushed patch without the new-style branch prefix. -old_style_prefix=true +if [ -n `get_top 2/dev/null` ]; then + # If there is at least one pushed patch, we set + # old_style_prefix according to how it was pushed. It is only + # possible to change the prefix style while no patches are + # applied. + if [ $branch = $raw_git_branch ]; then + old_style_prefix=true + else + old_style_prefix=false + fi else -old_style_prefix=false + old_style_prefix=$reuse_branch fi _main $@ diff --git a/regression/scaffold b/regression/scaffold index e4d7487..e4d2f35 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -93,6 +93,7 @@ function setup_git_repo git config log.date default git config log.decorate no git config guilt.diffstat false + git config guilt.reusebranch false } function setup_guilt_repo diff --git a/regression/t-062.out b/regression/t-062.out new file mode 100644 index 000..ad5c081 --- /dev/null +++ b/regression/t-062.out @@ -0,0 +1,420 @@ +% setup_repo +% git config guilt.reusebranch true +% guilt push -a +Applying patch..modify +Patch applied. +Applying patch..add +Patch applied. +Applying patch..remove +Patch applied. +Applying patch..mode +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 71596bf71b72c2717e1aee378aabefbfa19ab7c8 .git/patches/master/status +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +r 33633e7a1aa31972f125878baf7807be57b1672d .git/refs/patches/master/modify +r 37d588cc39848368810e88332bd03b083f2ce3ac .git/refs/patches/master/add +r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba .git/refs/patches/master/mode +r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 .git/refs/patches/master/remove +% git for-each-ref +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/patches/master/mode +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% guilt pop +Now at remove. +% git for-each-ref +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commitrefs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% guilt push +Applying patch..mode +Patch applied. +% git for-each-ref +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commitrefs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commitrefs/patches/master/add +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba
[GUILT v3 30/31] Added a short style guide, and Emacs settings.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- .dir-locals.el | 3 +++ Documentation/Contributing | 15 +++ 2 files changed, 18 insertions(+) create mode 100644 .dir-locals.el diff --git a/.dir-locals.el b/.dir-locals.el new file mode 100644 index 000..50ef2b7 --- /dev/null +++ b/.dir-locals.el @@ -0,0 +1,3 @@ +((nil . ((indent-tabs-mode . t) +(tab-width . 8))) + (sh-mode . ((sh-basic-offset . 8 diff --git a/Documentation/Contributing b/Documentation/Contributing index abf3480..0da49d6 100644 --- a/Documentation/Contributing +++ b/Documentation/Contributing @@ -4,6 +4,21 @@ Documentation/SubmittingPatches file. :) 1) Hack on the code a bit +Please follow this style guide: + + - Use tabs for indentation. + + - Put then on the same line as if. + + - Follow the style of the existing code, except if it breaks the + above guidlines. + + - If you change the code to conform to the style guide, please do so + in a separate commit that does not change anything else. + +Please check that you change does not break make test. Please add +new testcases for any new functionality, and if you fix a bug. + 2) Make a patch: Use diff -up or diff -uprN to create patches. Or simply use git to -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Fedex Delivery Post
Fedex Delivery Post Open Attachments File and Contact: fedex-courierservic...@fengv.commailto:fedex-courierservic...@fengv.com FEDEX.docx Description: FEDEX.docx
[GUILT v3 13/31] Check that guilt header '.*' fails.
Signed-off-by: Per Cederqvist ced...@opera.com Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net --- regression/t-028.out | 7 +++ regression/t-028.sh | 4 2 files changed, 11 insertions(+) diff --git a/regression/t-028.out b/regression/t-028.out index 1564c09..ea72a3a 100644 --- a/regression/t-028.out +++ b/regression/t-028.out @@ -49,3 +49,10 @@ Signed-off-by: Commiter Name commiter@email % guilt header non-existant Patch non-existant is not in the series +% guilt header .* +.* does not uniquely identify a patch. Did you mean any of these? + modify + add + remove + mode + patch-with-some-desc diff --git a/regression/t-028.sh b/regression/t-028.sh index 88e9adb..2ce0378 100755 --- a/regression/t-028.sh +++ b/regression/t-028.sh @@ -31,4 +31,8 @@ done shouldfail guilt header non-existant +# This is an evil variant of a non-existant patch. However, this +# patch name is a regexp that just happens to match an existing patch. +shouldfail guilt header '.*' + # FIXME: how do we check that -e works? -- 1.8.3.1 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v3 08/31] Added more test cases for guilt new: empty patches.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Fri, May 16, 2014 at 04:45:55PM +0200, Per Cederqvist wrote: Test that empty patches are handled correctly, both with and without the guilt.diffstat configuration option. Signed-off-by: Per Cederqvist ced...@opera.com --- regression/t-020.out | 269 +++ regression/t-020.sh | 60 2 files changed, 329 insertions(+) diff --git a/regression/t-020.out b/regression/t-020.out index af45734..42433dc 100644 --- a/regression/t-020.out +++ b/regression/t-020.out @@ -1128,3 +1128,272 @@ f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/status +% guilt new empty.patch +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat true +% guilt refresh +Patch empty.patch refreshed +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7d261b8caad0f161c21daf5de65eeb521ff8c067 .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 .git/refs/patches/master/empty.patch +% git log -p +commit c7a139f532a43c3c8b0e068cac04f8f6af0f94e1 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +patch empty.patch + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author: Author Name author@email +Date: Mon Jan 1 00:00:00 2007 + + +initial + +Signed-off-by: Commiter Name commiter@email + +diff --git a/def b/def +new file mode 100644 +index 000..8baef1b +--- /dev/null b/def +@@ -0,0 +1 @@ ++abc +% git config guilt.diffstat false +% guilt pop +All patches popped. +% guilt push +Applying patch..empty.patch +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 7ad87a0bdb8cf0a57cfc384633edabbb9c2bfa1b .git/patches/master/empty.patch +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +f d15a1d2d34493f790c78ddacb8815b0b9536ee2b .git/patches/master/series +f da39a3ee5e6b4b0d3255bfef95601890afd80709 .git/patches/master/empty.patch~ +f e90b964f01cbef60bbe00c38c55d9ea86618a66a .git/patches/master/status +r 8ed27228b117c0c88abf3d586bcc43c68e975cea .git/refs/patches/master/empty.patch +% git log -p +commit 8ed27228b117c0c88abf3d586bcc43c68e975cea +Author: Per Cederqvist ce...@lysator.liu.se +Date: Mon Jan 1 00:00:00 2007 + + +Fix a bug. + +This commit fixes a serious bug. + +FIXME: +- add a test case +- track down the bug +- actually fix it + +commit d4850419ccc1146c7169f500725ce504b9774ed0 +Author:
Re: [PATCH v8 21/44] refs.c: ref_transaction_commit should not free the transaction
On Thu, May 15, 2014 at 5:20 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Ronnie Sahlberg wrote:
Change ref_transaction_commit so that it does not free the transaction.
Instead require that a caller will end a transaction by either calling
ref_transaction_rollback or ref_transaction_free.
Can I always use ref_transaction_rollback instead of ref_transaction_free?
It would be convenient if my cleanup code could always call _rollback
instead of having to do something different for success and errors.
Currently, yes.
But it might make sense to change these so rollback only clears the
updates that are in flight from the transaction and
free only frees the transaction itself iff there are no updates in flight.
I.e. the success and error would then differ like this :
...
if (transaction_commit()) {
transaction_rollback()
transaction_free()
return error(some error)
}
transaction_free()
Another way to ask the question: what is it valid to do with a
transaction after commiting?
Right now the only valid thing to do is either rollback or free. But
we could allow other things too :
re-usable transactions.
-
I don't know if this is a good reason or not, but one reason we might
want to keep
two different names could be if we want to start allowing to re-use
transactions.
For example for cases/backends where transaction_begin() might be very
expensive.
For that case I would imagine we could allow to do things such as
t = transaction_begin()
...
/* first transaction */
transaction_update(...)
transaction_commit(...)
if transaction failed transaction_rollback(...)
/* second transaction, first transaction cleared all updates in
flight either through commit or through rollback */
transaction_update()
transaction_commit()
if transaction failed transaction_rollback(...)
...
transaction_free()
(In order to do something like this we would still need to do some
changes so that rollback will only free the updates that were in
flight but not free the transaction itself.)
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 21/44] refs.c: ref_transaction_commit should not free the transaction
On Fri, May 16, 2014 at 8:02 AM, Ronnie Sahlberg sahlb...@google.com wrote:
On Thu, May 15, 2014 at 5:20 PM, Jonathan Nieder jrnie...@gmail.com wrote:
Ronnie Sahlberg wrote:
Change ref_transaction_commit so that it does not free the transaction.
Instead require that a caller will end a transaction by either calling
ref_transaction_rollback or ref_transaction_free.
Can I always use ref_transaction_rollback instead of ref_transaction_free?
It would be convenient if my cleanup code could always call _rollback
instead of having to do something different for success and errors.
Currently, yes.
But it might make sense to change these so rollback only clears the
updates that are in flight from the transaction and
free only frees the transaction itself iff there are no updates in flight.
I.e. the success and error would then differ like this :
...
if (transaction_commit()) {
transaction_rollback()
transaction_free()
return error(some error)
}
transaction_free()
But we do not really need rollback right now. If / when we decide to
need re-startable/re-usable transactions we can add it back when
needed.
Let me update the patch series and remove transaction_rollback and
replace all calls to it with calls to transaction_free instead.
Another way to ask the question: what is it valid to do with a
transaction after commiting?
Right now the only valid thing to do is either rollback or free. But
we could allow other things too :
re-usable transactions.
-
I don't know if this is a good reason or not, but one reason we might
want to keep
two different names could be if we want to start allowing to re-use
transactions.
For example for cases/backends where transaction_begin() might be very
expensive.
For that case I would imagine we could allow to do things such as
t = transaction_begin()
...
/* first transaction */
transaction_update(...)
transaction_commit(...)
if transaction failed transaction_rollback(...)
/* second transaction, first transaction cleared all updates in
flight either through commit or through rollback */
transaction_update()
transaction_commit()
if transaction failed transaction_rollback(...)
...
transaction_free()
(In order to do something like this we would still need to do some
changes so that rollback will only free the updates that were in
flight but not free the transaction itself.)
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v3 12/31] guilt header: more robust header selection.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Fri, May 16, 2014 at 04:45:59PM +0200, Per Cederqvist wrote: If you run something like guilt header '.*' the command would crash, because the grep comand that tries to ensure that the patch exist would detect a match, but the later code expected the match to be exact. Fixed by comparing exact strings. And as a creeping feature guilt header will now try to use the supplied patch name as an unachored regexp if no exact match was found. If the regexp yields a unique match, it is used; if more than one patch matches, the names of all patches are listed and the command fails. (Exercise left to the reader: generalized this so that guilt push also accepts a unique regular expression.) Signed-off-by: Per Cederqvist ced...@opera.com --- guilt-header | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/guilt-header b/guilt-header index 41e00cc..c3d24f9 100755 --- a/guilt-header +++ b/guilt-header @@ -45,10 +45,33 @@ esac [ -z $patch ] die No patches applied. # check that patch exists in the series -ret=`get_full_series | grep -e ^$patch\$ | wc -l` -if [ $ret -eq 0 ]; then - die Patch $patch is not in the series +TMP_FULL_SERIES=`get_tmp_file series` +get_full_series $TMP_FULL_SERIES +found_patch= +while read x; do + if [ $x = $patch ]; then + found_patch=$patch + break + fi +done $TMP_FULL_SERIES +if [ -z $found_patch ]; then + TMP_MATCHES=`get_tmp_file series` + grep $patch $TMP_FULL_SERIES $TMP_MATCHES + nr=`wc -l $TMP_MATCHES` + if [ $nr -gt 1 ]; then + echo $patch does not uniquely identify a patch. Did you mean any of these? 2 + sed 's/^/ /' $TMP_MATCHES 2 + rm -f $TMP_MATCHES $TMP_FULL_SERIES + exit 1 + elif [ $nr -eq 0 ]; then + rm -f $TMP_MATCHES $TMP_FULL_SERIES + die Patch $patch is not in the series + fi + found_patch=`cat $TMP_MATCHES` + rm -f $TMP_MATCHES fi +rm -f $TMP_FULL_SERIES +patch=$found_patch # FIXME: warn if we're editing an applied patch -- 1.8.3.1 -- Failure is not an option, It comes bundled with your Microsoft product. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GUILT v3 29/31] Added guilt.reusebranch configuration option.
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net On Fri, May 16, 2014 at 04:46:16PM +0200, Per Cederqvist wrote: When the option is true, Guilt does not create a new Git branch when patches are applied. This way, you can switch between Guilt 0.35 and the current version of Guilt with no issues. By default, the option is false, so that all users will immediately get the added protection against pushing a branch upstream with a patch applied. While this might break guilt if a user is running both version 0.35 and the current version against the same local repository, it will not lead to data loss, and that situation is probably rare. Signed-off-by: Per Cederqvist ced...@opera.com --- guilt| 24 ++- regression/scaffold | 1 + regression/t-062.out | 420 +++ regression/t-062.sh | 130 4 files changed, 569 insertions(+), 6 deletions(-) create mode 100644 regression/t-062.out create mode 100755 regression/t-062.sh diff --git a/guilt b/guilt index 9947acc..ac7d046 100755 --- a/guilt +++ b/guilt @@ -853,6 +853,9 @@ guilt_push_diff_context=1 # default diffstat value: true or false DIFFSTAT_DEFAULT=false +# default guilt.reusebranch value: true or false +REUSE_BRANCH_DEFAULT=false + # Prefix for guilt branches. GUILT_PREFIX=guilt/ @@ -864,6 +867,10 @@ GUILT_PREFIX=guilt/ diffstat=`git config --bool guilt.diffstat` [ -z $diffstat ] diffstat=$DIFFSTAT_DEFAULT +# reuse Git branch? +reuse_branch=`git config --bool guilt.reusebranch` +[ -z $reuse_branch ] reuse_branch=$REUSE_BRANCH_DEFAULT + # # The following gets run every time this file is source'd # @@ -928,13 +935,18 @@ else die Unsupported operating system: $UNAME_S fi -if [ $branch = $raw_git_branch ] [ -n `get_top 2/dev/null` ] -then -# This is for compat with old repositories that still have a -# pushed patch without the new-style branch prefix. -old_style_prefix=true +if [ -n `get_top 2/dev/null` ]; then + # If there is at least one pushed patch, we set + # old_style_prefix according to how it was pushed. It is only + # possible to change the prefix style while no patches are + # applied. + if [ $branch = $raw_git_branch ]; then + old_style_prefix=true + else + old_style_prefix=false + fi else -old_style_prefix=false + old_style_prefix=$reuse_branch fi _main $@ diff --git a/regression/scaffold b/regression/scaffold index e4d7487..e4d2f35 100644 --- a/regression/scaffold +++ b/regression/scaffold @@ -93,6 +93,7 @@ function setup_git_repo git config log.date default git config log.decorate no git config guilt.diffstat false + git config guilt.reusebranch false } function setup_guilt_repo diff --git a/regression/t-062.out b/regression/t-062.out new file mode 100644 index 000..ad5c081 --- /dev/null +++ b/regression/t-062.out @@ -0,0 +1,420 @@ +% setup_repo +% git config guilt.reusebranch true +% guilt push -a +Applying patch..modify +Patch applied. +Applying patch..add +Patch applied. +Applying patch..remove +Patch applied. +Applying patch..mode +Patch applied. +% list_files +d .git/patches +d .git/patches/master +d .git/refs/patches +d .git/refs/patches/master +f 22930c6d1f1938f298a4fca51c57e4b47171db21 .git/patches/master/mode +f 413390f3906f16f30b054a4fb86c1e014b964504 .git/patches/master/remove +f 71596bf71b72c2717e1aee378aabefbfa19ab7c8 .git/patches/master/status +f 9c18cc7abe6b87f18503714a80a677b4094eb457 .git/patches/master/add +f bacb4aad8a55fe4e7aa58a9ae169990bb764069f .git/patches/master/series +f bc9ab2e0f5db99d483961e956e814d963f0309f8 .git/patches/master/modify +r 33633e7a1aa31972f125878baf7807be57b1672d .git/refs/patches/master/modify +r 37d588cc39848368810e88332bd03b083f2ce3ac .git/refs/patches/master/add +r ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba .git/refs/patches/master/mode +r ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 .git/refs/patches/master/remove +% git for-each-ref +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit refs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commit refs/patches/master/add +ccd56089d1b5305a9d35617cb7f6f4b06ffa68ba commit refs/patches/master/mode +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% guilt pop +Now at remove. +% git for-each-ref +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/heads/master +37d588cc39848368810e88332bd03b083f2ce3ac commit refs/patches/master/add +33633e7a1aa31972f125878baf7807be57b1672d commit refs/patches/master/modify +ffb7faa126a6d91bcdd44a494f76b96dd860b8b9 commit refs/patches/master/remove +% guilt push +Applying patch..mode +Patch applied. +% git
Re: [GUILT v3 14/31] Use git check-ref-format to validate patch names.
On Fri, May 16, 2014 at 04:46:01PM +0200, Per Cederqvist wrote:
The valid_patchname now lets git check-ref-format do its job instead
of trying (and failing) to implement the same rules. See
git-check-ref-format(1) for a list of the rules.
Refer to the git-check-ref-format(1) man page in the error messages
produced when valid_patchname indicates that the name is bad.
Added testcases that breaks most of the rules in that man-page.
Git version 1.8.5 no longer allows the single character @ as a
branch name. Guilt always rejects that name, for increased
compatibility.
Signed-off-by: Per Cederqvist ced...@opera.com
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
---
guilt| 21 ++-
guilt-fork | 2 +-
guilt-import | 2 +-
guilt-new| 2 +-
regression/t-025.out | 426
+--
regression/t-025.sh | 12 +-
regression/t-032.out | 4 +-
7 files changed, 446 insertions(+), 23 deletions(-)
diff --git a/guilt b/guilt
index 3fc524e..23cc2da 100755
--- a/guilt
+++ b/guilt
@@ -132,14 +132,19 @@ fi
# usage: valid_patchname patchname
valid_patchname()
{
- case $1 in
- /*|./*|../*|*/./*|*/../*|*/.|*/..|*/|*\ *|*\*)
- return 1;;
- *:*)
- return 1;;
- *)
- return 0;;
- esac
+ if git check-ref-format --allow-onelevel $1; then
I know I already signed off on this, but I just tried to run the regression
suite with your changes on 1.7.3.2. It blows up because --allow-onelevel is
not recognized. Yes, 1.7 is a bit on the old side but I think it's
reasonable to still support it. Thoughts?
+ # Starting with Git version 1.8.5, a branch cannot be
+ # the single character @. Make sure guilt rejects
+ # that name even if we are currently using an older
+ # version of Git. This ensures that the test suite
+ # runs fine using any version of Git.
+ if [ $1 = @ ]; then
+ return 1
+ fi
+ return 0
+ else
+ return 1
+ fi
}
get_branch()
diff --git a/guilt-fork b/guilt-fork
index a85d391..6447e55 100755
--- a/guilt-fork
+++ b/guilt-fork
@@ -37,7 +37,7 @@ else
fi
if ! valid_patchname $newpatch; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
if [ -e $GUILT_DIR/$branch/$newpatch ]; then
diff --git a/guilt-import b/guilt-import
index 3e9b3bb..928e325 100755
--- a/guilt-import
+++ b/guilt-import
@@ -40,7 +40,7 @@ if [ -e $GUILT_DIR/$branch/$newname ]; then
fi
if ! valid_patchname $newname; then
- die The specified patch name contains invalid characters (:).
+ die The specified patch name is invalid according to
git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/guilt-new b/guilt-new
index 9528438..9f7fa44 100755
--- a/guilt-new
+++ b/guilt-new
@@ -64,7 +64,7 @@ fi
if ! valid_patchname $patch; then
disp Patchname is invalid. 2
- die it cannot begin with '/', './' or '../', or contain /./, /../, or
whitespace
+ die It must follow the rules in git-check-ref-format(1).
fi
# create any directories as needed
diff --git a/regression/t-025.out b/regression/t-025.out
index 7811ab1..01bc406 100644
--- a/regression/t-025.out
+++ b/regression/t-025.out
@@ -141,7 +141,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new white space
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -211,7 +211,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new /abc
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -235,7 +235,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8 .git/refs/patches/master/prepend
% guilt new ./blah
Patchname is invalid.
-it cannot begin with '/', './' or '../', or contain /./, /../, or whitespace
+It must follow the rules in git-check-ref-format(1).
% list_files
d .git/patches
d .git/patches/master
@@ -259,7 +259,7 @@ f da39a3ee5e6b4b0d3255bfef95601890afd80709
.git/patches/master/prepend
r acdeef96ee30eb34bbbf65d11de5cf7da4b5fee8
[PATCH v2 0/2] Reroll patch series. Pretty print truncate does not work
In this reroll (against v1) remarks of Nguyễn are respected. Comments style changed from C++ to C. variable declaration moved back to the beginning of a function. Also, added tests for the same case for git rev-list (see t6006-rev-list-format.sh) Alexey Shumkin (2): t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set pretty.c: format string with truncate respects logOutputEncoding pretty.c | 7 +- t/t4205-log-pretty-formats.sh | 169 ++ t/t6006-rev-list-format.sh| 75 ++- 3 files changed, 248 insertions(+), 3 deletions(-) -- 1.9.2-17 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] t4205, t6006: Add failing tests for the case when i18n.logOutputEncoding is set
Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.
There were no breakages as far as were no tests for the case
when both a commit message and logOutputEncoding are not UTF-8.
Add failing tests for that which will be fixed in the next patch.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
t/t4205-log-pretty-formats.sh | 169 ++
t/t6006-rev-list-format.sh| 75 ++-
2 files changed, 242 insertions(+), 2 deletions(-)
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 2a6278b..6791e0d 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -153,6 +153,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting. i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(40)%s
actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+message twoZ
+message oneZ
+add barZ
+$(commit_msg)Z
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting at the nth column' '
git log --pretty=format:%h %|(40)%s actual
# complete the incomplete line at the end
@@ -166,6 +179,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting at the nth column.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%h
%|(40)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+$head1 message twoZ
+$head2 message oneZ
+$head3 add barZ
+$head4 $(commit_msg)Z
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with no padding' '
git log --pretty=format:%(1)%s actual
# complete the incomplete line at the end
@@ -179,6 +205,19 @@ EOF
test_cmp expected actual
'
+test_expect_success 'left alignment formatting with no padding.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log --pretty=format:%(1)%s
actual
+ # complete the incomplete line at the end
+ echo actual
+ cat EOF | iconv -f utf-8 -t iso8859-1 expected
+message two
+message one
+add bar
+$(commit_msg)
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with trunc' '
git log --pretty=format:%(10,trunc)%s actual
# complete the incomplete line at the end
@@ -192,6 +231,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+message ..
+message ..
+add bar Z
+initial...
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with ltrunc' '
git log --pretty=format:%(10,ltrunc)%s actual
# complete the incomplete line at the end
@@ -205,6 +257,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,ltrunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+..sage two
+..sage one
+add bar Z
+..${sample_utf8_part}lich
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'left alignment formatting with mtrunc' '
git log --pretty=format:%(10,mtrunc)%s actual
# complete the incomplete line at the end
@@ -218,6 +283,19 @@ EOF
test_cmp expected actual
'
+test_expect_failure 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
+ git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,mtrunc)%s actual
+ # complete the incomplete line at the end
+ echo actual
+ qz_to_tab_space EOF | iconv -f utf-8 -t iso8859-1 expected
+mess.. two
+mess.. one
+add bar Z
+init..lich
+EOF
+ test_cmp expected actual
+'
+
test_expect_success 'right alignment formatting' '
git log --pretty=format:%(40)%s actual
# complete the incomplete line at the end
@@
[PATCH v2 2/2] pretty.c: format string with truncate respects logOutputEncoding
Pretty format string %(N,[ml]trunc)%s truncates subject to a given
length with an appropriate padding. This works for non-ASCII texts when
i18n.logOutputEncoding is UTF-8 only (independently of a printed commit
message encoding) but does not work when i18n.logOutputEncoding is NOT
UTF-8.
In 7e77df3 (pretty: two phase conversion for non utf-8 commits, 2013-04-19)
'format_commit_item' function assumes commit message to be in UTF-8.
And that was so until ecaee80 (pretty: --format output should honor
logOutputEncoding, 2013-06-26) where conversion to logOutputEncoding was
added before calling 'format_commit_message'.
Correct this by converting a commit message to UTF-8 first (as it
assumed in 7e77df3 (pretty: two phase conversion for non utf-8 commits,
2013-04-19)). Only after that convert a commit message to an actual
logOutputEncoding.
Signed-off-by: Alexey Shumkin alex.crez...@gmail.com
Reviewed-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
pretty.c | 7 ++-
t/t4205-log-pretty-formats.sh | 8
t/t6006-rev-list-format.sh| 6 +++---
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/pretty.c b/pretty.c
index 6e266dd..25e8825 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1507,13 +1507,18 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
+ /*
+* convert a commit message to UTF-8 first
+* as far as 'format_commit_item' assumes it in UTF-8
+*/
context.message = logmsg_reencode(commit,
context.commit_encoding,
- output_enc);
+ utf8);
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
+ /* then convert a commit message to an actual output encoding */
if (output_enc) {
if (same_encoding(utf8, output_enc))
output_enc = NULL;
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index 6791e0d..7426fe2 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -231,7 +231,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with trunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -257,7 +257,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with ltrunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,ltrunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -283,7 +283,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
+test_expect_success 'left alignment formatting with mtrunc.
i18n.logOutputEncoding' '
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,mtrunc)%s actual
# complete the incomplete line at the end
echo actual
@@ -465,7 +465,7 @@ EOF
test_cmp expected actual
'
-test_expect_failure 'left/right alignment formatting with stealing.
i18n.logOutputEncoding' '
+test_expect_success 'left/right alignment formatting with stealing.
i18n.logOutputEncoding' '
git commit --amend -m short --author long long long l...@me.com
git -c i18n.logOutputEncoding=iso8859-1 log
--pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual
# complete the incomplete line at the end
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 09cdf24..04811fd 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -256,7 +256,7 @@ commit $head1
$added_iso88591
EOF
-test_format complex-subject-trunc %($truncate_count,trunc)%s failure EOF
+test_format complex-subject-trunc %($truncate_count,trunc)%s EOF
commit $head3
Test printing of c..
commit $head2
@@ -265,7 +265,7 @@ commit $head1
added (hinzugef${added_utf8_part_iso88591}gt..
EOF
-test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s failure EOF
+test_format complex-subject-mtrunc %($truncate_count,mtrunc)%s EOF
commit $head3
Test prin..ex bodies
commit $head2
@@ -274,7 +274,7 @@ commit $head1
added (hi..f${added_utf8_part_iso88591}gt) foo
EOF
-test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s failure EOF
+test_format complex-subject-ltrunc %($truncate_count,ltrunc)%s EOF
commit $head3
.. of complex bodies
commit $head2
--
1.9.2-17
--
To unsubscribe from this list: send the
Re: [GUILT v3 09/31] Test suite: properly check the exit status of commands.
On Fri, May 16, 2014 at 04:45:56PM +0200, Per Cederqvist wrote:
The cmd and shouldfail functions checked the exit status of the
replace_path function instead of the actual command that was running.
(The $? construct checks the exit status of the last command in a
pipeline, not the first command.)
Updated t-032.sh, which used shouldfail instead of cmd in one
place. (The comment in the script makes it clear that the command is
expected to succeed.)
Signed-off-by: Per Cederqvist ced...@opera.com
---
regression/scaffold | 17 +++--
regression/t-032.sh | 2 +-
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/regression/scaffold b/regression/scaffold
index 5c8b73e..e4d7487 100644
--- a/regression/scaffold
+++ b/regression/scaffold
@@ -51,18 +51,23 @@ function filter_dd
function cmd
{
echo % $@
- $@ 21 | replace_path return 0
- return 1
+ (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
Wow. This took a while to decipher :)
Signed-off-by: Josef 'Jeff' Sipek jef...@josefsipek.net
+ exit $rv
+ )
+ return $?
}
# usage: shouldfail cmd..
function shouldfail
{
echo % $@
- (
- $@ 21 || return 0
- return 1
- ) | replace_path
+ ! (
+ exec 31
+ rv=`(($@ 21; echo $? 4) | replace_path 3 ) 41`
+ exit $rv
+ )
return $?
}
diff --git a/regression/t-032.sh b/regression/t-032.sh
index b1d5f19..bba401e 100755
--- a/regression/t-032.sh
+++ b/regression/t-032.sh
@@ -28,7 +28,7 @@ shouldfail guilt import -P foo3 foo
cmd guilt import -P foo2 foo
# ok
-shouldfail guilt import foo
+cmd guilt import foo
# duplicate patch name (implicit)
shouldfail guilt import foo
--
1.8.3.1
--
Fact: 28.1% of all statistics are generated randomly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] config: be strict on core.commentChar
Nguyễn Thái Ngọc Duy wrote:
--- a/config.c
+++ b/config.c
@@ -826,8 +826,12 @@ static int git_default_core_config(const char *var,
const char *value)
if (!strcmp(var, core.commentchar)) {
const char *comment;
int ret = git_config_string(comment, var, value);
- if (!ret)
- comment_line_char = comment[0];
+ if (!ret) {
+ if (comment[0] !comment[1])
+ comment_line_char = comment[0];
+ else
+ return error(core.commentChar should only be
one character);
+ }
Perhaps, to decrease indentation a little:
if (ret)
return ret;
if (comment[0] !comment[1])
comment_line_char = comment[0];
else
return error(...);
return 0;
[...]
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -1348,12 +1348,6 @@ test_expect_success status (core.commentchar with
submodule summary) '
test_i18ncmp expect output
'
-test_expect_success status (core.commentchar with two chars with submodule
summary) '
- test_config core.commentchar ;;
- git -c status.displayCommentPrefix=true status output
- test_i18ncmp expect output
Could keep the test to avoid regressions:
test_config core.commentchar ;;
test_must_fail git -c status.displayCommentPrefix=true status
Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] commit: allow core.commentChar=auto for character auto selection
Nguyễn Thái Ngọc Duy wrote:
core.commentChar starts with '#' as in default but if it's already in
the prepared message, find another one among a small subset. This
should stop surprises because git strips some lines unexpectedly.
Probably worth mentioning this only kicks in if someone explicitly
configures [core] commentchar = auto.
Would it be a goal to make 'auto' the default eventually if people
turn out to like it?
[...]
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -594,6 +594,40 @@ static char *cut_ident_timestamp_part(char *string)
return ket;
}
+static void adjust_comment_line_char(const struct strbuf *sb)
+{
+ char candidates[] = @!#$%^|:;~;
This prefers '@' over '#'. Intended?
[...]
+ char *candidate;
+ const char *p;
+
+ if (!sb-len)
+ return;
+
+ if (!strchr(candidates, comment_line_char))
+ candidates[0] = comment_line_char;
Could do
if (!memchr(sb-buf, comment_line_char, sb-len))
return;
to solve the precedence problem. The comment_line_char not appearing
in the message is the usual case and handling it separately means it
gets handled faster.
[...]
--- a/config.c
+++ b/config.c
@@ -829,6 +829,8 @@ static int git_default_core_config(const char *var, const
char *value)
if (!ret) {
if (comment[0] !comment[1])
comment_line_char = comment[0];
+ else if (!strcasecmp(comment, auto))
+ auto_comment_line_char = 1;
Is there a way to disable 'auto' if 'auto' is already set?
comment_line_char still can be set and matters when 'auto' is set.
Should they be separate settings?
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -563,4 +563,29 @@ test_expect_success 'commit --status with custom comment
character' '
[...]
+ GIT_EDITOR=.git/FAKE_EDITOR test_must_fail \
Shells make it obnoxiously hard to set a one-shot envvar while
calling a function without the setting leaking into later commands.
(
test_set_editor .git/FAKE_EDITOR
test_must_fail ...
)
or
test_must_fail env GIT_EDITOR=.git/FAKE_EDITOR ...
should do the trick.
Thanks and hope that helps,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 06/44] refs.c: add an err argument to delete_ref_loose
Add an err argument to delete_loose_ref so that we can pass a descriptive
error string back to the caller. Pass the err argument from transaction
commit to this function so that transaction users will have a nice error
string if the transaction failed due to delete_loose_ref.
Add a new function unlink_or_err that we can call from delete_ref_loose. This
function is similar to unlink_or_warn except that we can pass it an err
argument. If err is non-NULL the function will populate err instead of
printing a warning().
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 39 +--
1 file changed, 33 insertions(+), 6 deletions(-)
diff --git a/refs.c b/refs.c
index 57ec72a..bc444c7 100644
--- a/refs.c
+++ b/refs.c
@@ -2491,17 +2491,43 @@ static int repack_without_ref(const char *refname)
return repack_without_refs(refname, 1, NULL);
}
-static int delete_ref_loose(struct ref_lock *lock, int flag)
+static int add_err_if_unremovable(const char *op, const char *file,
+ struct strbuf *e, int rc)
+{
+ int err = errno;
+ if (rc 0 err != ENOENT) {
+ strbuf_addf(e, unable to %s %s: %s,
+ op, file, strerror(errno));
+ errno = err;
+ }
+ return rc;
+}
+
+static int unlink_or_err(const char *file, struct strbuf *err)
+{
+ if (err)
+ return add_err_if_unremovable(unlink, file, err,
+ unlink(file));
+ else
+ return unlink_or_warn(file);
+}
+
+static int delete_ref_loose(struct ref_lock *lock, int flag, struct strbuf
*err)
{
if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
/* loose */
- int err, i = strlen(lock-lk-filename) - 5; /* .lock */
+ int res, i = strlen(lock-lk-filename) - 5; /* .lock */
lock-lk-filename[i] = 0;
- err = unlink_or_warn(lock-lk-filename);
+ res = unlink_or_err(lock-lk-filename, err);
lock-lk-filename[i] = '.';
- if (err errno != ENOENT)
+ if (res errno != ENOENT) {
+ if (err)
+ strbuf_addf(err,
+ failed to delete loose ref '%s',
+ lock-lk-filename);
return 1;
+ }
}
return 0;
}
@@ -2514,7 +2540,7 @@ int delete_ref(const char *refname, const unsigned char
*sha1, int delopt)
lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
if (!lock)
return 1;
- ret |= delete_ref_loose(lock, flag);
+ ret |= delete_ref_loose(lock, flag, NULL);
/* removing the loose one could have resurrected an earlier
* packed one. Also, if it was not loose we need to repack
@@ -3494,7 +3520,8 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (update-lock) {
delnames[delnum++] = update-lock-ref_name;
- ret |= delete_ref_loose(update-lock, update-type);
+ ret |= delete_ref_loose(update-lock, update-type,
+ err);
}
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 00/44] Use ref transactions for all ref updates
This patch series can also be found at https://github.com/rsahlberg/git/tree/ref-transactions This patch series is based on next and expands on the transaction API. It converts all ref updates, inside refs.c as well as external, to use the transaction API for updates. This makes most of the ref updates to become atomic when there are failures locking or writing to a ref. This version completes the work to convert all ref updates to use transactions. Now that all updates are through transactions I will start working on cleaning up the reading of refs and to create an api for managing reflogs but all that will go in a different patch series. Version 10: - Add err argument to _update/_delete/_create - Remove the ref_transaction_rollback function - Other changes based on JN's reviews. Version 9: - Lots of updates to error messages based on JN's review. Version 8: - Updates after review by JN - Improve commit messages - Add a patch that adds an err argument to repack_without_refs - Add a patch that adds an err argument to delete_loose_ref - Better document that a _update/_delete/_create failure means the whole transaction has failed and needs rollback. Version 7: - Updated commit messages per JNs review comments. - Changed REF_ISPRUNING and REF_ISPACKONLY to be private flags and not exposed through refs.h Version 6: - Convert all updates in refs.c to use transactions too. Version 5: - Reword commit messages for having _create/_delete/_update returning success/failure. There are no conditions yet that return an error from these failures but there will be in the future. So we still check the return from these functions in the callers in preparation for this. - Don't leak memory by just passing a strbuf_detach() pointer to functions. Use obj.buf and explicitely strbuf_release the data afterwards. - Remove the function update_ref_lock. - Remove the function update_ref_write. - Track transaction status and die(BUG:) if we call _create/_delete/_update/ _commit for a transaction that is not OPEN. Version 4: - Rename patch series from Use ref transactions from most callers to Use ref transactions for all ref updates. - Convert all external ref writes to use transactions and make write_ref_sha1 and lock_ref_sha1 static functions. - Change the ref commit and free handling so we no longer pass pointer to pointer to _commit. _commit no longer frees the transaction. The caller MUST call _free itself. - Change _commit to take a strbuf pointer instead of a char* for error reporting back to the caller. - Re-add the walker patch after fixing it. Version 3: - Remove the walker patch for now. Walker needs more complex solution so defer it until the basics are done. - Remove the onerr argument to ref_transaction_commit(). All callers that need to die() on error now have to do this explicitely. - Pass an error string from ref_transaction_commit() back to the callers so that they can craft a nice error message upon failures. - Make ref_transaction_rollback() accept NULL as argument. - Change ref_transaction_commit() to take a pointer to pointer argument for the transaction and have it clear the callers pointer to NULL when invoked. This allows for much nicer handling of transaction rollback on failure. Version 2: - Add a patch to ref_transaction_commit to make it honor onerr even if the error triggered in ref_Transaction_commit itself rather than in a call to other functions (that already honor onerr). - Add a patch to make the update_ref() helper function use transactions internally. - Change ref_transaction_update to die() instead of error() if we pass if a NULL old_sha1 but have have_old == true. - Change ref_transaction_create to die() instead of error() if new_sha1 is false but we pass it a null_sha1. - Change ref_transaction_delete die() instead of error() if we pass if a NULL old_sha1 but have have_old == true. - Change several places to do if(!transaction || ref_transaction_update() || ref_Transaction_commit()) die(generic-message) instead of checking each step separately and having a different message for each failure. Most users are likely not interested in what step of the transaction failed and only whether it failed or not. - Change commit.c to only pass a pointer to ref_transaction_update iff current_head is non-NULL. The previous patch used to compute a garbage pointer for current_head-object.sha1 and relied on the fact that ref_transaction_update would not try to dereference this pointer if !!current_head was 0. - Updated commit message for the walker_fetch change to try to justify why the change in locking semantics should not be harmful. Ronnie Sahlberg (44): refs.c: constify the sha arguments for ref_transaction_create|delete|update refs.c: allow passing NULL to ref_transaction_free refs.c: add a strbuf argument to ref_transaction_commit for error
[PATCH v10 16/44] sequencer.c: use ref transactions for all ref updates
Change to use ref transactions for all updates to refs.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
sequencer.c | 24
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 0a80c58..b047fb8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -272,23 +272,31 @@ static int error_dirty_index(struct replay_opts *opts)
static int fast_forward_to(const unsigned char *to, const unsigned char *from,
int unborn, struct replay_opts *opts)
{
- struct ref_lock *ref_lock;
+ struct ref_transaction *transaction;
struct strbuf sb = STRBUF_INIT;
- int ret;
+ struct strbuf err = STRBUF_INIT;
read_cache();
if (checkout_fast_forward(from, to, 1))
exit(1); /* the callee should have complained already */
- ref_lock = lock_any_ref_for_update(HEAD, unborn ? null_sha1 : from,
- 0, NULL);
- if (!ref_lock)
- return error(_(Failed to lock HEAD during fast_forward_to));
strbuf_addf(sb, %s: fast-forward, action_name(opts));
- ret = write_ref_sha1(ref_lock, to, sb.buf);
+
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, HEAD, to, from,
+ 0, !unborn, err) ||
+ (ref_transaction_commit(transaction, sb.buf, err)
+!(transaction = NULL))) {
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(sb);
+ strbuf_release(err);
+ return -1;
+ }
strbuf_release(sb);
- return ret;
+ return 0;
}
static int do_recursive_merge(struct commit *base, struct commit *next,
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 18/44] branch.c: use ref transaction for all ref updates
Change create_branch to use a ref transaction when creating the new branch.
ref_transaction_create will check that the ref does not already exist and fail
otherwise meaning that we no longer need to keep a lock on the ref during the
setup_tracking. This simplifies the code since we can now do the transaction
in one single step.
If the forcing flag is false then use ref_transaction_create since this will
fail if the ref already exist. Otherwise use ref_transaction_update.
This also fixes a race condition in the old code where two concurrent
create_branch could race since the lock_any_ref_for_update/write_ref_sha1
did not protect against the ref already existing. I.e. one thread could end up
overwriting a branch even if the forcing flag is false.
Reviewed-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 29 +++--
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/branch.c b/branch.c
index 660097b..8bf7588 100644
--- a/branch.c
+++ b/branch.c
@@ -226,7 +226,6 @@ void create_branch(const char *head,
int force, int reflog, int clobber_head,
int quiet, enum branch_track track)
{
- struct ref_lock *lock = NULL;
struct commit *commit;
unsigned char sha1[20];
char *real_ref, msg[PATH_MAX + 20];
@@ -285,15 +284,6 @@ void create_branch(const char *head,
die(_(Not a valid branch point: '%s'.), start_name);
hashcpy(sha1, commit-object.sha1);
- if (!dont_change_ref) {
- lock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);
- if (!lock)
- die_errno(_(Failed to lock ref for update));
- }
-
- if (reflog)
- log_all_ref_updates = 1;
-
if (forcing)
snprintf(msg, sizeof msg, branch: Reset to %s,
start_name);
@@ -301,13 +291,24 @@ void create_branch(const char *head,
snprintf(msg, sizeof msg, branch: Created from %s,
start_name);
+ if (reflog)
+ log_all_ref_updates = 1;
+
+ if (!dont_change_ref) {
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
+
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, sha1,
+ null_sha1, 0, !forcing, err) ||
+ ref_transaction_commit(transaction, msg, err))
+ die(%s, err.buf);
+ }
+
if (real_ref track)
setup_tracking(ref.buf + 11, real_ref, track, quiet);
- if (!dont_change_ref)
- if (write_ref_sha1(lock, sha1, msg) 0)
- die_errno(_(Failed to write ref));
-
strbuf_release(ref);
free(real_ref);
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 21/44] refs.c: ref_transaction_commit should not free the transaction
Change ref_transaction_commit so that it does not free the transaction.
Instead require that a caller will end a transaction by calling
ref_transaction_free.
By having the transaction object remaining valid after _commit returns allows
us to write much nicer code and still be able to call ref_transaction_free
safely. Instead of this horribleness
t = ref_transaction_begin();
if ((!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
!!oldval)) ||
(ref_transaction_commit(t, action, err) !(t = NULL))) {
ref_transaction_free(t);
we can now just do the much nicer
t = ref_transaction_begin();
if (!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
!!oldval) ||
ref_transaction_commit(t, action, err)) {
ref_transaction_free(t);
... die/return ...
ref_transaction_free(transaction);
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
branch.c | 1 +
builtin/commit.c | 1 +
builtin/replace.c| 1 +
builtin/tag.c| 1 +
builtin/update-ref.c | 1 +
fast-import.c| 4 ++--
refs.c | 12 +---
refs.h | 8 +++-
sequencer.c | 4 ++--
9 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/branch.c b/branch.c
index 8bf7588..f78a28b 100644
--- a/branch.c
+++ b/branch.c
@@ -304,6 +304,7 @@ void create_branch(const char *head,
null_sha1, 0, !forcing, err) ||
ref_transaction_commit(transaction, msg, err))
die(%s, err.buf);
+ ref_transaction_free(transaction);
}
if (real_ref track)
diff --git a/builtin/commit.c b/builtin/commit.c
index c429216..6b888f2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1726,6 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
rollback_index_files();
die(%s, err.buf);
}
+ ref_transaction_free(transaction);
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/replace.c b/builtin/replace.c
index e8932cd..af7f72d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -164,6 +164,7 @@ static int replace_object_sha1(const char *object_ref,
ref_transaction_commit(transaction, NULL, err))
die(%s, err.buf);
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/builtin/tag.c b/builtin/tag.c
index b05f9a5..30b471c 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -708,6 +708,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
0, !is_null_sha1(prev), err) ||
ref_transaction_commit(transaction, NULL, err))
die(%s, err.buf);
+ ref_transaction_free(transaction);
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index cdb71a8..4c2145e 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -373,6 +373,7 @@ int cmd_update_ref(int argc, const char **argv, const char
*prefix)
update_refs_stdin();
if (ref_transaction_commit(transaction, msg, err))
die(%s, err.buf);
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/fast-import.c b/fast-import.c
index 60d4538..cb3f5af 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1710,13 +1710,13 @@ static int update_branch(struct branch *b)
if (!transaction ||
ref_transaction_update(transaction, b-name, b-sha1, old_sha1,
0, 1, err) ||
- (ref_transaction_commit(transaction, msg, err)
-!(transaction = NULL))) {
+ ref_transaction_commit(transaction, msg, err)) {
ref_transaction_free(transaction);
error(%s, err.buf);
strbuf_release(err);
return -1;
}
+ ref_transaction_free(transaction);
return 0;
}
diff --git a/refs.c b/refs.c
index 2c3f070..266a792 100644
--- a/refs.c
+++ b/refs.c
@@ -3446,10 +3446,10 @@ int update_ref(const char *action, const char *refname,
struct strbuf err = STRBUF_INIT;
t = ref_transaction_begin();
- if ((!t ||
+ if (!t ||
ref_transaction_update(t, refname, sha1, oldval, flags,
- !!oldval, err)) ||
- (ref_transaction_commit(t, action, err) !(t = NULL))) {
+ !!oldval, err) ||
+ ref_transaction_commit(t, action, err)) {
const char *str = update_ref
[PATCH v10 19/44] refs.c: change update_ref to use a transaction
Change the update_ref helper function to use a ref transaction internally.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 25 +
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/refs.c b/refs.c
index a588194..78312b5 100644
--- a/refs.c
+++ b/refs.c
@@ -3442,11 +3442,28 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr)
{
- struct ref_lock *lock;
- lock = update_ref_lock(refname, oldval, flags, NULL, onerr);
- if (!lock)
+ struct ref_transaction *t;
+ struct strbuf err = STRBUF_INIT;
+
+ t = ref_transaction_begin();
+ if ((!t ||
+ ref_transaction_update(t, refname, sha1, oldval, flags,
+ !!oldval, err)) ||
+ (ref_transaction_commit(t, action, err) !(t = NULL))) {
+ const char *str = update_ref failed for ref '%s': %s;
+
+ ref_transaction_free(t);
+ switch (onerr) {
+ case UPDATE_REFS_MSG_ON_ERR:
+ error(str, refname, err.buf); break;
+ case UPDATE_REFS_DIE_ON_ERR:
+ die(str, refname, err.buf); break;
+ case UPDATE_REFS_QUIET_ON_ERR: break;
+ }
+ strbuf_release(err);
return 1;
- return update_ref_write(action, refname, sha1, lock, NULL, onerr);
+ }
+ return 0;
}
static int ref_update_compare(const void *r1, const void *r2)
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 17/44] fast-import.c: change update_branch to use ref transactions
Change update_branch() to use ref transactions for updates.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
fast-import.c | 25 +++--
1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 6707a66..60d4538 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1679,39 +1679,44 @@ found_entry:
static int update_branch(struct branch *b)
{
static const char *msg = fast-import;
- struct ref_lock *lock;
+ struct ref_transaction *transaction;
unsigned char old_sha1[20];
+ struct strbuf err = STRBUF_INIT;
if (read_ref(b-name, old_sha1))
hashclr(old_sha1);
+
if (is_null_sha1(b-sha1)) {
if (b-delete)
delete_ref(b-name, old_sha1, 0);
return 0;
}
- lock = lock_any_ref_for_update(b-name, old_sha1, 0, NULL);
- if (!lock)
- return error(Unable to lock %s, b-name);
if (!force_update !is_null_sha1(old_sha1)) {
struct commit *old_cmit, *new_cmit;
old_cmit = lookup_commit_reference_gently(old_sha1, 0);
new_cmit = lookup_commit_reference_gently(b-sha1, 0);
- if (!old_cmit || !new_cmit) {
- unlock_ref(lock);
+ if (!old_cmit || !new_cmit)
return error(Branch %s is missing commits., b-name);
- }
if (!in_merge_bases(old_cmit, new_cmit)) {
- unlock_ref(lock);
warning(Not updating %s
(new tip %s does not contain %s),
b-name, sha1_to_hex(b-sha1),
sha1_to_hex(old_sha1));
return -1;
}
}
- if (write_ref_sha1(lock, b-sha1, msg) 0)
- return error(Unable to update %s, b-name);
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, b-name, b-sha1, old_sha1,
+ 0, 1, err) ||
+ (ref_transaction_commit(transaction, msg, err)
+!(transaction = NULL))) {
+ ref_transaction_free(transaction);
+ error(%s, err.buf);
+ strbuf_release(err);
+ return -1;
+ }
return 0;
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 35/44] refs.c: make delete_ref use a transaction
Change delete_ref to use a ref transaction for the deletion. At the same time
since we no longer have any callers of repack_without_ref we can now delete
this function.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 31 ++-
1 file changed, 10 insertions(+), 21 deletions(-)
diff --git a/refs.c b/refs.c
index 3a5f1e5..b16cf4c 100644
--- a/refs.c
+++ b/refs.c
@@ -2495,11 +2495,6 @@ static int repack_without_refs(const char **refnames,
int n, struct strbuf *err)
return ret;
}
-static int repack_without_ref(const char *refname)
-{
- return repack_without_refs(refname, 1, NULL);
-}
-
static int add_err_if_unremovable(const char *op, const char *file,
struct strbuf *e, int rc)
{
@@ -2543,24 +2538,18 @@ static int delete_ref_loose(struct ref_lock *lock, int
flag, struct strbuf *err)
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
- struct ref_lock *lock;
- int ret = 0, flag = 0;
+ struct ref_transaction *transaction;
- lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
- if (!lock)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_delete(transaction, refname, sha1, delopt,
+ sha1 !is_null_sha1(sha1), NULL) ||
+ ref_transaction_commit(transaction, NULL, NULL)) {
+ ref_transaction_free(transaction);
return 1;
- ret |= delete_ref_loose(lock, flag, NULL);
-
- /* removing the loose one could have resurrected an earlier
-* packed one. Also, if it was not loose we need to repack
-* without it.
-*/
- ret |= repack_without_ref(lock-ref_name);
-
- unlink_or_warn(git_path(logs/%s, lock-ref_name));
- clear_loose_ref_cache(ref_cache);
- unlock_ref(lock);
- return ret;
+ }
+ ref_transaction_free(transaction);
+ return 0;
}
/*
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 13/44] tag.c: use ref transactions when doing updates
Change tag.c to use ref transactions for all ref updates.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/tag.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index c6e8a71..b05f9a5 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -548,7 +548,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
struct strbuf ref = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
- struct ref_lock *lock;
struct create_tag_options opt;
char *cleanup_arg = NULL;
int annotate = 0, force = 0, lines = -1;
@@ -556,6 +555,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
const char *msgfile = NULL, *keyid = NULL;
struct msg_arg msg = { 0, STRBUF_INIT };
struct commit_list *with_commit = NULL;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct option options[] = {
OPT_CMDMODE('l', list, cmdmode, N_(list tag names), 'l'),
{ OPTION_INTEGER, 'n', NULL, lines, N_(n),
@@ -701,11 +702,12 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
- lock = lock_any_ref_for_update(ref.buf, prev, 0, NULL);
- if (!lock)
- die(_(%s: cannot lock the ref), ref.buf);
- if (write_ref_sha1(lock, object, NULL) 0)
- die(_(%s: cannot update the ref), ref.buf);
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref.buf, object, prev,
+ 0, !is_null_sha1(prev), err) ||
+ ref_transaction_commit(transaction, NULL, err))
+ die(%s, err.buf);
if (force !is_null_sha1(prev) hashcmp(prev, object))
printf(_(Updated tag '%s' (was %s)\n), tag,
find_unique_abbrev(prev, DEFAULT_ABBREV));
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 41/44] refs.c: add a new flag for transaction delete for refs we know are packed only
Add a new flag REF_ISPACKONLY that we can use in ref_transaction_delete.
This flag indicates that the ref does not exist as a loose ref andf only as
a packed ref. If this is the case we then change the commit code so that
we skip taking out a lock file and we skip calling delete_ref_loose.
Check for this flag and die(BUG:...) if used with _update or _create.
At the start of the transaction, before we even start locking any refs,
we add all such REF_ISPACKONLY refs to delnames so that we have a list of
all pack only refs that we will be deleting during this transaction.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/refs.c b/refs.c
index 564feb6..c5d41bb 100644
--- a/refs.c
+++ b/refs.c
@@ -33,6 +33,10 @@ static inline int bad_ref_char(int ch)
* pruned.
*/
#define REF_ISPRUNING 0x0100
+/** Deletion of a ref that only exists as a packed ref in which case we do not
+ * need to lock the loose ref during the transaction.
+ */
+#define REF_ISPACKONLY 0x0200
/*
* Try to read one refname component from the front of refname. Return
@@ -3366,6 +3370,9 @@ int ref_transaction_update(struct ref_transaction
*transaction,
if (transaction-status != REF_TRANSACTION_OPEN)
die(BUG: update on transaction that is not open);
+ if (flags REF_ISPACKONLY)
+ die(BUG: REF_ISPACKONLY can not be used with updates);
+
update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
update-flags = flags;
@@ -3392,6 +3399,9 @@ int ref_transaction_create(struct ref_transaction
*transaction,
if (transaction-status != REF_TRANSACTION_OPEN)
die(BUG: create on transaction that is not open);
+ if (flags REF_ISPACKONLY)
+ die(BUG: REF_ISPACKONLY can not be used with creates);
+
update = add_update(transaction, refname);
hashcpy(update-new_sha1, new_sha1);
@@ -3507,10 +3517,20 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
if (ret)
goto cleanup;
+ for (i = 0; i n; i++) {
+ struct ref_update *update = updates[i];
+
+ if (update-flags REF_ISPACKONLY)
+ delnames[delnum++] = update-refname;
+ }
+
/* Acquire all locks while verifying old values */
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
+ if (update-flags REF_ISPACKONLY)
+ continue;
+
update-lock = lock_ref_sha1_basic(update-refname,
(update-have_old ?
update-old_sha1 :
@@ -3548,6 +3568,9 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
for (i = 0; i n; i++) {
struct ref_update *update = updates[i];
+ if (update-flags REF_ISPACKONLY)
+ continue;
+
if (update-lock) {
ret |= delete_ref_loose(update-lock, update-type,
err);
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 23/44] fetch.c: change s_update_ref to use a ref transaction
Change s_update_ref to use a ref transaction for the ref update.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
builtin/fetch.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a93c893..8cf70cd 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -375,7 +375,7 @@ static int s_update_ref(const char *action,
{
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
- static struct ref_lock *lock;
+ struct ref_transaction *transaction;
if (dry_run)
return 0;
@@ -384,15 +384,16 @@ static int s_update_ref(const char *action,
snprintf(msg, sizeof(msg), %s: %s, rla, action);
errno = 0;
- lock = lock_any_ref_for_update(ref-name,
- check_old ? ref-old_sha1 : NULL,
- 0, NULL);
- if (!lock)
- return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
- STORE_REF_ERROR_OTHER;
- if (write_ref_sha1(lock, ref-new_sha1, msg) 0)
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_update(transaction, ref-name, ref-new_sha1,
+ ref-old_sha1, 0, check_old, NULL) ||
+ ref_transaction_commit(transaction, msg, NULL)) {
+ ref_transaction_free(transaction);
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
STORE_REF_ERROR_OTHER;
+ }
+ ref_transaction_free(transaction);
return 0;
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 26/44] fast-import.c: use a ref transaction when dumping tags
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
fast-import.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index cb3f5af..9c76c73 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1735,15 +1735,22 @@ static void dump_tags(void)
{
static const char *msg = fast-import;
struct tag *t;
- struct ref_lock *lock;
char ref_name[PATH_MAX];
+ struct strbuf err = STRBUF_INIT;
+ struct ref_transaction *transaction;
+ transaction = ref_transaction_begin();
for (t = first_tag; t; t = t-next_tag) {
- sprintf(ref_name, tags/%s, t-name);
- lock = lock_ref_sha1(ref_name, NULL);
- if (!lock || write_ref_sha1(lock, t-sha1, msg) 0)
- failure |= error(Unable to update %s, ref_name);
+ sprintf(ref_name, refs/tags/%s, t-name);
+
+ if (ref_transaction_update(transaction, ref_name, t-sha1,
+ NULL, 0, 0, err))
+ failure |= error(%s, err.buf);
}
+ if (failure || ref_transaction_commit(transaction, msg, err))
+ failure |= error(%s, err.buf);
+ ref_transaction_free(transaction);
+ strbuf_release(err);
}
static void dump_marks_helper(FILE *f,
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 44/44] refs.c: remove forward declaration of write_ref_sha1
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/refs.c b/refs.c
index 71e3059..bf84306 100644
--- a/refs.c
+++ b/refs.c
@@ -260,8 +260,6 @@ struct ref_entry {
};
static void read_loose_refs(const char *dirname, struct ref_dir *dir);
-static int write_ref_sha1(struct ref_lock *lock,
- const unsigned char *sha1, const char *logmsg);
static struct ref_dir *get_ref_dir(struct ref_entry *entry)
{
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 42/44] refs.c: pass a skip list to name_conflict_fn
Allow passing a list of refs to skip checking to name_conflict_fn.
There are some conditions where we want to allow a temporary conflict and skip
checking those refs. For example if we have a transaction that
1, guarantees that m is a packed refs and there is no loose ref for m
2, the transaction will delete m from the packed ref
3, the transaction will create conflicting m/m
For this case we want to be able to lock and create m/m since we know that the
conflict is only transient. I.e. the conflict will be automatically resolved
by the transaction when it deletes m.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 43 +--
1 file changed, 33 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index c5d41bb..3967333 100644
--- a/refs.c
+++ b/refs.c
@@ -798,11 +798,19 @@ struct name_conflict_cb {
const char *refname;
const char *oldrefname;
const char *conflicting_refname;
+ const char **skip;
+ int skipnum;
};
static int name_conflict_fn(struct ref_entry *entry, void *cb_data)
{
struct name_conflict_cb *data = (struct name_conflict_cb *)cb_data;
+ int i;
+ for(i = 0; i data-skipnum; i++) {
+ if (!strcmp(entry-name, data-skip[i])) {
+ return 0;
+ }
+ }
if (data-oldrefname !strcmp(data-oldrefname, entry-name))
return 0;
if (names_conflict(data-refname, entry-name)) {
@@ -817,15 +825,21 @@ static int name_conflict_fn(struct ref_entry *entry, void
*cb_data)
* conflicting with the name of an existing reference in dir. If
* oldrefname is non-NULL, ignore potential conflicts with oldrefname
* (e.g., because oldrefname is scheduled for deletion in the same
- * operation).
+ * operation). skip contains a list of refs we want to skip checking for
+ * conflicts with. Refs may be skipped due to us knowing that it will
+ * be deleted later during a transaction that deletes one reference and then
+ * creates a new conflicting reference. For example a rename from m to m/m.
*/
static int is_refname_available(const char *refname, const char *oldrefname,
- struct ref_dir *dir)
+ struct ref_dir *dir,
+ const char **skip, int skipnum)
{
struct name_conflict_cb data;
data.refname = refname;
data.oldrefname = oldrefname;
data.conflicting_refname = NULL;
+ data.skip = skip;
+ data.skipnum = skipnum;
sort_ref_dir(dir);
if (do_for_each_entry_in_dir(dir, 0, name_conflict_fn, data)) {
@@ -2037,7 +2051,8 @@ int dwim_log(const char *str, int len, unsigned char
*sha1, char **log)
static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
- int flags, int *type_p)
+ int flags, int *type_p,
+ const char **skip, int skipnum)
{
char *ref_file;
const char *orig_refname = refname;
@@ -2084,7 +2099,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
* name is a proper prefix of our refname.
*/
if (missing
-!is_refname_available(refname, NULL, get_packed_refs(ref_cache)))
{
+!is_refname_available(refname, NULL,
+ get_packed_refs(ref_cache),
+ skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2142,7 +2159,7 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
+ return lock_ref_sha1_basic(refname, old_sha1, flags, type_p, NULL, 0);
}
/*
@@ -2620,6 +2637,9 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
const char *symref = NULL;
+ if (!strcmp(oldrefname, newrefname))
+ return 0;
+
if (log S_ISLNK(loginfo.st_mode))
return error(reflog for %s is a symlink, oldrefname);
@@ -2630,10 +2650,12 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (!symref)
return error(refname %s not found, oldrefname);
- if (!is_refname_available(newrefname, oldrefname,
get_packed_refs(ref_cache)))
+ if (!is_refname_available(newrefname, oldrefname,
+ get_packed_refs(ref_cache), NULL, 0))
return 1;
- if (!is_refname_available(newrefname, oldrefname,
[PATCH v10 32/44] refs.c: remove the update_ref_write function
Since we only call update_ref_write from a single place and we only call it
with onerr==QUIET_ON_ERR we can just as well get rid of it and just call
write_ref_sha1 directly.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 35 +--
1 file changed, 9 insertions(+), 26 deletions(-)
diff --git a/refs.c b/refs.c
index 04de777..b1acf51 100644
--- a/refs.c
+++ b/refs.c
@@ -3283,25 +3283,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
}
-static int update_ref_write(const char *action, const char *refname,
- const unsigned char *sha1, struct ref_lock *lock,
- struct strbuf *err, enum action_on_err onerr)
-{
- if (write_ref_sha1(lock, sha1, action) 0) {
- const char *str = Cannot update the ref '%s'.;
- if (err)
- strbuf_addf(err, str, refname);
-
- switch (onerr) {
- case UPDATE_REFS_MSG_ON_ERR: error(str, refname); break;
- case UPDATE_REFS_DIE_ON_ERR: die(str, refname); break;
- case UPDATE_REFS_QUIET_ON_ERR: break;
- }
- return 1;
- }
- return 0;
-}
-
/**
* Information needed for a single ref update. Set new_sha1 to the
* new value or to zero to delete the ref. To check the old value
@@ -3539,14 +3520,16 @@ int ref_transaction_commit(struct ref_transaction
*transaction,
struct ref_update *update = updates[i];
if (!is_null_sha1(update-new_sha1)) {
- ret = update_ref_write(msg,
- update-refname,
- update-new_sha1,
- update-lock, err,
- UPDATE_REFS_QUIET_ON_ERR);
- update-lock = NULL; /* freed by update_ref_write */
- if (ret)
+ ret = write_ref_sha1(update-lock, update-new_sha1,
+msg);
+ update-lock = NULL; /* freed by write_ref_sha1 */
+ if (ret) {
+ const char *str = Cannot update the ref '%s'.;
+
+ if (err)
+ strbuf_addf(err, str, update-refname);
goto cleanup;
+ }
}
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 43/44] refs.c: make rename_ref use a transaction
Change rename_ref to use a single transaction to perform the ref rename.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 73 ++
1 file changed, 20 insertions(+), 53 deletions(-)
diff --git a/refs.c b/refs.c
index 3967333..71e3059 100644
--- a/refs.c
+++ b/refs.c
@@ -2630,9 +2630,10 @@ static int rename_tmp_log(const char *newrefname)
int rename_ref(const char *oldrefname, const char *newrefname, const char
*logmsg)
{
- unsigned char sha1[20], orig_sha1[20];
- int flag = 0, logmoved = 0;
- struct ref_lock *lock;
+ unsigned char sha1[20];
+ int flag = 0;
+ struct ref_transaction *transaction;
+ struct strbuf err = STRBUF_INIT;
struct stat loginfo;
int log = !lstat(git_path(logs/%s, oldrefname), loginfo);
const char *symref = NULL;
@@ -2643,7 +2644,7 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (log S_ISLNK(loginfo.st_mode))
return error(reflog for %s is a symlink, oldrefname);
- symref = resolve_ref_unsafe(oldrefname, orig_sha1, 1, flag);
+ symref = resolve_ref_unsafe(oldrefname, sha1, 1, flag);
if (flag REF_ISSYMREF)
return error(refname %s is a symbolic ref, renaming it is not
supported,
oldrefname);
@@ -2665,62 +2666,28 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
return error(unable to pack refs);
- if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
- error(unable to delete old %s, oldrefname);
- goto rollback;
- }
-
- if (!read_ref_full(newrefname, sha1, 1, NULL)
- delete_ref(newrefname, sha1, REF_NODEREF)) {
- if (errno==EISDIR) {
- if (remove_empty_directories(git_path(%s,
newrefname))) {
- error(Directory not empty: %s, newrefname);
- goto rollback;
- }
- } else {
- error(unable to delete existing %s, newrefname);
- goto rollback;
- }
+ transaction = ref_transaction_begin();
+ if (!transaction ||
+ ref_transaction_delete(transaction, oldrefname, sha1,
+ REF_NODEREF | REF_ISPACKONLY,
+ 1, NULL, err) ||
+ ref_transaction_update(transaction, newrefname, sha1,
+ NULL, 0, 0, logmsg, err) ||
+ ref_transaction_commit(transaction, err)) {
+ ref_transaction_free(transaction);
+ error(rename_ref failed: %s, err.buf);
+ strbuf_release(err);
+ goto rollbacklog;
}
+ ref_transaction_free(transaction);
if (log rename_tmp_log(newrefname))
- goto rollback;
-
- logmoved = log;
-
- lock = lock_ref_sha1_basic(newrefname, NULL, 0, NULL, NULL, 0);
- if (!lock) {
- error(unable to lock %s for update, newrefname);
- goto rollback;
- }
- lock-force_write = 1;
- hashcpy(lock-old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
- error(unable to write current sha1 into %s, newrefname);
- goto rollback;
- }
-
- return 0;
-
- rollback:
- lock = lock_ref_sha1_basic(oldrefname, NULL, 0, NULL, NULL, 0);
- if (!lock) {
- error(unable to lock %s for rollback, oldrefname);
goto rollbacklog;
- }
- lock-force_write = 1;
- flag = log_all_ref_updates;
- log_all_ref_updates = 0;
- if (write_ref_sha1(lock, orig_sha1, NULL))
- error(unable to write current sha1 into %s, oldrefname);
- log_all_ref_updates = flag;
+ return 0;
rollbacklog:
- if (logmoved rename(git_path(logs/%s, newrefname),
git_path(logs/%s, oldrefname)))
- error(unable to restore logfile %s from %s: %s,
- oldrefname, newrefname, strerror(errno));
- if (!logmoved log
+ if (log
rename(git_path(TMP_RENAMED_LOG), git_path(logs/%s, oldrefname)))
error(unable to restore logfile %s from TMP_RENAMED_LOG: %s,
oldrefname, strerror(errno));
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 38/44] refs.c: pack all refs before we start to rename a ref
This means that most loose refs will no longer be present after the rename
which triggered a test failure since it assumes the file for an unrelated
ref would still be present after the rename.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c| 3 +++
t/t3200-branch.sh | 2 +-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/refs.c b/refs.c
index 1210345..f4899e0 100644
--- a/refs.c
+++ b/refs.c
@@ -2635,6 +2635,9 @@ int rename_ref(const char *oldrefname, const char
*newrefname, const char *logms
return error(unable to move logfile logs/%s to
TMP_RENAMED_LOG: %s,
oldrefname, strerror(errno));
+ if (pack_refs(PACK_REFS_ALL | PACK_REFS_PRUNE))
+ return error(unable to pack refs);
+
if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {
error(unable to delete old %s, oldrefname);
goto rollback;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ac31b71..fafd38a 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -289,7 +289,7 @@ test_expect_success 'renaming a symref is not allowed' '
git symbolic-ref refs/heads/master2 refs/heads/master
test_must_fail git branch -m master2 master3
git symbolic-ref refs/heads/master2
- test_path_is_file .git/refs/heads/master
+ test_path_is_missing .git/refs/heads/master
test_path_is_missing .git/refs/heads/master3
'
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 33/44] refs.c: remove lock_ref_sha1
lock_ref_sha1 was only called from one place in refc.c and only provided
a check that the refname was sane before adding back the initial refs/
part of the ref path name, the initial refs/ that this caller had already
stripped off before calling lock_ref_sha1.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/refs.c b/refs.c
index b1acf51..69623e4 100644
--- a/refs.c
+++ b/refs.c
@@ -2126,15 +2126,6 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
return NULL;
}
-static struct ref_lock *lock_ref_sha1(const char *refname, const unsigned char
*old_sha1)
-{
- char refpath[PATH_MAX];
- if (check_refname_format(refname, 0))
- return NULL;
- strcpy(refpath, mkpath(refs/%s, refname));
- return lock_ref_sha1_basic(refpath, old_sha1, 0, NULL);
-}
-
struct ref_lock *lock_any_ref_for_update(const char *refname,
const unsigned char *old_sha1,
int flags, int *type_p)
@@ -2339,8 +2330,12 @@ static void try_remove_empty_parents(char *name)
/* make sure nobody touched the ref, and unlink */
static void prune_ref(struct ref_to_prune *r)
{
- struct ref_lock *lock = lock_ref_sha1(r-name + 5, r-sha1);
+ struct ref_lock *lock;
+
+ if (check_refname_format(r-name + 5, 0))
+ return;
+ lock = lock_ref_sha1_basic(r-name, r-sha1, 0, NULL);
if (lock) {
unlink_or_warn(git_path(%s, r-name));
unlock_ref(lock);
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 39/44] refs.c: move the check for valid refname to lock_ref_sha1_basic
Move the check for check_refname_format from lock_any_ref_for_update
to lock_ref_sha1_basic. At some later stage we will get rid of
lock_any_ref_for_update completely.
This leaves lock_any_ref_for_updates as a no-op wrapper which could be removed.
But this wrapper is also called from an external caller and we will soon
make changes to the signature to lock_ref_sha1_basic that we do not want to
expose to that caller.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
refs.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/refs.c b/refs.c
index f4899e0..f63a356 100644
--- a/refs.c
+++ b/refs.c
@@ -2044,6 +2044,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char
*refname,
int missing = 0;
int attempts_remaining = 3;
+ if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
+ return NULL;
+
lock = xcalloc(1, sizeof(struct ref_lock));
lock-lock_fd = -1;
@@ -2135,8 +2138,6 @@ struct ref_lock *lock_any_ref_for_update(const char
*refname,
const unsigned char *old_sha1,
int flags, int *type_p)
{
- if (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))
- return NULL;
return lock_ref_sha1_basic(refname, old_sha1, flags, type_p);
}
--
2.0.0.rc3.510.g20c254b
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
