* Re: In-body from headers ignored
From: Junio C Hamano @ 2017-02-05 23:43 UTC (permalink / raw)
To: Liam Breck; +Cc: git
In-Reply-To: <CAKvHMgQLKccm2LcL4LGhz0afVthaS2gvEcLtoHX2TcDnr1npbw@mail.gmail.com>
Liam Breck <liam@networkimprov.net> writes:
> git format-patch & send-email generate the in-body From header.
>
> git am recognizes it.
>
> git commit & format-patch & send-email ignore it. (The latter two will
> add a new header above an extant one.) Is there a rationale for this?
I may be misunderstanding you, but I am guessing that you are
expecting
$ GIT_AUTHOR_NAME="Liam Breck" GIT_AUTHOR_EMAIL='liam@n.net' \
git commit -m 'The title of the patch
From: Somebody Else <somebody@else.xz>
Subject: The real title of the patch
This is the (true) first line of the body of the message.'
to record a commit object that would give
$ git cat-file commit HEAD
tree ....
parent ....
author Somebody Else <somebody@else.xz> ....
committer ...
The real title of the patch
This is the (true) first line of the body of the message.
and seeing that the real author is still you, the title is "The real
title of the patch", and the first paragraph of the body consists of
these two lines that begin with "From: and "Subject:".
This is very much deliberate. "git commit" does not care if the
second paragraph in the body of the message resembles e-mail
headers, because it is a command that can be used by people who do
not even e-mail patches.
"git format-patch" and "git am" are all about passing the commit
between people over e-mail, and they know that the second paragraph
can have "From:", "Subject:" or "Date:" to override what "am"
obtains from the header lines of the e-mail that carried the
message, because the e-mail headers can be different (e.g. you may
be forwarding somebody else's e-mail but you may not be able to
forge the real "From:" line to your MUA/MTA) from what you really
want to use. On the receiving end, "am" tells "mailinfo" program to
inspect the message body with "--inbody-headers" option. After
"mailinfo" inspects the message, "am" invokes the underlying
machinery to actually make the commit (which corresponds to "git
commit"), but at that point the invoked "git commit" does not even
see these in-body header lines. That is how division of labor
between these layers of the commands are structured.
^ permalink raw reply
* Re: In-body from headers ignored
From: Liam Breck @ 2017-02-06 0:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqa8a0431z.fsf@gitster.mtv.corp.google.com>
On Sun, Feb 5, 2017 at 3:43 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Liam Breck <liam@networkimprov.net> writes:
>
>> git format-patch & send-email generate the in-body From header.
>>
>> git am recognizes it.
>>
>> git commit & format-patch & send-email ignore it. (The latter two will
>> add a new header above an extant one.) Is there a rationale for this?
>
> I may be misunderstanding you, but I am guessing that you are
> expecting
>...
> and seeing that the real author is still you, the title is "The real
> title of the patch", and the first paragraph of the body consists of
> these two lines that begin with "From: and "Subject:".
>
> This is very much deliberate. "git commit" does not care if the
> second paragraph in the body of the message resembles e-mail
> headers, because it is a command that can be used by people who do
> not even e-mail patches.
> ...
Hi, thanks for the detailed reply :-)
OK, that resolves my Q about git commit.
But should not git format-patch & send-email detect an extant in-body
From or Subject header?
Suppose I need to forward a patchset and cannot amend the commits in
git? Or that a patchset author is not the copyright holder, whom I
want to be logged as the author upstream?
There is a workaround: delete the From headers in the patches, but I
was looking for a command-line option or alternate default behavior
:-)
^ permalink raw reply
* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Junio C Hamano @ 2017-02-06 0:15 UTC (permalink / raw)
To: Siddharth Kannan; +Cc: git, pranit.bauva, Matthieu.Moy, peff, pclouds, sandals
In-Reply-To: <1486299439-2859-1-git-send-email-kannan.siddharth12@gmail.com>
Siddharth Kannan <kannan.siddharth12@gmail.com> writes:
> @@ -158,6 +158,51 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>
> if (quiet)
> rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> +
> + /*
> + * Check if any argument has a "-" in it, which has been referred to as a
> + * shorthand for @{-1}. Handles methods that might be used to list commits
> + * as mentioned in git rev-list --help
> + */
> +
> + for(i = 0; i < argc; ++i) {
> + if (!strcmp(argv[i], "-")) {
> + argv[i] = "@{-1}";
> + } else if (!strcmp(argv[i], "^-")) {
> + argv[i] = "^@{-1}";
> + } else if (strlen(argv[i]) >= 4) {
> +
> + ...
> + }
> + }
> +
> argc = setup_revisions(argc, argv, rev, opt);
"Turn '-' to '@{-1}' before we do the real parsing" can never be a
reasonable strategy to implement the desired "'-' means the tip of
the previous branch" correctly. To understand why, you only need to
imagine what happens to this command:
$ git log --grep '^-'
Turning it into "git log --grep '^@{-1}'" obviously is not what the
end-users want, so that is an immediate bug in the version of Git
with this patch applied.
Even if this were not a patch for the "log" command but for some
other command, a change with the above approach is very much
unwelcome, even if that other command does not currently have any
option that takes arbitrary string the user may want to specify
(like "find commit with a line that matches this pattern" option
called "--grep" the "log" command has). That is because it will
make it impossible to enhance the command by adding such an option
in the future. So it is also adding the problems to future
developers (and users) of Git.
A correct solution needs to know if the argument is at the position
where a revision (or revision range) is expected and then give the
tip of the previous branch when it sees "-" (and other combinations
this patch tries to cover). In other words, the parser always knows
what it is parsing, and if and only if it is parsing a rev, react to
"-" and think "ah, the user wants me to use the tip of the previous
branch".
But the code that knows that it expects to see a revision already
exists, and it is the real parser. In the above snippet,
setup_revisions() is the one that does the real parsing of argv[].
The code there knows when it wants to see a rev, and takes argv[i]
and turns into an object to call add_pending_object(). That codepath
may not yet know that "-" means the tip of the previous branch, and
that is where the change needs to go.
Such a properly-done update does not need to textually replace "-"
with "@{-1}" in argv[]; the codepath is where it understands what
any textual representation of a rev the user gave it means, and it
understands "@{-1}" there. It would be the matter of updating it to
also understand what "-" means.
A correct solution will be a lot more involved, of course, and I
think it will be larger than a reasonable microproject for people
new to the codebase.
I didn't check the microproject ideas page myself; whether it says
that turning "-" unconditionally to "@{-1}" is a good idea, or it
hints that supporting "-" as "the tip of the previous branch" in
more commands is a reasonable byte-sized microproject, I think it is
misleading and misguided. Can somebody remove that entry so that we
won't waste time of new developers (which would lead to discouraging
them)? Thanks.
^ permalink raw reply
* Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"
From: Siddharth Kannan @ 2017-02-06 2:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, pranit.bauva, Matthieu.Moy, peff, pclouds, sandals
In-Reply-To: <xmqqtw882n08.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Sun, Feb 05, 2017 at 04:15:03PM -0800, Junio C Hamano wrote:
> Siddharth Kannan <kannan.siddharth12@gmail.com> writes:
>
> > @@ -158,6 +158,51 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
> >
> > if (quiet)
> > rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> > +
> > + /*
> > + * Check if any argument has a "-" in it, which has been referred to as a
> > + * shorthand for @{-1}. Handles methods that might be used to list commits
> > + * as mentioned in git rev-list --help
> > + */
> > +
> > + for(i = 0; i < argc; ++i) {
> > + if (!strcmp(argv[i], "-")) {
> > + argv[i] = "@{-1}";
> > + } else if (!strcmp(argv[i], "^-")) {
> > + argv[i] = "^@{-1}";
> > + } else if (strlen(argv[i]) >= 4) {
> > +
> > + ...
> > + }
> > + }
> > +
> > argc = setup_revisions(argc, argv, rev, opt);
>
> "Turn '-' to '@{-1}' before we do the real parsing" can never be a
> reasonable strategy to implement the desired "'-' means the tip of
> the previous branch" correctly. To understand why, you only need to
> imagine what happens to this command:
>
> $ git log --grep '^-'
>
> Turning it into "git log --grep '^@{-1}'" obviously is not what the
> end-users want, so that is an immediate bug in the version of Git
> with this patch applied.
>
> Even if this were not a patch for the "log" command but for some
> other command, a change with the above approach is very much
> unwelcome, even if that other command does not currently have any
> option that takes arbitrary string the user may want to specify
> (like "find commit with a line that matches this pattern" option
> called "--grep" the "log" command has). That is because it will
> make it impossible to enhance the command by adding such an option
> in the future. So it is also adding the problems to future
> developers (and users) of Git.
Understood!
>
> A correct solution needs to know if the argument is at the position
> where a revision (or revision range) is expected and then give the
> tip of the previous branch when it sees "-" (and other combinations
> this patch tries to cover). In other words, the parser always knows
> what it is parsing, and if and only if it is parsing a rev, react to
> "-" and think "ah, the user wants me to use the tip of the previous
> branch".
Ah, okay. I will do another one of the suggestions as my micro project
but continue to look into this part of the code and try to find the
right place to write the code to implement the present patch.
>
> But the code that knows that it expects to see a revision already
> exists, and it is the real parser. In the above snippet,
> setup_revisions() is the one that does the real parsing of argv[].
> The code there knows when it wants to see a rev, and takes argv[i]
> and turns into an object to call add_pending_object(). That codepath
> may not yet know that "-" means the tip of the previous branch, and
> that is where the change needs to go.
>
> Such a properly-done update does not need to textually replace "-"
> with "@{-1}" in argv[]; the codepath is where it understands what
> any textual representation of a rev the user gave it means, and it
> understands "@{-1}" there. It would be the matter of updating it to
> also understand what "-" means.
>
> A correct solution will be a lot more involved, of course, and I
> think it will be larger than a reasonable microproject for people
> new to the codebase.
>
> I didn't check the microproject ideas page myself; whether it says
> that turning "-" unconditionally to "@{-1}" is a good idea, or it
> hints that supporting "-" as "the tip of the previous branch" in
> more commands is a reasonable byte-sized microproject, I think it is
> misleading and misguided. Can somebody remove that entry so that we
> won't waste time of new developers (which would lead to discouraging
> them)? Thanks.
Thanks a lot for writing this detailed reply! I will definitely take
into account all of the points mentioned here in the future patches I
send.
- Siddharth Kannan
^ permalink raw reply
* Re: [PATCH v3] parse-remote: remove reference to unused op_prep
From: Siddharth Kannan @ 2017-02-06 2:28 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Git List
In-Reply-To: <CAFZEwPOdL4mOAnmTUqs5LmfdG2GQCieVGVQ7T3ZWR0n+d6tCQQ@mail.gmail.com>
Hey Pranit,
On Sun, Feb 05, 2017 at 02:45:46AM +0530, Pranit Bauva wrote:
> Hey Siddharth,
>
> On Sat, Feb 4, 2017 at 8:01 PM, Siddharth Kannan
> <kannan.siddharth12@gmail.com> wrote:
> > The error_on_missing_default_upstream helper function learned to
> > take op_prep argument with 15a147e618 ("rebase: use @{upstream}
> > if no upstream specified", 2011-02-09), but as of 045fac5845
> > ("i18n: git-parse-remote.sh: mark strings for translation",
> > 2016-04-19), the argument is no longer used. Remove it.
> >
> > Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
>
> This looks good to me! Thanks :)
>
> Regards,
> Pranit Bauva
Should I send this patch with "To:" set to Junio and "Cc:" set to the
mailing list, as mentioend in the SubmittingPatches document?
- Siddharth Kannan
^ permalink raw reply
* [PATCH] Document the --no-gui option in difftool
From: Denton Liu @ 2017-02-06 3:41 UTC (permalink / raw)
To: git; +Cc: davvid
Prior to this, the `--no-gui` option was not documented in the manpage.
This commit introduces this into the manpage
Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
Documentation/git-difftool.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 224fb3090..a2661d9cc 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -87,9 +87,11 @@ instead. `--no-symlinks` is the default on Windows.
-g::
--gui::
+--no-gui::
When 'git-difftool' is invoked with the `-g` or `--gui` option
the default diff tool will be read from the configured
- `diff.guitool` variable instead of `diff.tool`.
+ `diff.guitool` variable instead of `diff.tool`. The `--no-gui`
+ option can be used to override this setting.
--[no-]trust-exit-code::
'git-difftool' invokes a diff tool individually on each file.
--
2.12.0.rc0.208.g81c5d00b2
^ permalink raw reply related
* Re: [PATCH 1/4] git-prompt.sh: add submodule indicator
From: Stefan Beller @ 2017-02-06 4:23 UTC (permalink / raw)
To: Junio C Hamano
Cc: Benjamin Fuchs, git@vger.kernel.org, SZEDER Gábor,
brian m. carlson, ville.skytta
In-Reply-To: <xmqqk29bsz2o.fsf@gitster.mtv.corp.google.com>
On Mon, Jan 30, 2017 at 7:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Benjamin Fuchs <email@benjaminfuchs.de> writes:
>
>> In [2/4] I got rid of the loop by feedback of Gábor.
>> Sorry if my patch wasn't well formed.
>
> While it might be the way other development communities work, in the
> Git development community, we do not work that way when presenting
> your second and subsequent attempt to the community.
>
> Having the initial draft from the original developers that records
> the bugs and misdesigns in an earlier parts of a series and separate
> patches that record how the problems were fixed to arrive at the
> final shape of the codebase might be interesting to the original
> developers, and they may even find such a history valuable, but in
> the context of the history that will be recorded in the official
> tree of the project for eternity, that just adds useless noise.
>
> Instead of keeping the original, in which problems were pointed out,
> and adding later commits to correct them incrementally, a patch is
> "rerolled". That is, you are expected to learn from the review
> comments and pretend as if you did the work from scratch and you
> already possessed the wisdom lent by the reviewers when you started
> your work. In the "rerolled" patches you send, you pretend as if
> you worked without making mistakes you made in the earlier rounds at
> all, producing (more) perfect patches from the beginning.
>
> In reality, you may locally be using Git tools like rebase,
> cherry-pick and "add -p" while preparing these "rerolled" rounds of
> patches, but the name of the game is to hide that effort from the
> public and pretend to be a perfect human, recording the result of
> exercising your best ability in the official history ;-).
>
> So this is OK:
>
> 0/3: I want to improve X, and for that I identified that I need
> A, B and C done. A or B alone is already an improvement, but A
> and B together makes it even more useful, and implementation of
> C requires patches to do A and B.
>
> 1/3: do A
> 2/3: do B
> 3/3: do C, building on A and B
>
> This is not:
>
> 0/3: I want to improve X, and for that I need to do C.
> 1/3: I couldn't do C, and I did A instead.
> 2/3: A was totally useless. I fix it to do B.
> 3/3: B is not very useful, either. I fix it to do C.
>
I agree with Junio here,
"git rebase --interactive" and then editing/squashing commits
is your friend.
(unrelated side note:)
At GitMerge facebook presented their improvements on mercurial
and one of the things was "hg absorb". It would take the dirty hunks/lines
of the working tree and amend them into the "stack of commits", i.e. into
your local unpublished history. So instead of making fixup commits
and doing the whole interactive rebase thing, it would do it automatically
for you. I think that is a neat time saver.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH 1/4] git-prompt.sh: add submodule indicator
From: Jacob Keller @ 2017-02-06 5:55 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Benjamin Fuchs, git@vger.kernel.org,
SZEDER Gábor, brian m. carlson, ville.skytta
In-Reply-To: <CAGZ79kY_1ELUZ2wZwNbQ+HrDnRBM3ngt9HKHKPmvaJEcoAFTtg@mail.gmail.com>
On Sun, Feb 5, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote:
>
> (unrelated side note:)
> At GitMerge facebook presented their improvements on mercurial
> and one of the things was "hg absorb". It would take the dirty hunks/lines
> of the working tree and amend them into the "stack of commits", i.e. into
> your local unpublished history. So instead of making fixup commits
> and doing the whole interactive rebase thing, it would do it automatically
> for you. I think that is a neat time saver.
>
> Thanks,
> Stefan
How exactly was it different from doing "git commit --fixup xyz" and
"git rebase -i --autosquash"? Like, what was the advantage to the user
facing workflow? Just curious to see if we could learn something from
it.
Regards,
Jake
^ permalink raw reply
* git/git-scm.com GH Issue Tracker
From: Samuel Lijin @ 2017-02-06 6:15 UTC (permalink / raw)
To: git@vger.kernel.org
I've went through a bunch of open issues on the git/git-scm.com repo
(specifically, everything after #600) and I think the bulk of them can
be closed.
I've taken the liberty of classifying them as shown below.
- Sam
# Irrelevant but someone should take a look
693
# Irrelevant to git-scm.com and should be closed
# Each of these had had someone comment on it saying as much
939 912 906 905 901 896 894 886 885 884 883 879 877 871 868 865 861 840
837 834 828 813 811 807 796 795 790 774 751 748 745 744 729 727 721 719
711 690 686 686 674 673 671 667 660 653 635 631 621 615 613 612 611 610
609 608
# Resolved, duplicate, or non-issue
936 875 863 858 856 839 786 785 761 760 754 752 736 723 720 704 684 683
675 663 662 661 657 651 649 640 637 634 633 628 623 616 614 605 602 601
# Relevant and should be kept open
929 890 859 855 854 826 812 808 804 787 777 768 747 715 703 701 695 694
678 668 665 649 646 639 620 617
^ permalink raw reply
* subtree merging fails
From: Stavros Liaskos @ 2017-02-06 8:48 UTC (permalink / raw)
To: git
Hi,
Please check this issue for a description of the bug:
https://github.com/git/git-scm.com/issues/896#issuecomment-277587626
Regards,
Stavros
^ permalink raw reply
* Re: git/git-scm.com GH Issue Tracker
From: Thomas Ferris Nicolaisen @ 2017-02-06 8:59 UTC (permalink / raw)
To: Samuel Lijin; +Cc: git@vger.kernel.org, Jeff King
In-Reply-To: <CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com>
Adding Peff to cc as he is the current maintainer of the git-scm.com site/repo.
On Mon, Feb 6, 2017 at 7:15 AM, Samuel Lijin <sxlijin@gmail.com> wrote:
>
> I've taken the liberty of classifying them as shown below.
>
As a community member who cares a lot about that site, thank you! I
would love to contribute by reviewing your reviews, but personally
can't find the time (focusing free time on podcast production from
Git-Merge instead ;)).
^ permalink raw reply
* Re: [PATCH 1/4] git-prompt.sh: add submodule indicator
From: Stefan Beller @ 2017-02-06 10:13 UTC (permalink / raw)
To: Jacob Keller
Cc: Junio C Hamano, Benjamin Fuchs, git@vger.kernel.org,
SZEDER Gábor, brian m. carlson, ville.skytta
In-Reply-To: <CA+P7+xp0QJZkiYzgBhPvYPsG7iqRDhRQUjcdgf_GHU-93bSO-g@mail.gmail.com>
On Sun, Feb 5, 2017 at 9:55 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Sun, Feb 5, 2017 at 8:23 PM, Stefan Beller <sbeller@google.com> wrote:
>>
>> (unrelated side note:)
>> At GitMerge facebook presented their improvements on mercurial
>> and one of the things was "hg absorb". It would take the dirty hunks/lines
>> of the working tree and amend them into the "stack of commits", i.e. into
>> your local unpublished history. So instead of making fixup commits
>> and doing the whole interactive rebase thing, it would do it automatically
>> for you. I think that is a neat time saver.
>>
>> Thanks,
>> Stefan
>
> How exactly was it different from doing "git commit --fixup xyz" and
> "git rebase -i --autosquash"? Like, what was the advantage to the user
> facing workflow? Just curious to see if we could learn something from
> it.
My impression is that all local changes were split up and the "xyz"
was determined based off a heuristic, e.g. blame(?) and then the
rebase is run automatically after that, i.e. having just one command for
a complete workflow here, moving up a whole level in abstraction.
^ permalink raw reply
* Re: git/git-scm.com GH Issue Tracker
From: Duy Nguyen @ 2017-02-06 10:18 UTC (permalink / raw)
To: Samuel Lijin; +Cc: git@vger.kernel.org
In-Reply-To: <CAJZjrdX_8tjMhRac9QQOW8m_S2DprFPV=uZp8mFT+g6bASVd-w@mail.gmail.com>
On Mon, Feb 6, 2017 at 1:15 PM, Samuel Lijin <sxlijin@gmail.com> wrote:
> # Irrelevant but someone should take a look
>
> 693
To save people some time (and since i looked at it anyway), this is
about whether "warning in tree xxx: contains zero-padded file modes:
from fsck should be a warning or error. It is a warning now even
though "git -c transfer.fsckobjects=true clone" treats it as an error.
There are some discussions in the past [1] [2] about this.
There's also a question "And I failed to find in the documentation if
transfer.fsckobjects could be disabled per repository, can you confirm
it's not possible for now ?"
(sorry no answer from me)
[1] http://public-inbox.org/git/%3CCAEBDL5W3DL0v=TusuB7Vg-4bWdAJh5d2Psc1N0Qe+KK3bZH3=Q@mail.gmail.com%3E/
[2] http://public-inbox.org/git/%3C20100326215600.GA10910@spearce.org%3E/
--
Duy
^ permalink raw reply
* Re: gitconfig get out of sync with submodule entries on branch switch
From: Stefan Beller @ 2017-02-06 10:35 UTC (permalink / raw)
To: Benjamin Schindler; +Cc: git@vger.kernel.org
In-Reply-To: <0f14df64-1aa2-e671-9785-4e5e0a076ae6@gmail.com>
Answering the original email, as I feel we're going down the wrong rabbit
hole in the existing thread.
On Mon, Jan 30, 2017 at 8:21 AM, Benjamin Schindler
<beschindler@gmail.com> wrote:
> Hi
>
> Consider the following usecase: I have the master branch where I have a
> submodule A. I create a branch where I rename the submodule to be in the
> directory B. After doing all of this, everything looks good.
> Now, I switch back to master. The first oddity is, that it fails to remove
> the folder B because there are still files in there:
>
> bschindler@metis ~/Projects/submodule_test (testbranch) $ git checkout
> master
> warning: unable to rmdir other_submodule: Directory not empty
> Switched to branch 'master'
checkout currently doesn't support submodules, so it should neither
try to delete B nor try to repopulate A when switching back to master.
checkout ought to not even touch the existing submodule B.
>
> Git submodule deinit on B fails because the submodule is not known to git
> anymore (after all, the folder B exists only in the other branch). I can
> easily just remove the folder B from disk and initialize the submodule A
> again, so all seems good.
by initializing you mean populating(?), i.e.
git submodule update
would work without the --init flag or preceding "git submodule init A".
That ought to not redownload A, but just put files back in the working tree
from the submodule git directory inside the superprojects git dir.
>
> However, what is not good is that the submodule b is still known in
> .git/config.
Oh, I see. You did not just rename the path, but also the name
in the .gitmodules?
> This is in particular a problem for us, because I know a number
> of tools which use git config to retrieve the submodule list. Is it
> therefore a bug that upon branch switch, the submodule gets deregistered,
> but its entry in .git/config remains?
The config remains as it indicates that you express(ed) interest in
submodule A, such that when switching branches
master->renamedToB->master
then we'd still care about A. As for the tools, I'd rather see them use
git submodule status/summary
instead of directly looking at the config, because the config may
change in the future.
>
> thanks a lot
> Benjamin Schindler
>
> P.s. I did not subscribe to the mailing list, please add me at least do CC.
> Thanks
^ permalink raw reply
* Re: gitconfig get out of sync with submodule entries on branch switch
From: Benjamin Schindler @ 2017-02-06 12:17 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kaZUOO4qusCDF9=VJ-6QPjAvc5eSaazjWWEocRMHuTSug@mail.gmail.com>
On 06.02.2017 11:35, Stefan Beller wrote:
> Answering the original email, as I feel we're going down the wrong rabbit
> hole in the existing thread.
>
> On Mon, Jan 30, 2017 at 8:21 AM, Benjamin Schindler
> <beschindler@gmail.com> wrote:
>> Hi
>>
>> Consider the following usecase: I have the master branch where I have a
>> submodule A. I create a branch where I rename the submodule to be in the
>> directory B. After doing all of this, everything looks good.
>> Now, I switch back to master. The first oddity is, that it fails to remove
>> the folder B because there are still files in there:
>>
>> bschindler@metis ~/Projects/submodule_test (testbranch) $ git checkout
>> master
>> warning: unable to rmdir other_submodule: Directory not empty
>> Switched to branch 'master'
>
> checkout currently doesn't support submodules, so it should neither
> try to delete B nor try to repopulate A when switching back to master.
> checkout ought to not even touch the existing submodule B.
Well, it tried to remove the folder (the rmdir warning) but it failed so
in some sense you are right. Is there a technical reason for this
default though? Here, I frequently have to point out to people that they
need to initialize/update the submodule on e.g. clone.
>
>>
>> Git submodule deinit on B fails because the submodule is not known to git
>> anymore (after all, the folder B exists only in the other branch). I can
>> easily just remove the folder B from disk and initialize the submodule A
>> again, so all seems good.
>
> by initializing you mean populating(?), i.e.
>
> git submodule update
>
> would work without the --init flag or preceding "git submodule init A".
> That ought to not redownload A, but just put files back in the working tree
> from the submodule git directory inside the superprojects git dir.
>
>>
>> However, what is not good is that the submodule b is still known in
>> .git/config.
>
> Oh, I see. You did not just rename the path, but also the name
> in the .gitmodules?
I wasn't even aware that the submodule name was something different from
the path because the name is by default set to be the path to it. So
yes, I didn't just relocate it, it had a different name.
>
>> This is in particular a problem for us, because I know a number
>> of tools which use git config to retrieve the submodule list. Is it
>> therefore a bug that upon branch switch, the submodule gets deregistered,
>> but its entry in .git/config remains?
>
> The config remains as it indicates that you express(ed) interest in
> submodule A, such that when switching branches
>
> master->renamedToB->master
>
> then we'd still care about A. As for the tools, I'd rather see them use
>
> git submodule status/summary
>
> instead of directly looking at the config, because the config may
> change in the future.
That was my feeling but its good to know to have more solid reasons why
that would be.
Cheers
Benjamin
>
>>
>> thanks a lot
>> Benjamin Schindler
>>
>> P.s. I did not subscribe to the mailing list, please add me at least do CC.
>> Thanks
^ permalink raw reply
* A little help understanding output from git blame --reverse
From: Edmundo Carmona Antoranz @ 2017-02-06 12:38 UTC (permalink / raw)
To: Git List
Hi!
While doing some research developing difflame I found some output from
git blame --reverse that I can't quite understand. Perhaps another set
of eyeballs could help me.
I'm "difflaming" HEAD~100 (02db2d0421b97fcb6211) and HEAD
(066fb0494e6398eb). Specifically file abspath.c.
If we diff (as in plain old git diff) HEAD~100..HEAD we can see that
line 63 (from HEAD~100 revision) was deleted between HEAD~100 and
HEAD:
@@ -58,86 +95,136 @@ blah blah
goto error_out;
}
- strbuf_reset(&sb);
- strbuf_addstr(&sb, path);
-
- while (depth--) {
- if (!is_directory(sb.buf)) {
So, if I do a "reverse" blame operation on the file, I would expect to
see the last revision where that line was _present_ on the file:
c5f3cba126 61) strbuf_reset(&sb);
c5f3cba126 62) strbuf_addstr(&sb, path);
066fb0494e 63)
c5f3cba126 64) while (depth--) {
c5f3cba126 65) if (!is_directory(sb.buf)) {
line 63 shows up as if it had been last present on the file on
revision 066fb0494e, which is HEAD, which kind of doesn't make a lot
of sense to me because given that the line is not present on the file
on HEAD (as we can guess from diff output) it means it was "forcefully
present" on some previous revision (and that's what I would expect to
see reported on blame --reverse output).
What am I missing? Thanks in advance.
^ permalink raw reply
* Re: A little help understanding output from git blame --reverse
From: Edmundo Carmona Antoranz @ 2017-02-06 12:40 UTC (permalink / raw)
To: Git List
In-Reply-To: <CAOc6etZ7iuPKRQkYSZDrDRW0hxbu1aYMRuzB1iXAPv+EEnXJEg@mail.gmail.com>
On Mon, Feb 6, 2017 at 6:38 AM, Edmundo Carmona Antoranz
<eantoranz@gmail.com> wrote:
> I'm "difflaming" HEAD~100 (02db2d0421b97fcb6211) and HEAD
> (066fb0494e6398eb). Specifically file abspath.c.
I just noticed that I'm standing on a private branch. Replace HEAD for
"4e59582ff" when doing your analysis. You should get the same results.
^ permalink raw reply
* [PATCH] worktree: fix option descriptions for `prune`
From: Patrick Steinhardt @ 2017-02-06 13:13 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Nguyễn Thái Ngọc Duy, ps
The `verbose` and `expire` options of the `git worktree prune`
subcommand have wrong descriptions in that they pretend to relate to
objects. But as the git-worktree(1) correctly states, these options have
nothing to do with objects but only with worktrees. Fix the description
accordingly.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
builtin/worktree.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 9a97e37a3..831fe058a 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -125,9 +125,9 @@ static int prune(int ac, const char **av, const char *prefix)
{
struct option options[] = {
OPT__DRY_RUN(&show_only, N_("do not remove, show only")),
- OPT__VERBOSE(&verbose, N_("report pruned objects")),
+ OPT__VERBOSE(&verbose, N_("report pruned working trees")),
OPT_EXPIRY_DATE(0, "expire", &expire,
- N_("expire objects older than <time>")),
+ N_("expire working trees older than <time>")),
OPT_END()
};
--
2.11.1
^ permalink raw reply related
* [PATCH v3] tag: generate useful reflog message
From: cornelius.weig @ 2017-02-06 13:58 UTC (permalink / raw)
To: git; +Cc: Cornelius Weig, karthik.188, peff, gitster
In-Reply-To: <xmqqo9yg43uo.fsf@gitster.mtv.corp.google.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:
Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging <short-sha1> (<description>)". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
(<description>)" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:
- "tree object"
- "blob object"
- "other tag object"
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
builtin/tag.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
t/t7004-tag.sh | 16 +++++++++++++++-
2 files changed, 68 insertions(+), 2 deletions(-)
diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..638b68e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,
}
}
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+ enum object_type type;
+ struct commit *c;
+ char *buf;
+ unsigned long size;
+ int subject_len = 0;
+ const char *subject_start;
+
+ char *rla = getenv("GIT_REFLOG_ACTION");
+ if (rla) {
+ strbuf_addstr(sb, rla);
+ } else {
+ strbuf_addstr(sb, _("tag: tagging "));
+ strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+ }
+
+ strbuf_addstr(sb, " (");
+ type = sha1_object_info(sha1, NULL);
+ switch (type) {
+ default:
+ strbuf_addstr(sb, _("internal object"));
+ break;
+ case OBJ_COMMIT:
+ c = lookup_commit_reference(sha1);
+ buf = read_sha1_file(sha1, &type, &size);
+ if (!c || !buf) {
+ die(_("commit object %s could not be read"),
+ sha1_to_hex(sha1));
+ }
+ subject_len = find_commit_subject(buf, &subject_start);
+ strbuf_insert(sb, sb->len, subject_start, subject_len);
+ strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
+ free(buf);
+ break;
+ case OBJ_TREE:
+ strbuf_addstr(sb, _("tree object"));
+ break;
+ case OBJ_BLOB:
+ strbuf_addstr(sb, _("blob object"));
+ break;
+ case OBJ_TAG:
+ strbuf_addstr(sb, _("other tag object"));
+ break;
+ }
+ strbuf_addch(sb, ')');
+}
+
struct msg_arg {
int given;
struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
{
struct strbuf buf = STRBUF_INIT;
struct strbuf ref = STRBUF_INIT;
+ struct strbuf reflog_msg = STRBUF_INIT;
unsigned char object[20], prev[20];
const char *object_ref, *tag;
struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
else
die(_("Invalid cleanup mode %s"), cleanup_arg);
+ create_reflog_msg(object, &reflog_msg);
+
if (create_tag_object) {
if (force_sign_annotate && !annotate)
opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (!transaction ||
ref_transaction_update(transaction, ref.buf, object, prev,
create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
- NULL, &err) ||
+ reflog_msg.buf, &err) ||
ref_transaction_commit(transaction, &err))
die("%s", err.buf);
ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
strbuf_release(&err);
strbuf_release(&buf);
strbuf_release(&ref);
+ strbuf_release(&reflog_msg);
return 0;
}
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..3c4cb58 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
test_must_fail git reflog exists refs/tags/mytag
'
+git log -1 > expected \
+ --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
test_expect_success 'creating a tag with --create-reflog should create reflog' '
test_when_finished "git tag -d tag_with_reflog" &&
git tag --create-reflog tag_with_reflog &&
- git reflog exists refs/tags/tag_with_reflog
+ git reflog exists refs/tags/tag_with_reflog &&
+ sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+ test_cmp expected actual
+'
+
+git log -1 > expected \
+ --format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+ test_when_finished "git tag -d tag_with_reflog" &&
+ git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+ git reflog exists refs/tags/tag_with_reflog &&
+ sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+ test_cmp expected actual
'
test_expect_success '--create-reflog does not create reflog on failure' '
--
2.10.2
^ permalink raw reply related
* Re: [PATCH] p5302: create repositories for index-pack results explicitly
From: Jeff King @ 2017-02-06 14:41 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git List
In-Reply-To: <251bdc20-19a7-9a6c-9f5a-c7e661810c70@web.de>
On Sun, Feb 05, 2017 at 12:43:29PM +0100, René Scharfe wrote:
> Before 7176a314 (index-pack: complain when --stdin is used outside of a
> repo) index-pack silently created a non-existing target directory; now
> the command refuses to work unless it's used against a valid repository.
> That causes p5302 to fail, which relies on the former behavior. Fix it
> by setting up the destinations for its performance tests using git init.
Ah, right. Thanks for catching this.
I think p5302 was wrong to rely on the old behavior, and your patch is
the right fix.
-Peff
^ permalink raw reply
* Re: [PATCH v3 5/5] stash: teach 'push' (and 'create') to honor pathspec
From: Jeff King @ 2017-02-06 15:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Gummerer, git, Stephan Beyer, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <xmqqmvdz3ied.fsf@gitster.mtv.corp.google.com>
On Sun, Feb 05, 2017 at 11:09:14PM -0800, Junio C Hamano wrote:
> > @@ -99,6 +104,10 @@ create_stash () {
> > if test -z "$new_style"
> > then
> > stash_msg="$*"
> > + while test $# != 0
> > + do
> > + shift
> > + done
> > fi
>
> The intent is correct, but I would probaly empty the "$@" not with
> an iteration of shifts but with a single
>
> set x && shift
>
> ;-)
Perhaps it is not portable, but I think "set --" does the same thing and
is perhaps less obscure.
-Peff
^ permalink raw reply
* Re: [PATCH v3 1/5] Documentation/stash: remove mention of git reset --hard
From: Jeff King @ 2017-02-06 15:22 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170205202642.14216-2-t.gummerer@gmail.com>
On Sun, Feb 05, 2017 at 08:26:38PM +0000, Thomas Gummerer wrote:
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2e9cef06e6..2e9e344cd7 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -47,8 +47,9 @@ OPTIONS
>
> save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
>
> - Save your local modifications to a new 'stash', and run `git reset
> - --hard` to revert them. The <message> part is optional and gives
> + Save your local modifications to a new 'stash' and roll them
> + back to HEAD (in the working tree and in the index).
> + The <message> part is optional and gives
Nice. I think what you ended up with here is concise and accurate.
-Peff
^ permalink raw reply
* Cross-referencing the Git mailing list archive with their corresponding commits in `pu`
From: Johannes Schindelin @ 2017-02-06 15:34 UTC (permalink / raw)
To: Josh Triplett; +Cc: git
Hi Josh,
as discussed at the GitMerge, I am trying to come up with tooling that
will allow for substantially less tedious navigation between the local
repository, the mailing list, and what ends up in the `pu` branch.
That tooling would *still* not help lowering the barrier of entry for
contributing to Git by a lot, as it would *still* not address the problem
that mails sent from the most prevalent desktop mail client, as well as
mails sent from the most prevalent web mail client, are simply and
unceremoniously dropped. (This problem was acknowledged by quite a few
nods even at the Contributors' Summit...) But still, we decided to start
*somewhere* and this tooling is what we agreed on.
It is quite a bit harder going than I would like: as we have figured out,
the Subject: line is not a good way to link the commits with the original
mails containing the patches, as commit messages are modified before being
pushed often enough to make this a fragile matching.
So I thought maybe the From: line (from the body, if available, otherwise
from the header) in conjunction with the "Date:" header would work. But a
preliminary study shows that there are 336 From: + Date: combinations in
the Git mailing list archive that are not unique. 71 of these are shared
by three or more mails, even, and 9 are shared by more than 10 mails,
respectively. This is bad!
Unsurprisingly, the top 10 of these cases were obviously caused by the
builtin `git am` bug where it would not reset the author date properly.
Surprisingly, though, there were a few cases from 2005, too.
I had a quick look to find out what was the culprit (looking at the
17-strong patch series "Documentation fixes in response to my previous
listing" by Nikolai Weibull, but I am at a loss there: the mail claims to
be sent by git-send-email and the patches appear to be generated by
git-format-patch as of v0.99.9l, neither of which had a Date:-related bug
back in that time frame. My best guess is that the patches were mishandled
by a tool similar to rebase -i (which entered Git only at v1.5.3).
For details, see:
http://public-inbox.org/git/11340844841342-git-send-email-mailing-lists.git@rawuncut.elitemail.org/
(this is also an example where public-inbox' thread detection went utterly
wrong, including way too many mails in the "thread")
There was even a case of duplicated Date: headers in 2012. Now, this case
is very curious, as there have been 7 mails with identical Date: header,
but it was not a 6-strong patch series. Instead, it was a 4-strong patch
series that needed three iterations before it was accepted, and the
identical Date: header appears only in v2's patches (*not* in its cover
letter) and it *disappeared* in v3's 4/4, where it was set *back* by a
week (to the Date: it had in v1).
For details, see
http://public-inbox.org/git/cover.1354693001.git.Sebastian.Leske@sleske.name/
and
http://public-inbox.org/git/cover.1354324110.git.Sebastian.Leske@sleske.name/
and
http://public-inbox.org/git/b115a546fa783b4121d118bb8fdb9270443f90fa.1353691892.git.Sebastian.Leske@sleske.name/
This last example also demonstrates a very curious test case for a
different difficulty in trying to reconstruct lost correspondences: the
patch series was applied *twice*, independently of each other. First, on
the day v3 was submitted, it was applied on top of v1.8.1-rc0 (as commits
ee26a6e2b8..dd465ce66f), although it was not merged until v1.8.1-rc3. 22
days later, it was reapplied on top of maint so it could enter v1.8.0.3
(back then, Git still had "patchlevel" versions): c2999adcd5..008c208c2c.
As you can see, there is a many-to-many relationship here, even if you do
leave the *original* branch out of the picture entirely.
Will keep you posted,
Dscho
P.S.: I used public-inbox.org links instead of commit references to the
Git repository containing the mailing list archive, because the format of
said Git repository is so unfavorable that it was determined very quickly
in a discussion between Patrick Reynolds (GitHub) and myself that it would
put totally undue burden on GitHub to mirror it there (compare also Carlos
Nieto's talk at GitMerge titled "Top Ten Worst Repositories to host on
GitHub").
^ permalink raw reply
* Re: [PATCH v3 2/5] stash: introduce push verb
From: Jeff King @ 2017-02-06 15:46 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170205202642.14216-3-t.gummerer@gmail.com>
On Sun, Feb 05, 2017 at 08:26:39PM +0000, Thomas Gummerer wrote:
> + -m|--message)
> + shift
> + stash_msg=${1?"-m needs an argument"}
> + ;;
I think this is our first use of the "?" parameter expansion magic. It
_is_ in POSIX, so it may be fine. We may get complaints from people on
weird shell variants, though. If that's the only reason to avoid it, I'd
be inclined to try it and see, as it is much shorter.
OTOH, most of the other usage errors call usage(), and this one doesn't.
Nor is the error message translatable. Perhaps we should stick to the
longer form (and add a helper function if necessary to reduce the
boilerplate).
> +save_stash () {
> + push_options=
> + while test $# != 0
> + do
> + case "$1" in
> + --help)
> + show_help
> + ;;
> + --)
> + shift
> + break
> + ;;
> + -*)
> + # pass all options through to push_stash
> + push_options="$push_options $1"
> + ;;
> + *)
> + break
> + ;;
> + esac
> + shift
> + done
I suspect you could just let "--help" get handled in the pass-through
case (it generally takes precedence over errors found in other options,
but I do not see any other parsing errors that could be found by this
loop). It is not too bad to keep it, though (the important thing is that
we're not duplicating all of the push_stash options here).
> + if test -z "$stash_msg"
> + then
> + push_stash $push_options
> + else
> + push_stash $push_options -m "$stash_msg"
> + fi
Hmm. So $push_options is subject to word-splitting here. That's
necessary to split the options back apart. It does the wrong thing if
any of the options had spaces in them. But I don't think there are any
valid options which do so, and "save" would presumably not grow any new
options (they would go straight to "push").
So there is a detectable behavior change:
[before]
$ git stash "--bogus option"
error: unknown option for 'stash save': --bogus option
To provide a message, use git stash save -- '--bogus option'
[etc...]
[after]
$ git stash "--bogus option"
error: unknown option for 'stash save': --bogus
To provide a message, use git stash save -- '--bogus'
but it's probably an acceptable casualty (the "right" way would be to
shell-quote everything you stuff into $push_options and then eval the
result when you invoke push_stash).
Likewise, it's usually a mistake to just stick a new option (like "-m")
after a list of unknown options. But it's OK here because we know we
removed any "--" or non-option arguments.
-Peff
^ permalink raw reply
* Re: [PATCH v3 3/5] stash: add test for the create command line arguments
From: Jeff King @ 2017-02-06 15:50 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170205202642.14216-4-t.gummerer@gmail.com>
On Sun, Feb 05, 2017 at 08:26:40PM +0000, Thomas Gummerer wrote:
> +test_expect_success 'create stores correct message' '
> + >foo &&
> + git add foo &&
> + STASH_ID=$(git stash create "create test message") &&
> + echo "On master: create test message" >expect &&
> + git show --pretty=%s ${STASH_ID} | head -n1 >actual &&
> + test_cmp expect actual
> +'
This misses failure exit codes from "git show" because it's on the left
side of a pipe. Perhaps "git show -s" or "git log -1" would get you the
same output without needing "head" (and saving a process and the time
spent generating the diff, as a bonus).
Ditto in the other tests from this patch and later ones.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox