git log -z doesn't separate commits with NULs
Hello all.
I wanted to generate several files with some statistics using "git log -z" command. I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.sh
If I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.
I'm newbie to xargs, thus I tested with and that worked as I expected. find . -type f -print0 | xargs -0 ./1.sh
That produced a lost of "started" lines.
Thus I suspect there is a but in git log -z command and that doesn't "Separate the commits with NULs instead of with new newlines." as promised in the documents. Is my understanding correct or I don't understand the documentation or somehow pass wrong parameters into git log?
Thank you.
Best regards,
Nikolaj
Quoted 5 lines · Nikolaj Shurkaev
I wanted to generate several files with some statistics using "git log -z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.sh
I'm not sure what "1.sh" is expecting to take as input, but that will feed entire commits, including their commit message and entire diff, to the script on its command line.
That seems like an awkward interface, but we don't really know what your script intends to do. Maybe it is worth sharing the contents of the script.
Quoted 2 lines
If I put echo "started" into the file ~/1.sh I see that the file is called only once instead of multiple times.
Yes. The point of xargs is usually to cram as many arguments into each invocation of "1.sh" as possible, splitting into multiple invocations only when we hit the argument-list memory limit that the OS imposes.
If you want xargs to give each argument its own invocation of the script, use "xargs -n1".
Quoted 3 lines
I'm newbie to xargs, thus I tested with and that worked as I expected. find . -type f -print0 | xargs -0 ./1.sh
That produced a lost of "started" lines.
If you instrument your 1.sh more[1], you will find that is not executing once per file, but rather getting a large chunk of files per invocation.
[1] Try adding: echo "got args: $*"
Quoted 3 lines
Thus I suspect there is a but in git log -z command and that doesn't "Separate the commits with NULs instead of with new newlines." as promised in the documents.
You could verify that assertion by looking at the output. Try piping your "git log" command through "cat -A | less". When I try it, I see a NUL between each commit (cat -A will show it as "^@").
-Peff
Hello.
Thank you very much for your tips. They really helped me. I was trying to create patches that would affect only some given files or folders. By this moment I have the following:
GeneratePatches.sh
---------------------
#!/bin/bash
#parameter 1 - <since>..<to>
#parameter 2 - path to file
git log -z --reverse --format=email --patch "$1" -- "$2" | xargs --null --max-args=1 ./CreatePatchFile.sh
---------------------
and CreatePatchFile.sh
---------------------
#!/bin/bash
myPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l) let "myPatchNumber += 1"
patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch" echo "$@" > "$patchFile"
---------------------
I call
./GeneratePatches.sh HEAD~3..HEAD SomePath
and that produces something very similar to what I want.
Perhaps there is a better way to do that.
Thank you once again.
---
Best regards,
Nikolaj
23.02.2012 13:24, Jeff King пишет:
Quoted 41 lines · Nikolaj Shurkaev
Quoted 6 lines
I wanted to generate several files with some statistics using "git log -z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shI'm not sure what "1.sh" is expecting to take as input, but that will feed entire commits, including their commit message and entire diff, to the script on its command line.
That seems like an awkward interface, but we don't really know what your script intends to do. Maybe it is worth sharing the contents of the script.
Quoted 2 lines
If I put echo "started" into the file ~/1.sh I see that the file is called only once instead of multiple times.
Yes. The point of xargs is usually to cram as many arguments into each invocation of "1.sh" as possible, splitting into multiple invocations only when we hit the argument-list memory limit that the OS imposes.
If you want xargs to give each argument its own invocation of the script, use "xargs -n1".
Quoted 3 lines
I'm newbie to xargs, thus I tested with and that worked as I expected. find . -type f -print0 | xargs -0 ./1.sh
That produced a lost of "started" lines.If you instrument your 1.sh more[1], you will find that is not executing once per file, but rather getting a large chunk of files per invocation.
[1] Try adding: echo "got args: $*"
Quoted 3 lines
Thus I suspect there is a but in git log -z command and that doesn't "Separate the commits with NULs instead of with new newlines." as promised in the documents.
You could verify that assertion by looking at the output. Try piping your "git log" command through "cat -A | less". When I try it, I see a NUL between each commit (cat -A will show it as "^@").
-Peff
Quoted 24 lines · Nikolaj Shurkaev
Thank you very much for your tips. They really helped me. I was trying to create patches that would affect only some given files or folders. By this moment I have the following:
GeneratePatches.sh
---------------------
#!/bin/bash
#parameter 1 - <since>..<to>
#parameter 2 - path to file
git log -z --reverse --format=email --patch "$1" -- "$2" | xargs --null --max-args=1 ./CreatePatchFile.sh
---------------------and
CreatePatchFile.sh
---------------------
#!/bin/bashmyPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l) let "myPatchNumber += 1"
patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch" echo "$@" > "$patchFile"
---------------------
Quoted 5 lines
I call
./GeneratePatches.sh HEAD~3..HEAD SomePath
and that produces something very similar to what I want.Perhaps there is a better way to do that.
So what git-format-patch is lacking?
Signature · 1 line
Jakub Narebski
For example there are commits that affect not only files in folder A but files in folder B, C and so on. If I do format-patch that will give me nice patches, but there are modifications of folders B, C and so on there. I do not know a way to generate patches via format-patch that affect only files in folder A.
This is why I wrote those scripts.
23.02.2012 16:15, Jakub Narebski пишет:
Quoted 33 lines
Quoted 24 lines · Nikolaj Shurkaev
Thank you very much for your tips. They really helped me. I was trying to create patches that would affect only some given files or folders. By this moment I have the following:
GeneratePatches.sh
---------------------
#!/bin/bash
#parameter 1 -<since>..<to>
#parameter 2 - path to file
git log -z --reverse --format=email --patch "$1" -- "$2" | xargs --null --max-args=1 ./CreatePatchFile.sh
---------------------and
CreatePatchFile.sh
---------------------
#!/bin/bashmyPatchNumber=$(ls ./*-patch.patch 2>/dev/null | wc -l) let "myPatchNumber += 1"
patchFile="./"$(printf "%04d" $myPatchNumber)"-patch.patch" echo "$@"> "$patchFile"
---------------------Quoted 5 lines
I call
./GeneratePatches.sh HEAD~3..HEAD SomePath
and that produces something very similar to what I want.Perhaps there is a better way to do that.
So what git-format-patch is lacking?
Quoted 5 lines · Nikolaj Shurkaev
For example there are commits that affect not only files in folder A but files in folder B, C and so on. If I do format-patch that will give me nice patches, but there are modifications of folders B, C and so on there. I do not know a way to generate patches via format-patch that affect only files in folder A.
Doesn't:
git format-patch HEAD~3..HEAD SomePath
do what you want? It is certainly designed to, and it seems to work for me.
-Peff
Quoted 8 lines · Jeff King
On Thu, Feb 23, 2012 at 04:48:43PM +0300, Nikolaj Shurkaev wrote:
Doesn't:
git format-patch HEAD~3..HEAD SomePath
do what you want? It is certainly designed to, and it seems to work for me.
It is not quite "designed to", though.
It happens to work that way, and I do not think we want to forbid its use, but we would want to discourage anybody from blindly using it without thinking if the end results suits his/her purpose (and the reason should be obvious to those who think, the hint is "log message").
Hello.
Thank you for the hint.
git format-patch HEAD~3..HEAD -- SomePath
does exactly what I need. But that way of usage is not described in git documentation thus I thought there is no way to do that. I've just double checked
git format-patch --help
doesn't describe that. I'll propose to put something like this into git documentation
--------------------------------------------------------------------------------------------
Documentation/git-format-patch.txt +0 -0
@@ -23,6 +23,7 @@ SYNOPSIS
[--cover-letter] [--quiet]
[<common diff options>]
[ <since> | <revision range> ]
+ [[\--] <path>...]
DESCRIPTION
-----------
@@ -219,6 +220,12 @@ you can use --suffix=-patch to get 0001-description-of-my-change-patch. range are always formatted as creation patches, independently
of this flag.
+[\--] <path>...::
+ Put in patches only those modifications that affect specified files
+ and folders. It's important to understand that log message of the
+ commit may become inappropriate because some parts of patch may be
+ cut off.
+
CONFIGURATION
-------------
You can specify extra mail header lines to be added to each message,
--------------------------------------------------------------------------------------------
--
Nikolaj.
23.02.2012 23:07, Junio C Hamano пишет:
Quoted 17 lines
Quoted 8 lines · Jeff King
On Thu, Feb 23, 2012 at 04:48:43PM +0300, Nikolaj Shurkaev wrote:
Doesn't:
git format-patch HEAD~3..HEAD SomePath
do what you want? It is certainly designed to, and it seems to work for me.
It is not quite "designed to", though.
It happens to work that way, and I do not think we want to forbid its use, but we would want to discourage anybody from blindly using it without thinking if the end results suits his/her purpose (and the reason should be obvious to those who think, the hint is "log message").
Quoted 20 lines · Nikolaj Shurkaev
I'll propose to put something like this into git documentation
--------------------------------------------------------------------------------------------Documentation/git-format-patch.txt +0
-0@@ -23,6 +23,7 @@ SYNOPSIS[--cover-letter] [--quiet]
[<common diff options>]
[ <since> | <revision range> ]
+ [[\--] <path>...]DESCRIPTION
-----------Documentation/git-format-patch.txt +0
-0@@ -219,6 +220,12 @@ you can use `--suffix=-patch` to get
0001-description-of-my-change-patch.
range are always formatted as creation patches, independently of this flag.
Quoted 6 lines
+[\--] <path>...::
+ Put in patches only those modifications that affect specified files
+ and folders. It's important to understand that log message of the
+ commit may become inappropriate because some parts of patch may be
+ cut off.
+
I think that text looks OK. But to my mind, it is not that format-patch accepts a path parameter, but rather that it takes arbitrary log-like arguments. So you could do "git format-patch --grep=whatever", or even something like "git format-patch --cherry".
I don't know how well tested every option is, though, so maybe it's not a good idea to encourage the use of random options.
-Peff
Quoted 7 lines · Jeff King
Quoted 7 lines
DESCRIPTION
-----------(quoted diff) +0
-0@@ -219,6 +220,12 @@ you can use `--suffix=-patch` to get
0001-description-of-my-change-patch.
range are always formatted as creation patches, independently of this flag.
Quoted 10 lines
Quoted 6 lines
+[\--] <path>...::
+ Put in patches only those modifications that affect specified files
+ and folders. It's important to understand that log message of the
+ commit may become inappropriate because some parts of patch may be
+ cut off.
+I think that text looks OK. But to my mind, it is not that format-patch accepts a path parameter, but rather that it takes arbitrary log-like arguments.
The above text is not telling the entire truth, though.
When the command is run with the "--full-diff" option, seleted commits will be shown in full. This is useful for example when you want to pick commits that add a new "frotz" driver, which obviously needs to include "drivers/frotz/" subdirectory, without missing necessary changes to the Makefiles in the higher level (e.g. "drivers/Makefile"), e.g.
git format-patch --full-diff v1.0..v1.1 -- drivers/frotz
In such a case, "some parts may be cut off" does not make the log message inappropriate.
On the other hand, people often do not use the resulting history of taking partial patches (i.e. without --full-diff) and feeding them to "am" as-is. The operation is used merely to give them a starting point for working on (possibly) an unrelated topic, and the history is further tweaked with "rebase -i" or even "commit --amend". It is not "inappropriate" that the log says more than what the patch does in such a use case. What the log says is irrelevant.
Quoted 2 lines
I don't know how well tested every option is, though, so maybe it's not a good idea to encourage the use of random options.
I obviously agree and also suspect that the real question is not "how well tested" but "if it makes sense".
I am reasonably sure that over time the options and features that make sense in the context of producing something that is useful with "am" have been already made to work well, but I also am fairly certain that the coverage of the code to explicitly reject options that do not make sense in that context would be spotty at best. For example, did we carefully design and implement how format-patch should behave when "-z" is given, or does the code happen to do whatever it happens to do? If the latter, did we consider rejecting "-z" when given from the command line and implement such safety?
Quoted 8 lines · Junio C Hamano
The above text is not telling the entire truth, though.
When the command is run with the "--full-diff" option, seleted commits will be shown in full. This is useful for example when you want to pick commits that add a new "frotz" driver, which obviously needs to include "
drivers/frotz/" subdirectory, without missing necessary changes to the Makefiles in the higher level (e.g. "drivers/Makefile"), e.g.
Quoted 4 lines
git format-patch --full-diff v1.0..v1.1 -- drivers/frotzIn such a case, "some parts may be cut off" does not make the log message inappropriate.
True. That is also a slightly dangerous thing to do, though, because you are omitting full patches in the middle that touch the same paths as the patches you include. So I might send you a patch against Makefile that does not apply, and nor do you have the interim sha1.
Of course that is a general problem with any commit-limiting in format-patch (e.g., --grep), and even with sending patches in general (I have to make sure I based my patch off of something reasonable in the first place). The key is to be clueful about what you are doing. So perhaps we are better off to refer the user to git-log(1), say that commit limiting options in general would work, but be careful with sending a partial result.
Quoted 7 lines
On the other hand, people often do not use the resulting history of taking partial patches (i.e. without --full-diff) and feeding them to "am" as-is. The operation is used merely to give them a starting point for working on (possibly) an unrelated topic, and the history is further tweaked with "rebase -i" or even "commit --amend". It is not "inappropriate" that the log says more than what the patch does in such a use case. What the log says is irrelevant.
Right. I think this comes down to the "clueful" bit. If you understand what it is you are asking git to do and deciding that the consequences are OK (either because you are not using the log message, or you know that the subset of a series you are sending should apply to what the receiver has).
Quoted 6 lines
Quoted 2 lines
I don't know how well tested every option is, though, so maybe it's not a good idea to encourage the use of random options.
I obviously agree and also suspect that the real question is not "how well tested" but "if it makes sense".
Quoted 9 lines
I am reasonably sure that over time the options and features that make sense in the context of producing something that is useful with "am" have been already made to work well, but I also am fairly certain that the coverage of the code to explicitly reject options that do not make sense in that context would be spotty at best. For example, did we carefully design and implement how format-patch should behave when "-z" is given, or does the code happen to do whatever it happens to do? If the latter, did we consider rejecting "-z" when given from the command line and implement such safety?
Yeah, I think that is a good way of putting it. I tend to think the commit-limiting options are the useful and working ones, which is why I suggested mentioning them explicitly above. But I admit I don't use them myself, so I'm just guessing.
-Peff
Quoted 7 lines · Jeff King
True. That is also a slightly dangerous thing to do, though, because you are omitting full patches in the middle that touch the same paths as the patches you include....
... So
perhaps we are better off to refer the user to git-log(1), say that commit limiting options in general would work, but be careful with sending a partial result.
You seem to have spelled out everything I originally wrote in my reply that I later deleted before sending it out, and I think the reason that brought you to the three-line conclusion is the same one that made me I delete them ;-).
Using a partial patch essentially has the same risk as cherry-picking a commit into different context, and it is a more generic issue that this particular manual page should not waste tons of space to teach readers about. I think "Be careful and clueful" is sufficient and the best we can do without writing a textbook on distributed software development disciplines.
Quoted 3 lines
... I tend to think the
commit-limiting options are the useful and working ones, which is why I suggested mentioning them explicitly above.
I think I agree that is a sane thing to do.
Quoted 8 lines · Junio C Hamano
Quoted 7 lines
True. That is also a slightly dangerous thing to do, though, because you are omitting full patches in the middle that touch the same paths as the patches you include....
... So
perhaps we are better off to refer the user to git-log(1), say that commit limiting options in general would work, but be careful with sending a partial result.
Quoted 4 lines
You seem to have spelled out everything I originally wrote in my reply that I later deleted before sending it out, and I think the reason that brought you to the three-line conclusion is the same one that made me I delete them ;-).
OK, good. :)
Nikolaj, have you followed all of this? Do you want to try to improve your patch in this direction?
-Peff
As far as I understood from what I read and from a brief look at source code of git (builtin/log.c file) the best what we can do is just mention that a lot of log options are applicable and warn user to use them with understanding of the consequences. I do not think that documentation of some of options in Documentation/git-format-patch.txt makes sense because some of the options are connected one with another. For example --full-diff and <path> are connected as Junio C Hamano wrote. And that dependency is already explained in Documentation/git-log.txt.
Another option that I see is to put git-log options description into a separate file and include that from git-log.txt and from git-format-patch.txt like that is done with rev-list-options.txt. That could be useful in a long term. Because new options may appear or some may disappear. Some dependency between options may be documented. However for me that would require to get better understanding how all that documentation is built. And that could require modifications of git-log help also.
Please let me know how to proceed with that properly. Perhaps I should start a separate discussion thread devoted to the documentation enhancement. Does thread subject matter?
Thank you.
----------------------
Documentation/git-format-patch.txt +9 -0
@@ -68,6 +68,15 @@ If given `--thread`, `git-format-patch` will generate `In-Reply-To` and as replies to the first mail; this also generates a `Message-Id` header to reference. +NOTES +----- + +`git format-patch` and `git log` share significant part of their +implementations. As a result a lot of options available and described for +`git log` will work for `git format-patch` also. However it's required to +use them carefully with understanding of the consequences. For example +applying `<path>` option may make log messages irrelevant. + OPTIONS ------- :git-format-patch: 1
25.02.2012 0:16, Jeff King пишет:
Quoted 20 lines · Junio C Hamano
Quoted 8 lines
Quoted 8 lines
True. That is also a slightly dangerous thing to do, though, because you are omitting full patches in the middle that touch the same paths as the patches you include....
... So
perhaps we are better off to refer the user to git-log(1), say that commit limiting options in general would work, but be careful with sending a partial result.Quoted 4 lines
You seem to have spelled out everything I originally wrote in my reply that I later deleted before sending it out, and I think the reason that brought you to the three-line conclusion is the same one that made me I delete them ;-).
OK, good. :)
Nikolaj, have you followed all of this? Do you want to try to improve your patch in this direction?
-Peff
Quoted 15 lines · Junio C Hamano
Quoted 7 lines · Jeff King
True. That is also a slightly dangerous thing to do, though, because you are omitting full patches in the middle that touch the same paths as the patches you include....
... So
perhaps we are better off to refer the user to git-log(1), say that commit limiting options in general would work, but be careful with sending a partial result.You seem to have spelled out everything I originally wrote in my reply that I later deleted before sending it out, and I think the reason that brought you to the three-line conclusion is the same one that made me I delete them ;-).
Quoted 6 lines
Using a partial patch essentially has the same risk as cherry-picking a commit into different context, and it is a more generic issue that this particular manual page should not waste tons of space to teach readers about. I think "Be careful and clueful" is sufficient and the best we can do without writing a textbook on distributed software development disciplines.
Perhaps git-format-patch should mention that it was created with path-limited patch in some email pseudo-header like X-Pathspec:
or something, don't you think?
Signature · 2 lines
Jakub Narebski Poland
Quoted 3 lines · Jakub Narebski
Perhaps git-format-patch should mention that it was created with path-limited patch in some email pseudo-header like X-Pathspec:
or something, don't you think?
What kind of workflow are you assuming, and who would benefit from such a header under that assumption?
It obviously would not help the person who is running format-patch, as he is very well aware that he is giving a pathspec when he runs the command. If the result is used privately to prepare a starting point of possibly unrelated work, it does not matter.
If you are assuming a workflow that involves a public review of mailed patches, it is very likely that such a header will be lost unless you are using git-send-email, as we strongly discourage copying and pasting the entire thing in the message body. Anybody sitting on the receiving end worth her salt would judge the submission by looking at the log message, diffstat and the patch, and the sender having used pathspec to format the patch would not be a reason for rejection at all---the quality of the submission is.
So offhand, I do not know under what workflow such an extra header would benefit people in what position in the workflow.
Quoted 17 lines · Nikolaj Shurkaev
Hello all.
I wanted to generate several files with some statistics using "git log -z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shIf I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.I'm newbie to xargs, thus I tested with and that worked as I expected. find . -type f -print0 | xargs -0 ./1.sh
That produced a lost of "started" lines.
Quoted 5 lines
Thus I suspect there is a but in git log -z command and that doesn't "Separate the commits with NULs instead of with new newlines." as promised in the documents.
Is my understanding correct or I don't understand the documentation or somehow pass wrong parameters into git log?
Just a guess, but doesn't the "--patch" option to git log ask it to produce a patch output? Surely that will override the -z: patch will not be expecting NULs.
Quoted 8 lines
Thank you.
Best regards,
Nikolaj
--
To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Luke Diamand
Quoted 5 lines
Thus I suspect there is a but in git log -z command and that doesn't "Separate the commits with NULs instead of with new newlines." as promised in the documents.
Is my understanding correct or I don't understand the documentation or somehow pass wrong parameters into git log?Just a guess, but doesn't the "--patch" option to git log ask it to produce a patch output? Surely that will override the -z: patch will not be expecting NULs.
No. You will get the patch text and the log message together, with commits separated by NUL. Some diff output formats will also respect "-z" to produce NULs internally (e.g., "--raw" will use it to separate filenames), but "--patch" does not.
-Peff
Am 23.02.2012 10:14, schrieb Nikolaj Shurkaev:
Quoted 10 lines
I wanted to generate several files with some statistics using "git log -z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shIf I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.
That is because xargs calls the program with as many arguments as possible, unless directed otherwise. Put this in the script:
echo "started $*"
and repeat. Then try this:
... | xargs -0 -n 1 ~/1.sh
-- Hannes
Hello.
You were right. I added parameter --max-args=1 to xargs command and that started work as I wanted initially. Thank you very much.
--
Nikolaj
23.02.2012 13:17, Johannes Sixt пишет:
Quoted 22 lines
Am 23.02.2012 10:14, schrieb Nikolaj Shurkaev:
Quoted 10 lines
I wanted to generate several files with some statistics using "git log -z" command.
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shIf I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.That is because xargs calls the program with as many arguments as possible, unless directed otherwise. Put this in the script:
echo "started $*"
and repeat. Then try this:
... | xargs -0 -n 1 ~/1.sh
-- Hannes
Quoted 8 lines · Nikolaj Shurkaev
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shIf I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.
If you want the command to be called once for each commit you need to pass --max-args=1 to xargs. Otherwise xargs will cumulate the arguments until --max-chars is reached, and the command is expected to loop over them.
Andreas.
Signature · 3 lines
Andreas Schwab, [email protected] GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Thank you.
That really helped me.
---
Nikolaj
23.02.2012 13:35, Andreas Schwab пишет:
Quoted 17 lines
Quoted 8 lines · Nikolaj Shurkaev
I did something like this:
git log -z --patch HEAD~10..HEAD -- SomePathHere | xargs -0 --max-chars=1000000 ~/1.shIf I put
echo "started"
into the file ~/1.sh I see that the file is called only once instead of multiple times.If you want the command to be called once for each commit you need to pass --max-args=1 to xargs. Otherwise xargs will cumulate the arguments until --max-chars is reached, and the command is expected to loop over them.
Andreas.