Re: git log -z doesn't separate commits with NULs

5 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: git log -z doesn't separate commits with NULs

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:08

Jeff King [off-list ref] writes:
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").

Re: git log -z doesn't separate commits with NULs

From: Nikolaj Shurkaev <hidden>
Date: 2016-06-15 22:53:08

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
--------------------------------------------------------------------------------------------
diff --git a/Documentation/git-format-patch.txt 
b/Documentation/git-format-patch.txt
index 6ea9be7..63267c6 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -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 пишет:
Jeff King[off-list ref]  writes:
quoted
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").

Re: git log -z doesn't separate commits with NULs

From: Jeff King <hidden>
Date: 2016-06-15 22:53:08

On Fri, Feb 24, 2012 at 12:21:13PM +0300, Nikolaj Shurkaev wrote:
quoted hunk
I'll propose to put something like this into git documentation
--------------------------------------------------------------------------------------------
diff --git a/Documentation/git-format-patch.txt
b/Documentation/git-format-patch.txt
index 6ea9be7..63267c6 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -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.
+
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

Re: git log -z doesn't separate commits with NULs

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:08

Jeff King [off-list ref] writes:
quoted
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.
+
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.
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?

Re: git log -z doesn't separate commits with NULs

From: Jeff King <hidden>
Date: 2016-06-15 22:53:08

On Fri, Feb 24, 2012 at 12:03:39PM -0800, Junio C Hamano wrote:
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.
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.
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
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?
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help