Re: [PATCH] git-am: flag suspiciously old or futuristic commits

[Paul Gortmaker]

On 2015-07-30 12:35 AM, Jacob Keller wrote:
 On Wed, Jul 29, 2015 at 3:20 PM, Stefan Beller sbel...@google.com wrote:
 On Wed, Jul 29, 2015 at 3:01 PM, Paul Gortmaker
 paul.gortma...@windriver.com wrote:
 The linux kernel repository has some commits in it with dates from
 the year 1970 and also 2030 (and possibly others).  We probably shouldi
 warn people when the dates look suspect.

 For commits in the future,  note that a committer in Australia
 could commit on New Years Day, and send it to a maintainer in North
 America and that would trip the notification on the maintainer's
 New Years Eve.  But that is unlikely, and the note is still
 correct; that the commit is from a future year.

 For commits in the past, I chose a somewhat arbitrary 30 year
 limit, which will allow stuff from post 1985; the thought being
 that someone might want to import an old repo into git from some
 other SCM.  We could alternatively set it to 5, which would then
 catch computers with a dead CMOS battery, at the risk of pestering
 the hypothetical museum curator of old bits.

 Sample output:

 paul@builder:~/git/linux-head$ grep Date: *patch
 future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400
 past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400

 paul@builder:~/git/linux-head$ git am future.patch
 note: commit is from future year 2037.
 Applying: arch/sh: make heartbeat driver explicitly non-modular
 paul@builder:~/git/linux-head$ git reset --hard HEAD~  /dev/null
 paul@builder:~/git/linux-head$ git am past.patch
 note: commit is from implausibly old year 1977.
 Applying: arch/sh: make heartbeat driver explicitly non-modular
 paul@builder:~/git/linux-head$

 Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
 ---
  git-am.sh | 15 +++
  1 file changed, 15 insertions(+)

 diff --git a/git-am.sh b/git-am.sh

 [+cc paul tan, who rewrote am in c as a GSoC project.]

 index 3af351ffaaf3..ff6deb8047a4 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -766,6 +766,21 @@ To restore the original branch and stop patching run 
 \\$cmdline --abort\.
 stop_here $this
 fi

 +   if test -n $GIT_AUTHOR_DATE
 +   then
 +   THIS_YEAR=`date +%Y`
 +   TOO_OLD=$(expr $THIS_YEAR - 30)
 +   TOO_NEW=$(expr $THIS_YEAR + 1)
 +   GIT_AUTHOR_YEAR=`date -d $GIT_AUTHOR_DATE +%Y`

 Would it make sense to not operate on year but on unix time, so the problem
 you mentioned in the commit message goes away?

 Another thought:
 Having this check in am seems a bit arbitrary to me (or rather
 workflow adapted ;) as
 we could also check in commit or pull (not sure if I actually mean the
 fetch or merge thereof)

 
 I think this makes most sense in am, as it is most likely to show up,
 in my mind, due to a format-patch mistake. If we do it during pull,
 would we just warn? how would we reject commits? that doesn't really
 fit..

So, it turns out this breaks t3400-rebase test since I was not aware
of the git internal date format (which that test suite uses).

And on top of that, in talking with a *BSD user, it seems the -d
flag isn't universal.  Over in that camp it means set DST offset. :(

So while I think the idea is sound, and worthwhile, scripting it will
become a lot more cumbersome and klunky.  If git-am becomes C, then
it would be easier to handle there, I think

Paul.
--

 
 We can't do it during commit, as obviously the broken machine will
 likely not be able to notice it at all.. We could check remotes during
 push but that doesn't solve this either..
 
 Maybe just emitting a warning during a fetch or am (since am doesn't
 use pull) would make the most sense?
 
 I don't think we can reject things when doing a fetch though, but we could 
 warn.
 
 Regards,
 Jake
 
--
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] git-am: flag suspiciously old or futuristic commits

[Paul Gortmaker]

The linux kernel repository has some commits in it with dates from
the year 1970 and also 2030 (and possibly others).  We probably shouldi
warn people when the dates look suspect.

For commits in the future,  note that a committer in Australia
could commit on New Years Day, and send it to a maintainer in North
America and that would trip the notification on the maintainer's
New Years Eve.  But that is unlikely, and the note is still
correct; that the commit is from a future year.

For commits in the past, I chose a somewhat arbitrary 30 year
limit, which will allow stuff from post 1985; the thought being
that someone might want to import an old repo into git from some
other SCM.  We could alternatively set it to 5, which would then
catch computers with a dead CMOS battery, at the risk of pestering
the hypothetical museum curator of old bits.

Sample output:

paul@builder:~/git/linux-head$ grep Date: *patch
future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400
past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400

paul@builder:~/git/linux-head$ git am future.patch
note: commit is from future year 2037.
Applying: arch/sh: make heartbeat driver explicitly non-modular
paul@builder:~/git/linux-head$ git reset --hard HEAD~  /dev/null
paul@builder:~/git/linux-head$ git am past.patch
note: commit is from implausibly old year 1977.
Applying: arch/sh: make heartbeat driver explicitly non-modular
paul@builder:~/git/linux-head$

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
---
 git-am.sh | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index 3af351ffaaf3..ff6deb8047a4 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -766,6 +766,21 @@ To restore the original branch and stop patching run 
\\$cmdline --abort\.
stop_here $this
fi
 
+   if test -n $GIT_AUTHOR_DATE
+   then
+   THIS_YEAR=`date +%Y`
+   TOO_OLD=$(expr $THIS_YEAR - 30)
+   TOO_NEW=$(expr $THIS_YEAR + 1)
+   GIT_AUTHOR_YEAR=`date -d $GIT_AUTHOR_DATE +%Y`
+
+   if [ $GIT_AUTHOR_YEAR -le $TOO_OLD ]; then
+   say $(gettext note: commit is from implausibly old 
year $GIT_AUTHOR_YEAR.)
+   fi
+   if [ $GIT_AUTHOR_YEAR -ge $TOO_NEW ]; then
+   say $(gettext note: commit is from future year 
$GIT_AUTHOR_YEAR.)
+   fi
+   fi
+
export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
 
case $resume in
-- 
2.5.0

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

Should git apply --check imply verbose?

[Paul Gortmaker]

TL;DR -- git apply --reject implies verbose, but the similar
git apply --check does not, which seems inconsistent.

Background:  A common (non-git) workflow can be to use patch --dry-run
to inspect whether a patch is feasible, and then use patch again
a 2nd time (w/o --dry-run) to actually apply it (and then work
through the rejects).

You can also do the above in a git repo, but you lose out because
patch doesn't (yet) capture the patched function names[1] in the
rejected hunks, making it hard to double check your work.

My initial thought was to replace the above two steps with
git apply --check ... and then git apply --reject ... so
that I could just abandon using patch altogether.

That works great, with just one snag that had me go reading the
source.  It seems that git apply --reject is verbose, and kind
of looks like the identical output I'd get if I used patch.  But
git apply --check is quite reserved in its output and doesn't
look at all like patch --dry-run.  I initially believed that
--check was stopping at the 1st failure, based on the output.

Only when I read the source did I realize it was checking all the
hunks silently, and adding a -v would make it similar to the
output from patch --dry-run.

Not a critical issue by any means, but having the -v implied
by --check (or perhaps having both default to non-verbose?)
might save other users from getting confused in the same way.

Thanks,
Paul.
--

[1] https://savannah.gnu.org/bugs/index.php?39819
--
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: Should git apply --check imply verbose?

[Paul Gortmaker]

On 13-08-20 01:57 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 TL;DR -- git apply --reject implies verbose, but the similar
 git apply --check does not, which seems inconsistent.
 
 Hmmm, I am of two minds.  From purely idealistic point of view, I
 can see why defaulting both to non-verbose may look a more
 attractive way to go, but I have my reservations that is more than
 the usual change-aversion.

OK, so given your feedback, how do you feel about a patch to the
documentation that indicates to use -v in combination with the
--check to get equivalent patch --dry-run behaviour?   If that
had existed, I'd have not gone rummaging around in the source, so
that should be good enough to help others avoid the same...

P.
--

 
 Historically, check was primarily meant to see if the patch is
 applicable cleanly in scripts, and we never thought it would make
 any sense to make it verbose by default.  
 
 On the other hand, the operation of reject, which was a much later
 invention, was primarily meant to be observed by humans to see how
 the patch failed to cleanly apply and where, to help them decide
 where to look in the target to wiggle the rejected hunk into (even
 when it is driven from a script).  It did not make much sense to
 squelch its output.
 
 In addition, because check is an idempotent operation that does
 not touch anything in the index or the working tree, running with
 check and then check verbose is possible if somebody runs it
 without verbose and then decides later that s/he wants to see the
 details.  But reject does touch the working tree files with
 applicable hunks, so after a quiet reject, there is no way to see
 the verbose output like you can with check.
 
--
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: Should git apply --check imply verbose?

[Paul Gortmaker]

On 13-08-20 02:51 PM, Jonathan Nieder wrote:
 Hi Paul,
 
 Paul Gortmaker wrote:
 
 OK, so given your feedback, how do you feel about a patch to the
 documentation that indicates to use -v in combination with the
 --check to get equivalent patch --dry-run behaviour?
 
 Sounds like a good idea to me.
 
 I assume you mean a note in the OPTIONS or EXAMPLES section of
 Documentation/git-apply.txt?

I hadn't looked exactly where yet, but wherever makes sense and
wherever appears in TFM.

P.
--

 
 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: Should git apply --check imply verbose?

[Paul Gortmaker]

On 13-08-20 03:54 PM, Steven Rostedt wrote:
 On Tue, 20 Aug 2013 12:45:03 -0700
 Junio C Hamano gits...@pobox.com wrote:
 
 Steven Rostedt rost...@goodmis.org writes:

 I do not think it is necessarily a good idea to assume that people
 who are learning git apply know how GNU patch works.

 Linus told me that git apply was basically a replacement for patch.
 Why would you think it would not be a good idea to assume that people
 would not be familiar with how GNU patch works?

 The audience of Git these days are far more widely spread than the
 kernel circle.  I am not opposed to _helping_ those who happen to
 know patch, but I was against a description that assumes readers
 know it, i.e. making it a requirement to know patch to understand
 apply.
 
 Patch is used by much more than just the kernel folks ;-)  I've been
 using patch much longer than I've been doing kernel development.
 
 

 But I do agree that the description of -v, --verbose has a lot of
 room for improvement.

Report progress to stderr. By default, only a message about the
current patch being applied will be printed. This option will cause
additional information to be reported.

 It is totally unclear what additional information is reported at
 all.

 In other words, your enhancement to the documentation could go like:

  ... By default, ... With this option, you will additionally
  see such and such and such in the output (this is similar to
  what patch --dry-run would give you).  See the EXAMPLES
  section to get a feel of how it looks like.

 and I would not be opposed, as long as such and such and such are
 written in such a way that the reader does not have to have a prior
 experience with GNU patch in order to understand it.

 Clear?
 
 Looks good to me. Paul, what do you think?

Yep, I'll write something up tomorrow which loosely matches the above.

Thanks,
Paul.
--

 
 Thanks,
 
 -- Steve
 
--
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: [RFC/PATCH] apply: parse and act on --irreversible-delete output

[Paul Gortmaker]

On 12-08-02 05:20 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 The '-D' or '--irreversible-delete' option of format-patch is
 great for sending out patches to mailing lists, where there
 is little value in seeing thousands of lines of deleted code.
 Attention can then be focused on the changes relating to
 the binding of the deleted code (Makefiles, etc).

 However the original intent of commit 467ddc14f (git diff -D: omit
 the preimage of deletes) was as follows:

 To prevent such a patch from being applied by mistake, the
 output is designed not to be usable by git apply (or GNU patch);
 it is strictly for human consumption.

 The downside of this, is that patches to mailing lists which are
 then either managed with patchworks, or dealt with directly by
 maintainers, will need manual intervention if they are to be used.

 But with the index lines, there is no reason why we can't act
 intelligently and automatically on these with git apply.
 If we can unambiguously map what was recorded as the deleted
 SHA prefix to the SHA of the matching blob filename in our tree,
 then we set the image len to zero which facilitates the delete.

 Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
 ---

 For a recent use case example, see:
  http://www.spinics.net/lists/netdev/msg206519.html

 Could be wrapped in an am.applyirreversible if for some reason
 global deployment was considered unwise?

  Documentation/diff-options.txt |  5 +++--
  builtin/apply.c| 30 ++
  2 files changed, 33 insertions(+), 2 deletions(-)

 diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
 index cf4b216..efaaf1c 100644
 --- a/Documentation/diff-options.txt
 +++ b/Documentation/diff-options.txt
 @@ -328,8 +328,9 @@ endif::git-log[]
  --irreversible-delete::
  Omit the preimage for deletes, i.e. print only the header but not
  the diff between the preimage and `/dev/null`. The resulting patch
 -is not meant to be applied with `patch` nor `git apply`; this is
 -solely for people who want to just concentrate on reviewing the
 +is not meant to be applied with `patch` (but can be with `git apply`).
 
 ... but only when you are applying to the exact version the patch
 was created from, no?

True, I can add that extra detail/limitation to the docs.

 
 +This is for people who want to avoid seeing/mailing all the deleted
 +file content, and instead just concentrate on reviewing the
  text after the change. In addition, the output obviously lack
  enough information to apply such a patch in reverse, even manually,
  hence the name of the option.
 diff --git a/builtin/apply.c b/builtin/apply.c
 index d453c83..363da63 100644
 --- a/builtin/apply.c
 +++ b/builtin/apply.c
 @@ -2933,6 +2933,36 @@ static int apply_fragments(struct image *img, struct 
 patch *patch)
  if (patch-is_binary)
  return apply_binary(img, patch);
  
 +/* output from --irreversible-delete (looks like empty file delete) */
 +if (patch-is_delete  0  !frag  img-len  0) {
 
 What is (img-len  0) part trying to ensure?
 
 If somebody gives you an irreversible deletion of an empty file,
 shouldn't this codepath handle it the same way?

The format-patch output of the deletion of an empty file is
identical with or without the switch, so I didn't want to
accidentally limit people from normal empty file deletions
by invoking special checks on them that did not exist before.

 
 +unsigned char file_sha1[20], patch_sha1[20];
 +struct object_context oc;
 +
 +if (apply_in_reverse) {
 +error(_(can not reverse an irreversible-delete patch
 +  on file '%s'.), name);
 +return -1;
 +}
 
 The return value of error() is already -1, so you can just return
 it without { stmt; return -1; }.

OK, will update.  I'd inadvertently got the separate statements
by copying the code below it, which did a conditional return based
on what apply_with_reject was set to, but I'm not sure any special
reject behaviour for an irreversible delete fail makes sense.

 
 +
 +strcpy(oc.path, name);
 +if (get_sha1_with_context(patch-old_sha1_prefix,
 +GET_SHA1_BLOB | GET_SHA1_QUIETLY, patch_sha1, oc)) {
 +error(_(the deleted SHA prefix of file '%s' (%s), does
 +   not seem to exist in this repository.), name,
 +  patch-old_sha1_prefix);
 +return -1;
 +}
 
 This is not sufficient to make sure patch_sha1 exists in your
 repository and is indeed a blob object.  GET_SHA1_BLOB is a hint to
 say if there are more than one that shares this prefix, ignore ones
 that are not blob---if there is only one remains, then even though
 the prefix is ambiguous in this repository, we will take it.  If
 you

[PATCH] apply: delete unused deflate_origlen from patch struct

[Paul Gortmaker]

It hasn't been used since 2006, as of commit 3cd4f5e8

git-apply --binary: clean up and prepare for --reverse

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com

diff --git a/builtin/apply.c b/builtin/apply.c
index d453c83..3bf71dc 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -188,7 +188,6 @@ struct patch {
int is_new, is_delete;  /* -1 = unknown, 0 = false, 1 = true */
int rejected;
unsigned ws_rule;
-   unsigned long deflate_origlen;
int lines_added, lines_deleted;
int score;
unsigned int is_toplevel_relative:1;
-- 
1.7.12.rc1.1.ga9c166e

--
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] git-am: make a config setting for --keep-non-patch switch

[Paul Gortmaker]

In order to make a commit be invariant (excluding ID) over
a format-patch and subsequent am cycle, one needs to use
the '--keep-non-patch' so that commits like:

[PATCH] [i386] fix foo bar arch/x86/mm

only lose the [PATCH] and not the [i386] part.  Since it
is a common desire (e.g. linux kernel stable trees) to have
the subjects remain invariant during a backport, there is
a genuine need for making this the default behaviour from
a config file, versus specifying it in scripts and on the
command line each time.

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
---

See http://lkml.indiana.edu/hypermail/linux/kernel/1203.1/01817.html
for additional background; stable maintainers using it etc.

 Documentation/config.txt   | 9 +
 Documentation/git-am.txt   | 4 
 contrib/completion/git-completion.bash | 1 +
 git-am.sh  | 8 
 4 files changed, 22 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a95e5a4..47aded5 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -655,6 +655,15 @@ am.keepcr::
by giving '--no-keep-cr' from the command line.
See linkgit:git-am[1], linkgit:git-mailsplit[1].
 
+am.keepnonpatch::
+   Normally git-mailinfo strips from the Subject line, all leading
+   strings bracketed with [ and ] pairs.  If this setting is true,
+   git-am will call git-mailinfo with the parameter '-b' so that only
+   the pairs whose bracketed string contains the word PATCH are
+   stripped.  Can be overridden by giving ' '--no-keep-non-patch'
+   from the command line.
+   See linkgit:git-am[1], linkgit:git-mailinfo[1].
+
 apply.ignorewhitespace::
When set to 'change', tells 'git apply' to ignore changes in
whitespace, in the same way as the '--ignore-space-change'
diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 19d57a8..790efdb 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -41,7 +41,11 @@ OPTIONS
Pass `-k` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
 
 --keep-non-patch::
+--no-keep-non-patch::
Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
+   The `am.keepnonpatch` configuration variable can be used to specify
+   the default behaviour.  The `--no-keep-non-patch` is useful to
+   override any `am.keepnonpatch` setting.
 
 --keep-cr::
 --no-keep-cr::
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index ffedce7..04339df 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1758,6 +1758,7 @@ _git_config ()
advice.statusHints
alias.
am.keepcr
+   am.keepnonpatch
apply.ignorewhitespace
apply.whitespace
branch.autosetupmerge
diff --git a/git-am.sh b/git-am.sh
index c02e62d..9f6adbf 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -16,6 +16,7 @@ s,signoff   add a Signed-off-by line to the commit message
 u,utf8  recode into utf8 (default)
 k,keep  pass -k flag to git-mailinfo
 keep-non-patch  pass -b flag to git-mailinfo
+no-keep-non-patch do not pass -b flag to git-mailsplit, independent of 
am.keepnonpatch
 keep-cr pass --keep-cr flag to git-mailsplit for mbox format
 no-keep-cr  do not pass --keep-cr flag to git-mailsplit independent of 
am.keepcr
 c,scissors  strip everything before a scissors line
@@ -381,6 +382,11 @@ then
 keepcr=t
 fi
 
+if test $(git config --bool --get am.keepnonpatch) = true
+then
+keep=b
+fi
+
 while test $# != 0
 do
case $1 in
@@ -402,6 +408,8 @@ do
keep=t ;;
--keep-non-patch)
keep=b ;;
+   --no-keep-non-patch)
+   keep= ;;
-c|--scissors)
scissors=t ;;
--no-scissors)
-- 
1.7.12.rc1.1.gbce1580

--
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] git-am: make a config setting for --keep-non-patch switch

[Paul Gortmaker]

On 12-08-01 02:48 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 In order to make a commit be invariant (excluding ID) over
 a format-patch and subsequent am cycle, one needs to use
 the '--keep-non-patch' so that commits like:

  [PATCH] [i386] fix foo bar arch/x86/mm

 only lose the [PATCH] and not the [i386] part.  Since it
 is a common desire (e.g. linux kernel stable trees) to have
 the subjects remain invariant during a backport, there is
 a genuine need for making this the default behaviour from
 a config file, versus specifying it in scripts and on the
 command line each time.

 Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
 ---

 See http://lkml.indiana.edu/hypermail/linux/kernel/1203.1/01817.html
 for additional background; stable maintainers using it etc.
 
 That's a blast from the past; it would have been so much nicer
 if the patch came earlier ;-)

I just happened to rediscover the issue yesterday when backporting
a patch that had [S390] in it to linux-2.6.34 -- went looking for
any possible previous reports and found the above thread.  So the
timing was largely out of my control.  :)

 
 The patch looks from sane; we may want to have a test in t4150, just like
 we have tests for am.keepcr in t4253.  We have plenty of time as we
 are in feature freeze right now.

I'll take a look at the test cases, and resend once 1.7.12 is done.

Paul.
--

 
 Thanks.
 

  Documentation/config.txt   | 9 +
  Documentation/git-am.txt   | 4 
  contrib/completion/git-completion.bash | 1 +
  git-am.sh  | 8 
  4 files changed, 22 insertions(+)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index a95e5a4..47aded5 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -655,6 +655,15 @@ am.keepcr::
  by giving '--no-keep-cr' from the command line.
  See linkgit:git-am[1], linkgit:git-mailsplit[1].
  
 +am.keepnonpatch::
 +Normally git-mailinfo strips from the Subject line, all leading
 +strings bracketed with [ and ] pairs.  If this setting is true,
 +git-am will call git-mailinfo with the parameter '-b' so that only
 +the pairs whose bracketed string contains the word PATCH are
 +stripped.  Can be overridden by giving ' '--no-keep-non-patch'
 +from the command line.
 +See linkgit:git-am[1], linkgit:git-mailinfo[1].
 +
  apply.ignorewhitespace::
  When set to 'change', tells 'git apply' to ignore changes in
  whitespace, in the same way as the '--ignore-space-change'
 diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
 index 19d57a8..790efdb 100644
 --- a/Documentation/git-am.txt
 +++ b/Documentation/git-am.txt
 @@ -41,7 +41,11 @@ OPTIONS
  Pass `-k` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
  
  --keep-non-patch::
 +--no-keep-non-patch::
  Pass `-b` flag to 'git mailinfo' (see linkgit:git-mailinfo[1]).
 +The `am.keepnonpatch` configuration variable can be used to specify
 +the default behaviour.  The `--no-keep-non-patch` is useful to
 +override any `am.keepnonpatch` setting.
  
  --keep-cr::
  --no-keep-cr::
 diff --git a/contrib/completion/git-completion.bash 
 b/contrib/completion/git-completion.bash
 index ffedce7..04339df 100644
 --- a/contrib/completion/git-completion.bash
 +++ b/contrib/completion/git-completion.bash
 @@ -1758,6 +1758,7 @@ _git_config ()
  advice.statusHints
  alias.
  am.keepcr
 +am.keepnonpatch
  apply.ignorewhitespace
  apply.whitespace
  branch.autosetupmerge
 diff --git a/git-am.sh b/git-am.sh
 index c02e62d..9f6adbf 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -16,6 +16,7 @@ s,signoff   add a Signed-off-by line to the commit 
 message
  u,utf8  recode into utf8 (default)
  k,keep  pass -k flag to git-mailinfo
  keep-non-patch  pass -b flag to git-mailinfo
 +no-keep-non-patch do not pass -b flag to git-mailsplit, independent of 
 am.keepnonpatch
  keep-cr pass --keep-cr flag to git-mailsplit for mbox format
  no-keep-cr  do not pass --keep-cr flag to git-mailsplit independent of 
 am.keepcr
  c,scissors  strip everything before a scissors line
 @@ -381,6 +382,11 @@ then
  keepcr=t
  fi
  
 +if test $(git config --bool --get am.keepnonpatch) = true
 +then
 +keep=b
 +fi
 +
  while test $# != 0
  do
  case $1 in
 @@ -402,6 +408,8 @@ do
  keep=t ;;
  --keep-non-patch)
  keep=b ;;
 +--no-keep-non-patch)
 +keep= ;;
  -c|--scissors)
  scissors=t ;;
  --no-scissors)
--
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: False positive from orphaned_commit_warning() ?

[Paul Gortmaker]

On 12-07-25 05:52 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 Has anyone else noticed false positives coming from the
 orphan check?
 
 Thanks.  This should fix it.

Indeed it does.  Thanks for the fix (and git in general).

Paul.
--

 
  builtin/checkout.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/builtin/checkout.c b/builtin/checkout.c
 index 6acca75..d812219 100644
 --- a/builtin/checkout.c
 +++ b/builtin/checkout.c
 @@ -606,7 +606,7 @@ static int add_pending_uninteresting_ref(const char 
 *refname,
const unsigned char *sha1,
int flags, void *cb_data)
  {
 - add_pending_sha1(cb_data, refname, sha1, flags | UNINTERESTING);
 + add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
   return 0;
  }
  
 
--
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

False positive from orphaned_commit_warning() ?

[Paul Gortmaker]

Has anyone else noticed false positives coming from the
orphan check?  It is warning me about commits that are
clearly on master.  Here is an example, where I checkout
master~2 and then switch back to master.  It somehow thinks
that master~2 is orphaned, when master~2 is by definition
in the commit chain leading to master.

The repo is tiny, so anyone can try and reproduce this. (I've
done so on v1.7.9 and v1.7.11, on two different machines).

git://git.yoctoproject.org/yocto-kernel-tools.git

Paul.
-

paul@foo:~/git/yocto-kernel-tools$ git checkout master~2
Note: checking out 'master~2'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in
this
state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -b with the checkout command again.
Example:

  git checkout -b new_branch_name

HEAD is now at e693754... kgit-checkpoint: fix verify_branch variable
name typo
paul@foo:~/git/yocto-kernel-tools$ git checkout master
Warning: you are leaving 38 commits behind, not connected to
any of your branches:

  e693754 kgit-checkpoint: fix verify_branch variable name typo
  ee67a7b kgit-config-cleaner: fix redefintion processing
  579b1ba meta: support flexible meta branch naming
  4673bdb scc: allow kconf fragment searching
 ... and 34 more.

If you want to keep them by creating a new branch, this may be a good time
to do so with:

 git branch new_branch_name e6937544e030637cec029edee34737846a036ece

Switched to branch 'master'
paul@foo:~/git/yocto-kernel-tools$ git branch --contains 
e6937544e030637cec029edee34737846a036ece
* master
paul@foo:~/git/yocto-kernel-tools$ git --version
git version 1.7.11.1
paul@foo:~/git/yocto-kernel-tools$ cat .git/config 
[core]
repositoryformatversion = 0
filemode = true
bare = false
logallrefupdates = true
[remote origin]
fetch = +refs/heads/*:refs/remotes/origin/*
url = git://git.yoctoproject.org/yocto-kernel-tools.git
[branch master]
remote = origin
merge = refs/heads/master
paul@foo:~/git/yocto-kernel-tools$ 
---
--
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] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

If git am fails to apply something, the end user may need
to know where to find the patch.  This is normally known for
a single patch, but if the user is processing a mbox with
many patches, they may not have a single broken out patch
handy.  So, provide a helpful hint as to where they can
find the patch to do some sort of manual fixup, if we
are processing a mbox with more than one patch in it.

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com
---
[v2: drop text suggesting what to do with failed patch; only
 emit the help text if we are processing mbox with multi patches]

 git-am.sh |5 +
 1 file changed, 5 insertions(+)

diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..20b3b73 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -855,6 +855,11 @@ did you forget to use 'git add'?
if test $apply_status != 0
then
eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+   if test $patch_format = mbox  test $last -ne 1
+   then
+   eval_gettextln You can find the copy of the patch that 
failed here:
+   $dotest/patch
+   fi
stop_here_user_resolve $this
fi
 
-- 
1.7.9.7

--
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] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-13 03:58 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 If git am fails to apply something, the end user may need
 to know where to find the patch.  This is normally known for
 a single patch, but if the user is processing a mbox with
 many patches, they may not have a single broken out patch
 handy.  So, provide a helpful hint as to where they can
 find the patch to do some sort of manual fixup, if we
 are processing a mbox with more than one patch in it.
 
 I would rather see this done even for a single patch mbox.  The

OK, I got the opposite impression from your prev. mail when
you mentioned that I hadn't limited the message output at all.

I'm fine with the changes you've proposed below, and can squash that
into a v3 and resend again.

Paul.
--

 patch that was fed to git apply by git am and failed to apply is
 that one, not the one in the mbox you gave git am.  The latter may
 be ungrokkable with GNU patch or git apply, if the original was
 sent in Quoted-Printable and such MIME funnies, which is the whole
 point of having a separate file there for git am, instead of
 feeding the original.
 
 I am not sure if we should limit $patch_format to mbox, but I think
 showing this unconditionally regardless of mbox/stgit/hg will teach
 the user only one location to remember, so perhaps like this?
 
  Documentation/config.txt | 3 +++
  git-am.sh| 4 ++--
  2 files changed, 5 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 0e1168c..b1f0a75 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -143,6 +143,9 @@ advice.*::
   Advice shown when you used linkgit:git-checkout[1] to
   move to the detach HEAD state, to instruct how to create
   a local branch after the fact.
 + amWorkDir::
 + Advice that shows the location of the patch file when
 + linkgit:git-am[1] fails to apply it.
  --
  
  core.fileMode::
 diff --git a/git-am.sh b/git-am.sh
 index dc48f87..f1ae932 100755
 --- a/git-am.sh
 +++ b/git-am.sh
 @@ -834,9 +834,9 @@ did you forget to use 'git add'?
   if test $apply_status != 0
   then
   eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
 - if test $patch_format = mbox  test $last -ne 1
 + if test $(git config --bool advice.amworkdir) != false
   then
 - eval_gettextln You can find the copy of the patch that 
 failed here:
 + eval_gettextln The copy of the patch that failed is 
 found in:
 $dotest/patch
   fi
   stop_here_user_resolve $this
 

--
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] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

If git am wasn't run with --reject, we assume the end user
knows where to find the patch.  This is normally true for
a single patch, but if the user is processing a mbox with
many patches, they may not have a single broken out patch
handy.  So, provide a helpful hint as to where they can
find the patch to do the manual fixup before eventually
continuing with git add ... ; git am -r.

Signed-off-by: Paul Gortmaker paul.gortma...@windriver.com

diff --git a/git-am.sh b/git-am.sh
index f8b7a0c..32e6ac0 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -854,7 +854,10 @@ did you forget to use 'git add'?
fi
if test $apply_status != 0
then
-   eval_gettextln 'Patch failed at $msgnum $FIRSTLINE'
+   eval_gettextln Patch failed at $msgnum $FIRSTLINE
+You can try running the following command:
+   patch -p1 --dry-run  $dotest/patch
+in order to possibly get more information on why it failed.
stop_here_user_resolve $this
fi
 
-- 
1.7.9.7

--
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] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-12 01:45 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 If git am wasn't run with --reject, we assume the end user
 knows where to find the patch.  This is normally true for
 a single patch,
 
 Not at all.  Whether it is a single or broken, the patch is fed to
 underlying apply from an unadvertised place.

What I meant by this was the difference between:

git am 0001-some-standalone-single.patch
vs.
git am mbox

In the 1st, the standalone patch is 100% clear and easy to access,
because we really don't need/care about the unadvertised place.

Maybe I should have said knows how to get at the single patch?

 
 So, provide a helpful hint as to where they can
 find the patch ...
 
 This is OK, but you may want to give a way to squelch it once the
 user learns where it is by following the usual advice.* thing.
 
 ... to do the manual fixup before eventually
 continuing with git add ... ; git am -r.
 
 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

I'm not 100% sure I'm following what part here is not OK.  If you
can help me understand that, I'll respin the change accordingly.

Is it the assumption that the user will have the patch
command in /usr/bin not OK, or that the message implies that
git is somehow using /usr/bin/patch is not OK?

In case it helps any, a brief summary of my workflow is this:

git am /tmp/mbox
some random fail halfway in the queue
patch -p1 --dry-run  .git/rebase-apply/patch
# gauge status.  Is patch really invalid, or already applied?
# already applied; git am --skip
# no, if valid, but with minor issues, apply what we can.
patch -p1  .git/rebase-apply/patch
# manually deal with rejects (typically with wiggle)
git add any_new_files
git add -u
git am -r

Paul.
--

 
 

--
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] git-am: indicate where a failed patch is to be found.

[Paul Gortmaker]

On 12-07-12 02:53 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:
 
 On 12-07-12 01:45 PM, Junio C Hamano wrote:
 Paul Gortmaker paul.gortma...@windriver.com writes:

 If git am wasn't run with --reject, we assume the end user
 knows where to find the patch.  This is normally true for
 a single patch,

 Not at all.  Whether it is a single or broken, the patch is fed to
 underlying apply from an unadvertised place.

 What I meant by this was the difference between:

  git am 0001-some-standalone-single.patch
 vs.
  git am mbox

 In the 1st, the standalone patch is 100% clear and easy to access,
 because we really don't need/care about the unadvertised place.
 
 It does not matter at all that 0001-foo.patch only has a single
 patch.  If you are going to fix up the patch after you saw git am
 failed, you will be fixing .git/rebase-apply/patch with your editor
 and re-run git am without arguments, at which point git am will
 not look at your 0001-foo.patch file at all.

I think this is where our two thinking paths diverge.  You are
suggesting I edit and fix the patch.  Yes, occasionally I do
that, if it is a trivial context change.  But hand editing a
patch is not for Joe Average, and gets very complicated in all
but the trivial cases.  So, what happens _way_ more often, is that
I want to apply what can be applied, and deal with the rejects
on a one-by-one basis after that.  (BTW, this is not just me;
this patch came about from discussions with other kernel folks.)

 
 This is _NOT_ fine, especially if you suggest patch the user may
 not have, and more importantly does not have a clue why git apply
 rejected it (am does _not_ use patch at all).

 I'm not 100% sure I'm following what part here is not OK.  If you
 can help me understand that, I'll respin the change accordingly.
 
 Do not ever mention patch -p1.  It is not the command that git
 am uses, and it is not what detected the breakage in the patch.

This may be true, but it _is_ the command that I (and others) have
defaulted to using, if for no other reason than ignorance.

 
 The command to guide the user to is git apply.
 

OK.  But I don't see a --dry-run equivalent -- and git apply --check
just gives me a repeat of the same fail messages that git am did.

With patch -p1 --dry-run  I get information that immediately
lets me see whether the patch is viable or not.  Is there a way
to get a similar thing from git apply that I've overlooked?

Paul.
---
--
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