Re: Consequences of CRLF in index?

[Ross Kabus]

Is "* -text" in any way different than "-text" (without the * asterisk)? All
of my .gitattributes files have "-text" (no * asterisk) and I haven't noticed
any difference but could I be missing something subtle?

~Ross

[PATCH v2] commit-tree: do not complete line in -F input

[Ross Kabus]

"git commit-tree -F ", unlike "cat  | git
commit-tree" (i.e. feeding the same contents from the standard
input), added a missing final newline when the input ended in an
incomplete line.

Correct this inconsistency by leaving the incomplete line as-is,
as erring on the side of not touching the input is preferable
and expected for a plumbing command like "commit-tree".

Signed-off-by: Ross Kabus <rka...@aerotech.com>
---
Fixed up commit message with feedback from Junio.

 builtin/commit-tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 19e898fa4..2177251e2 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -102,7 +102,6 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
if (fd && close(fd))
die_errno("git commit-tree: failed to close 
'%s'",
  argv[i]);
-   strbuf_complete_line();
continue;
}
 
-- 
2.13.1.windows.2

Re: [PATCH] commit-tree: don't append a newline with -F

[Ross Kabus]

On Thu, Sep 7, 2017 at 9:35 PM, Junio C Hamano  wrote:
> commit-tree: do not complete line in -F input
>
> "git commit-tree -F ", unlike "cat  | git
> commit-tree" (i.e. feeding the same contents from the standard
> input), added a missing final newline when the input ended in an
> incomplete line.
>
> Correct this inconsistency by leaving the incomplete line as-is,
> as erring on the side of not touching the input is preferrable
> and expected for a plumbing command like "commit-tree".

That all makes sense to me and is clearer too, I like this change.
Do I need to resubmit the patch or will you just use that text instead?

[PATCH] commit-tree: don't append a newline with -F

[Ross Kabus]

This change makes it such that commit-tree -F never appends a newline
character to the supplied commit message (either from file or stdin).

Previously, commit-tree -F would always append a newline character to
the text brought in from file or stdin. This has caused confusion in a
number of ways:
  - This is a plumbing command and it is generally expected not to do
text cleanup or other niceties.
  - stdin piping with "-F -" appends a newline but stdin piping without
-F does not append a newline (inconsistent).
  - git-commit does not specifically append a newline to the "-F"
input. The issue is somewhat muddled by the fact that git-commit
does pass the message through its --cleanup option, which may add
such a newline. But for commit-tree to match "commit --cleanup=verbatim",
we should not do so here.

Signed-off-by: Ross Kabus <rka...@aerotech.com>
---
 builtin/commit-tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 19e898fa4..2177251e2 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -102,7 +102,6 @@ int cmd_commit_tree(int argc, const char **argv, const char 
*prefix)
if (fd && close(fd))
die_errno("git commit-tree: failed to close 
'%s'",
  argv[i]);
-   strbuf_complete_line();
continue;
}
 
-- 
2.13.1.windows.2

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

[Ross Kabus]

Gmail mangled that patch of course...
Ross Kabus
Software Engineer
www.aerotech.com | 412-968-2833


On Tue, Sep 5, 2017 at 4:57 PM, Ross Kabus <rka...@aerotech.com> wrote:
> From: Ross Kabus <rka...@aerotech.com>
> Date: Tue, 5 Sep 2017 13:54:52 -0400
> Subject: [PATCH] commit-tree: don't append a newline with -F
>
> This change makes it such that commit-tree -F never appends a newline
> character to the supplied commit message (either from file or stdin).
>
> Previously, commit-tree -F would always append a newline character to
> the text brought in from file or stdin. This has caused confusion in a
> number of ways:
>   - This is a plumbing command and it is generally expected not to do
> text cleanup or other niceties.
>   - stdin piping with "-F -" appends a newline but stdin piping without
> -F does not append a newline (inconsistent).
>   - git-commit has the --cleanup=verbatim option that prevents appending
> a newline with its -F argument. There is no verbatim counterpart to
> commit-tree -F (inconsistent).
> ---
>  builtin/commit-tree.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
> index 19e898fa4..2177251e2 100644
> --- a/builtin/commit-tree.c
> +++ b/builtin/commit-tree.c
> @@ -102,7 +102,6 @@ int cmd_commit_tree(int argc, const char **argv,
> const char *prefix)
>   if (fd && close(fd))
>   die_errno("git commit-tree: failed to close '%s'",
>argv[i]);
> - strbuf_complete_line();
>   continue;
>   }
>
> --
> 2.13.1.windows.2

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

[Ross Kabus]

From: Ross Kabus <rka...@aerotech.com>
Date: Tue, 5 Sep 2017 13:54:52 -0400
Subject: [PATCH] commit-tree: don't append a newline with -F

This change makes it such that commit-tree -F never appends a newline
character to the supplied commit message (either from file or stdin).

Previously, commit-tree -F would always append a newline character to
the text brought in from file or stdin. This has caused confusion in a
number of ways:
  - This is a plumbing command and it is generally expected not to do
text cleanup or other niceties.
  - stdin piping with "-F -" appends a newline but stdin piping without
-F does not append a newline (inconsistent).
  - git-commit has the --cleanup=verbatim option that prevents appending
a newline with its -F argument. There is no verbatim counterpart to
commit-tree -F (inconsistent).
---
 builtin/commit-tree.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/builtin/commit-tree.c b/builtin/commit-tree.c
index 19e898fa4..2177251e2 100644
--- a/builtin/commit-tree.c
+++ b/builtin/commit-tree.c
@@ -102,7 +102,6 @@ int cmd_commit_tree(int argc, const char **argv,
const char *prefix)
  if (fd && close(fd))
  die_errno("git commit-tree: failed to close '%s'",
   argv[i]);
- strbuf_complete_line();
  continue;
  }

-- 
2.13.1.windows.2

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

[Ross Kabus]

On Tue, Sep 5, 2017 at 11:36 AM, Jeff King  wrote:

> So I'd argue that "git commit -F" is doing a reasonable
> thing, and "commit-tree -F" should probably change to match it (because
> it's inconsistent, and because if anything the plumbing commit-tree
> should err more on the side of not touching its input than the porcelain
> commit command).

I would agree that "commit-tree -F" should change to match the behavior of
"git commit -F --cleanup=verbatim". I feel pretty strongly that this type of
cleanup logic shouldn't be done in a plumbing command, though I'm not sure
it is a big enough deal to change behavior/compatibility for everyone.

>   $ tree=$(git write-tree)
>   $ commit=$(printf end | git commit-tree $tree)
>   $ git cat-file commit $commit | xxd
>   ...
>   0090: 3034 3030 0a0a 656e 64   0400..end

Yup, confusion #2. I was using "-F -" which I see now is a different code
path. Reading via stdin without "-F -" _is_ the verbatim option. This
difference burned someone else on the mailing list as well. See:

https://public-inbox.org/git/cacauox6zt0wxxclxk+sk0e4hgfd_a_ewwkvwntoxk0-hzw7...@mail.gmail.com/

Probably more reason that "-F" should be 'verbatim' by default.

~Ross

Re: [Bug] commit-tree shouldn't append an extra newline to commit messages

[Ross Kabus]

On Sat, Sep 2, 2017 at 4:33 AM, Jeff King  wrote:

> But I am confused by your "inconsistent with git commit porcelain"
> comment. The porcelain git-commit definitely _does_ add a newline if one
> isn't present (and in fact runs the whole thing through git-stripspace
> to clean up whitespace oddities).

Ok I figured out my confusion. The repository I am working with did
commits with "git commit --cleanup=verbatim" thus do not have a newline.
This is why I thought there was an inconsistency.

> So I don't think "inconsistent with git-commit" is a compelling
> argument, unless I'm missing something.

Agreed, but now I guess I would argue that it is inconsistent because
it lacks a "verbatim" option like git-commit has. I would like to see
an argument like this for commit-tree but a clean way to add this option
didn't immediately jump out at me.

> And definitely it does not when taking the message in via stdin.

I'm not seeing this, I see commit-tree as adding a new line even via
stdin (and the code seems to corroborate this), unless I missed something.

~Ross

[Bug] commit-tree shouldn't append an extra newline to commit messages

[Ross Kabus]

Hello,

When doing git commit-tree to manually create a commit object, it can be seen
that the resulting commit's message has an extra appended newline (\n) that
was not present in the input for either argument -m or -F. This is both
undesirable and inconsistent with the git commit porcelain command.

In code this happens in the file "builtin\commit-tree.c" lines #80 and #105
(these lines numbers are the same for master, maint, and next branches). It
seems like the calls to "strbuf_complete_line()" should just be removed to
preserve the original input message exactly. As far as I can tell removing
this call doesn't have any unintended side-effects.

git version 2.13.1.windows.2

Thanks,
Ross