* Re: missing handling of "No newline at end of file" in git am
From: Olaf Hering @ 2017-02-14 20:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqh93w8q0r.fsf@gitster.mtv.corp.google.com>
Am Tue, 14 Feb 2017 12:40:36 -0800
schrieb Junio C Hamano <gitster@pobox.com>:
> Olaf Hering <olaf@aepfle.de> writes:
>
> > How is git send-email and git am supposed to handle a text file
> > which lacks a newline at the very end? This is about git 2.11.0.
>
> I think this has always worked, though.
For me it complains in line 721, which is the problematic one.
I try to apply from mutt via (cd /some/dir && git am), but that
probably does not make a difference.
How would I debug it?
Olaf
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Johannes Schindelin @ 2017-02-14 20:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git-for-windows, git
In-Reply-To: <xmqq60kdbqmy.fsf@gitster.mtv.corp.google.com>
Hi,
On Mon, 13 Feb 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > That is why I taught the Git for Windows CI job that tests the four
> > upstream Git integration branches to *also* bisect test breakages and
> > then upload comments to the identified commit on GitHub
>
> Good. I do not think it is useful to try 'pu' as an aggregate and
> expect it to always build and work [*1*], but your "bisect and
> pinpoint" approach makes it useful to identify individual topic that
> brings in a breakage.
Sadly the many different merge bases[*1*] between `next` and `pu` (which
are the obvious good/bad points for bisecting automatically) bring my
build agents to its knees. I may have to disable the bisecting feature as
a consequence.
Ciao,
Johannes
Footnote *1*: There are currently 21, some of which stemming back from a
year ago. For bisecting, they all have to be tested individually, putting
a major dent into bisect's otherwise speedy process.
^ permalink raw reply
* Re: [git-for-windows] Re: Continuous Testing of Git on Windows
From: Junio C Hamano @ 2017-02-14 21:08 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git-for-windows, git
In-Reply-To: <alpine.DEB.2.20.1702142150220.3496@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 13 Feb 2017, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > That is why I taught the Git for Windows CI job that tests the four
>> > upstream Git integration branches to *also* bisect test breakages and
>> > then upload comments to the identified commit on GitHub
>>
>> Good. I do not think it is useful to try 'pu' as an aggregate and
>> expect it to always build and work [*1*], but your "bisect and
>> pinpoint" approach makes it useful to identify individual topic that
>> brings in a breakage.
>
> Sadly the many different merge bases[*1*] between `next` and `pu` (which
> are the obvious good/bad points for bisecting automatically) bring my
> build agents to its knees. I may have to disable the bisecting feature as
> a consequence.
Probably a less resource intensive approach is to find the tips of
the topics not in 'next' but in 'pu' and test them. That would give
you which topic(s) are problematic, which is a better starting point
than "Oh, 'pu' does not build". After identifying which branch is
problematic, bisection of individual topic would be of more manageable
size.
$ git log --first-parent --oneline 'pu^{/^### match next}..pu'
will you the merges of topics left outside 'next'. I often reorder
to make the ones that look more OK than others closer to the bottom,
and if the breakages caused by them are caught earlier than they hit
'next', that would be ideal.
This is one of these times I wish "git bisect --first-parent" were
available. The above "log" gives me 27 merges right now, which
should be bisectable within 5 rounds to identify a single broken
topic (if there is only one breakage, that is).
^ permalink raw reply
* Re: [PATCH] completion: complete modified files for checkout with '--'
From: Cornelius Weig @ 2017-02-14 21:13 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: Git mailing list, j6t, Richard Wagner
In-Reply-To: <CAM0VKj=d+hAiF6_8TLuJfccNiPtHyg9F6zESA8SuTEeaLsrw4Q@mail.gmail.com>
On 02/14/2017 01:50 AM, SZEDER Gábor wrote:
> On Tue, Feb 14, 2017 at 12:33 AM, <cornelius.weig@tngtech.com> wrote:
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> The command line completion for git-checkout bails out when seeing '--'
>> as an isolated argument. For git-checkout this signifies the start of a
>> list of files which are to be checked out. Checkout of files makes only
>> sense for modified files,
>
> No, there is e.g. 'git checkout that-branch this-path', too.
Very true. Thanks for prodding me to this palpable oversight.
My error was to aim for a small improvement. I think the correct
approach is to improve the overall completion of git-checkout. IMHO it
is a completion bug that after giving a ref, completion will still offer
refs, e.g.
$ git checkout HEAD <TAB><TAB> --> list of refs
As far as I can see, giving two refs to checkout is always an error. The
correct behavior in the example above would be to offer paths instead.
I'll follow up with an improved version which considers these cases.
^ permalink raw reply
* [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: cornelius.weig @ 2017-02-14 21:24 UTC (permalink / raw)
To: git; +Cc: Cornelius Weig, szeder.dev, bitte.keine.werbung.einwerfen, j6t
In-Reply-To: <20170214212404.31469-1-cornelius.weig@tngtech.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
Git-checkout completes words starting with '--' as options and other
words as refs. Even after specifying a ref, further words not starting
with '--' are completed as refs, which is invalid for git-checkout.
This commit ensures that after specifying a ref, further non-option
words are completed as paths. Four cases are considered:
- If the word contains a ':', do not treat it as reference and use
regular revlist completion.
- If no ref is found on the command line, complete non-options as refs
as before.
- If the ref is HEAD or @, complete only with modified files because
checking out unmodified files is a noop.
This case also applies if no ref is given, but '--' is present.
- If a ref other than HEAD or @ is found, offer only valid paths from
that revision.
Note that one corner-case is not covered by the current implementation:
if a refname contains a ':' and is followed by '--' the completion would
not recognize the valid refname.
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
contrib/completion/git-completion.bash | 39 +++++++++++++++++++++++++++-------
1 file changed, 31 insertions(+), 8 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 4ab119d..df46f62 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1068,7 +1068,7 @@ _git_bundle ()
_git_checkout ()
{
- __git_has_doubledash && return
+ local i c=2 ref="" seen_double_dash=""
case "$cur" in
--conflict=*)
@@ -1081,13 +1081,36 @@ _git_checkout ()
"
;;
*)
- # check if --track, --no-track, or --no-guess was specified
- # if so, disable DWIM mode
- local flags="--track --no-track --no-guess" track=1
- if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
- track=''
- fi
- __gitcomp_nl "$(__git_refs '' $track)"
+ while [ $c -lt $cword ]; do
+ i="${words[c]}"
+ case "$i" in
+ --) seen_double_dash=1 ;;
+ -*|?*:*) ;;
+ *) ref="$i"; break ;;
+ esac
+ ((c++))
+ done
+
+ case "$ref,$seen_double_dash,$cur" in
+ ,,*:*)
+ __git_complete_revlist_file
+ ;;
+ ,,*)
+ # check for --track, --no-track, or --no-guess
+ # if so, disable DWIM mode
+ local flags="--track --no-track --no-guess" track=1
+ if [ -n "$(__git_find_on_cmdline "$flags")" ]; then
+ track=''
+ fi
+ __gitcomp_nl "$(__git_refs '' $track)"
+ ;;
+ ,1,*|@,*|HEAD,*)
+ __git_complete_index_file "--modified"
+ ;;
+ *)
+ __git_complete_tree_file "$ref" "$cur"
+ ;;
+ esac
;;
esac
}
--
2.10.2
^ permalink raw reply related
* [PATCH v2 1/2] completion: extract utility to complete paths from tree-ish
From: cornelius.weig @ 2017-02-14 21:24 UTC (permalink / raw)
To: git; +Cc: Cornelius Weig, szeder.dev, bitte.keine.werbung.einwerfen, j6t
In-Reply-To: <4f8a0aaa-4ce1-d4a6-d2e1-28aac7209c90@tngtech.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
The function __git_complete_revlist_file understands how to complete a
path such as 'topic:ref<TAB>'. In that case, the revision (topic) and
the path component (ref) are both part of the same word. However,
some cases require that the revision is specified elsewhere than the
current word for completion, such as 'git checkout topic ref<TAB>'.
In order to allow callers to specify the revision, extract a utility
function to complete paths from a tree-ish object. The utility will be
used later to implement path completion for git-checkout.
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
contrib/completion/git-completion.bash | 73 +++++++++++++++++++---------------
1 file changed, 41 insertions(+), 32 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6c6e1c7..4ab119d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -442,6 +442,46 @@ __git_compute_merge_strategies ()
__git_merge_strategies=$(__git_list_merge_strategies)
}
+# __git_complete_tree_file requires 2 argument:
+# 1: the the tree-like to look at for completion
+# 2: the path component to complete
+__git_complete_tree_file ()
+{
+ local pfx ls ref="$1" cur_="$2"
+ case "$cur_" in
+ ?*/*)
+ pfx="${cur_%/*}"
+ cur_="${cur_##*/}"
+ ls="$ref:$pfx"
+ pfx="$pfx/"
+ ;;
+ *)
+ ls="$ref"
+ ;;
+ esac
+
+ case "$COMP_WORDBREAKS" in
+ *:*) : great ;;
+ *) pfx="$ref:$pfx" ;;
+ esac
+
+ __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
+ | sed '/^100... blob /{
+ s,^.* ,,
+ s,$, ,
+ }
+ /^120000 blob /{
+ s,^.* ,,
+ s,$, ,
+ }
+ /^040000 tree /{
+ s,^.* ,,
+ s,$,/,
+ }
+ s/^.* //')" \
+ "$pfx" "$cur_" ""
+}
+
__git_complete_revlist_file ()
{
local pfx ls ref cur_="$cur"
@@ -452,38 +492,7 @@ __git_complete_revlist_file ()
?*:*)
ref="${cur_%%:*}"
cur_="${cur_#*:}"
- case "$cur_" in
- ?*/*)
- pfx="${cur_%/*}"
- cur_="${cur_##*/}"
- ls="$ref:$pfx"
- pfx="$pfx/"
- ;;
- *)
- ls="$ref"
- ;;
- esac
-
- case "$COMP_WORDBREAKS" in
- *:*) : great ;;
- *) pfx="$ref:$pfx" ;;
- esac
-
- __gitcomp_nl "$(git --git-dir="$(__gitdir)" ls-tree "$ls" 2>/dev/null \
- | sed '/^100... blob /{
- s,^.* ,,
- s,$, ,
- }
- /^120000 blob /{
- s,^.* ,,
- s,$, ,
- }
- /^040000 tree /{
- s,^.* ,,
- s,$,/,
- }
- s/^.* //')" \
- "$pfx" "$cur_" ""
+ __git_complete_tree_file "$ref" "$cur_"
;;
*...*)
pfx="${cur_%...*}..."
--
2.10.2
^ permalink raw reply related
* Re: [PATCH] show-branch: fix crash with long ref name
From: Christian Couder @ 2017-02-14 21:29 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git, Maxim Kuvyrkov
In-Reply-To: <20170214195513.7zae6x22advkrms6@sigill.intra.peff.net>
On Tue, Feb 14, 2017 at 8:55 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Feb 14, 2017 at 11:35:48AM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>>
>> > This fixes the problem, but I think we can simplify it quite a bit by
>> > using resolve_refdup(). Here's the patch series I ended up with:
>> >
>> > [1/3]: show-branch: drop head_len variable
>> > [2/3]: show-branch: store resolved head in heap buffer
>> > [3/3]: show-branch: use skip_prefix to drop magic numbers
Yeah, I noticed there were a number of things that could be improved
in the area, but I didn't want to spend too much time on this, so
thanks for this series.
>> Yes, the whole thing is my fault ;-) and I agree with what these
>> patches do.
>>
>> The second one lacks free(head) but I think that is OK; it is
>> something we allocate in cmd_*() and use pretty much thruout the
>> rest of the program.
>
> Yes, I actually tested the whole thing under ASAN (which was necessary
> to notice the problem), which complained about the leak. I don't mind
> adding a free(head), but there are a bunch of similar "leaks" in that
> function, so I didn't bother.
Yeah, I didn't bother either.
> I notice Christian's patch added a few tests. I don't know if we'd want
> to squash them in (I didn't mean to override his patch at all; I was
> about to send mine out when I noticed his, and I wondered if we wanted
> to combine the two efforts).
I think it would be nice to have at least one test. Feel free to
squash mine if you want.
^ permalink raw reply
* Re: [PATCH v3 4/5] stash: introduce new format create
From: Thomas Gummerer @ 2017-02-14 21:30 UTC (permalink / raw)
To: Jeff King
Cc: git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <20170213230532.sr7lpl26mcfa4gfc@sigill.intra.peff.net>
On 02/13, Jeff King wrote:
> On Mon, Feb 13, 2017 at 04:57:34PM -0500, Jeff King wrote:
>
> > Yeah, I think your patch is actually fixing that case. But your search
> > is only part of the story. You found somebody using "-m" explicitly, but
> > what about somebody blindly calling:
> >
> > git stash create $*
> >
> > That's now surprising to somebody who puts "-m" in their message.
> >
> > > I *think* this regression is acceptable, but I'm happy to introduce
> > > another verb if people think otherwise.
> >
> > Despite what I wrote above, I'm still inclined to say that this isn't an
> > important regression. I'd be surprised if "stash create" is used
> > independently much at all.
>
> Just thinking on this more...do we really care about "fixing" the
> interface of "stash create"? This is really just about refactoring what
> underlies the new "push", right?
>
> So we could just do:
>
> diff --git a/git-stash.sh b/git-stash.sh
> index 6d629fc43..ee37db135 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -711,7 +711,7 @@ clear)
> ;;
> create)
> shift
> - create_stash "$@" && echo "$w_commit"
> + create_stash -m "$*" && echo "$w_commit"
> ;;
> store)
> shift
>
> on top of your patch and keep the external interface the same.
>
> It might be nice to clean up the interface for "create" to match other
> ones, but from this discussion I think it is mostly a historical wart
> for scripting, and we are OK to just leave its slightly-broken interface
> in place forever.
Yeah tbh I don't personally care too much about modernizing the
interface to git stash create. What you have above makes a lot of
sense to me.
I also just noticed that git stash create -m message is not the worst
regression I introduced when trying to modernize this. In that case
only the -m would go missing, but that's probably not the end of the
world. The worse thing to do would be something like
git stash create -u untracked, where the intended message was "-u
untracked", but instead there is no message, but all untracked files
are now included in the stash as well.
In that light what you have above makes even more sense to me.
Thanks!
> I could go either way.
>
> -Peff
--
Thomas
^ permalink raw reply
* Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: Junio C Hamano @ 2017-02-14 21:31 UTC (permalink / raw)
To: cornelius.weig; +Cc: git, szeder.dev, bitte.keine.werbung.einwerfen, j6t
In-Reply-To: <20170214212404.31469-2-cornelius.weig@tngtech.com>
cornelius.weig@tngtech.com writes:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> Git-checkout completes words starting with '--' as options and other
> words as refs. Even after specifying a ref, further words not starting
> with '--' are completed as refs, which is invalid for git-checkout.
>
> This commit ensures that after specifying a ref, further non-option
> words are completed as paths. Four cases are considered:
>
> - If the word contains a ':', do not treat it as reference and use
> regular revlist completion.
> - If no ref is found on the command line, complete non-options as refs
> as before.
> - If the ref is HEAD or @, complete only with modified files because
> checking out unmodified files is a noop.
> This case also applies if no ref is given, but '--' is present.
Please at least do not do this one; a completion that is or pretends
to be more clever than the end users will confuse them at best and
annoy them. Maybe the user does not recall if she touched the path
or not, and just trying to be extra sure that it matches HEAD or
index by doing "git checkout [HEAD] path<TAB>". Leave the "make it
a noop" to Git, but just allow her do so.
I personally feel that "git checkout <anything>... foo<TAB>" should
just fall back to the normal "path on the filesystem" without any
cleverness, instead of opening a tree object or peek into the index.
^ permalink raw reply
* Re: [PATCH v3 0/5] stash: support pathspec argument
From: Thomas Gummerer @ 2017-02-14 21:36 UTC (permalink / raw)
To: Matthieu Moy
Cc: Jeff King, git, Stephan Beyer, Junio C Hamano, Marc Strapetz,
Johannes Schindelin, Øyvind A . Holm, Jakub Narębski
In-Reply-To: <vpq60kdjl63.fsf@anie.imag.fr>
On 02/14, Matthieu Moy wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > I'm almost convinced of special casing "-p". (Maybe I'm easy to
> > convince as well, because it would be convenient ;) ) However it's a
> > bit weird that now "git stash -p file" would work, but "git stash -m
> > message" wouldn't. Maybe we should do it the other way around, and
> > only special case "-q", and see if there is an non option argument
> > after that? From a glance at the options that's the only one where
> > "git stash -<option> <verb>" could make sense to the user.
>
> Special-casing the allowed cases makes it easier to change the behavior
> in the future if needed: it's easy to add special cases and it doesn't
> break backward compatibility.
>
> So, if we don't have a good reason to do otherwise, I'd rather stay on
> the future-proof side.
Ok, after reading the rest of the messages in the thread I'm convinced
we should just special case "-p" for now.
It's by far my most used stash command as well, after using git stash
without any arguments.
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH v4 4/7] stash: introduce new format create
From: Thomas Gummerer @ 2017-02-14 21:40 UTC (permalink / raw)
To: Matthieu Moy
Cc: git, Junio C Hamano, Jeff King, Johannes Schindelin,
Øyvind A . Holm, Jakub Narębski
In-Reply-To: <vpqefz0ohub.fsf@anie.imag.fr>
On 02/14, Matthieu Moy wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
> > create_stash () {
> > - stash_msg="$1"
> > - untracked="$2"
> > + stash_msg=
> > + untracked=
> > + new_style=
> > + while test $# != 0
> > + do
> > + case "$1" in
> > + -m|--message)
> > + shift
> > + test -z ${1+x} && usage
> > + stash_msg="$1"
> > + new_style=t
> > + ;;
> > + -u|--include-untracked)
> > + shift
> > + test -z ${1+x} && usage
> > + untracked="$1"
> > + new_style=t
> > + ;;
> > + *)
> > + if test -n "$new_style"
> > + then
> > + echo "invalid argument"
> > + option="$1"
> > + # TRANSLATORS: $option is an invalid option, like
> > + # `--blah-blah'. The 7 spaces at the beginning of the
> > + # second line correspond to "error: ". So you should line
> > + # up the second line with however many characters the
> > + # translation of "error: " takes in your language. E.g. in
> > + # English this is:
> > + #
> > + # $ git stash save --blah-blah 2>&1 | head -n 2
> > + # error: unknown option for 'stash save': --blah-blah
> > + # To provide a message, use git stash save -- '--blah-blah'
> > + eval_gettextln "error: unknown option for 'stash create': \$option"
>
> The TRANSLATORS: hint seems a typoed cut-and-paste from somewhere else.
> There are no 7 spaces in this message.
>
> Actually, if I read the code correctly, $option is not even necessarily
> an option as you're matching *. Perhaps you meant something like
>
> -*)
> option="$1"
> # TRANSLATORS: $option is an invalid option, like
> # `--blah-blah'. The 7 spaces at the beginning of the
> # second line correspond to "error: ". So you should line
> # up the second line with however many characters the
> # translation of "error: " takes in your language. E.g. in
> # English this is:
> #
> # $ git stash save --blah-blah 2>&1 | head -n 2
> # error: unknown option for 'stash save': --blah-blah
> # To provide a message, use git stash save -- '--blah-blah'
> eval_gettextln "error: unknown option for 'stash save': \$option
> To provide a message, use git stash save -- '\$option'"
> usage
> ;;
> *)
> if test -n "$new_style"
> then
> arg="$1"
> eval_gettextln "error: invalid argument for 'stash create': \$arg"
> usage
> fi
> break
> ;;
>
> (untested)
>
> Also, you may want to guard against
>
> git stash create "some message" -m "some other message"
>
> since you are already rejecting
>
> git stash create -m "some message" "some other message"
>
> ? Or perhaps apply "last one wins" for both "-m message" and
> "message"-without-dash-m.
Thanks, you're right I was missing some cases here. As I just
indicated in [1] however I think we can just make this an internal
interface, instead of user interface facing, so I think we'll need
less error checking.
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
[1]: http://public-inbox.org/git/20170214213038.GE652@hank/
^ permalink raw reply
* Re: [RFC-PATCHv2] submodules: add a background story
From: Stefan Beller @ 2017-02-14 21:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <xmqqo9yblz33.fsf@gitster.mtv.corp.google.com>
Sorry for dropping the ball here, I was stressed out a bit.
On Thu, Feb 9, 2017 at 3:32 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Do we need
>>
>> * an opinionated way to check for a specific state of a submodule
>> * (submodule helper to be plumbing?)
>> * expose the design mistake of having the (name->path) mapping inside the
>> working tree, i.e. never remove a name from the submodule config even when
>> the submodule doesn't exist any more.
>
> I am not sure about the last item.
>
> Are you talking about a case where submodule comes and goes (think:
> "git checkout v1.0" that would make submodules added since that
> version disappar)? .gitmodules that is checked out would not have
> any entry, but .git/config needs to record the end-user preference
> for the module, so that the user can do "git checkout -" to come
> back, no?
That is perfectly legit and I agree that is good design.
> IOW .git/config that mentions all the submodule the user
> ever showed interests in is not a design mistake, so you must be
> talking about something else, but I do not know what it is.
I mean that we
(1) have a gitmodules file tracked in git that includes the name.
The "tracking some information inside the version control to
help the very version control system" is also not bad. The bad part
is that the name *must not be changed* and
* we do not tell people about it in the docs
* we happily make commits that change the name of a submodule
(2) name the submodule by path be default
See
https://public-inbox.org/git/7e54658a-dcb2-64a7-3c67-0c4fa221b2fb@gmail.com/
> 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.
You could blame this specific instance on the user, but I rather blame it on Git
as such questions come up once in a while on the mailing list.
If we were to redesign the .gitmodules file, we might have it as
[submodule "path"]
url = git://example.org
branch = .
...
and the "path -> name/UID" mapping would be inside $GIT_DIR.
>
> Are they both in section (1)? I think the former (concepts) belongs
> to section 7 and the latter (file formats) belongs to section 5.
oops. Will fix.
>
>> diff --git a/Documentation/gitsubmodules.txt b/Documentation/gitsubmodules.txt
>> new file mode 100644
>> index 0000000000..3369d55ae9
>> --- /dev/null
>> +++ b/Documentation/gitsubmodules.txt
>> @@ -0,0 +1,194 @@
>> +gitsubmodules(7)
>> +================
>> +
>> +NAME
>> +----
>> +gitsubmodules - information about submodules
>> +
>> +SYNOPSIS
>> +--------
>> +$GIT_DIR/config, .gitmodules
>> +
>> +------------------
>> +git submodule
>> +------------------
>> +
>> +DESCRIPTION
>> +-----------
>> +
>> +A submodule allows you to keep another Git repository in a subdirectory
>> +...
>> +When cloning or pulling a repository containing submodules however,
>> +the submodules will not be checked out by default; You need to instruct
>> +'clone' to recurse into submodules. The 'init' and 'update' subcommands
>
> I think this is not "You need to", but rather "You can, if you want
> to have each and every submodules."
ok. In this man page for submodules I assumed an implicit
"[if you want these submodules to be there, then] you have to/need to ...
But I'll tone it down as it doesn't carry internal assumptions.
>> +
>> +** When you want to use a (third party) library tied to a specific version.
>> + Using submodules for a library allows you to have a clean history for
>> + your own project and only updating the library in the submodule when needed.
>
> I somehow do not see this as decoupling; it is keeping what is
> originally separate separate, isn't it?
ok I'll reword that to say keeping separate things separate.
>
>> +** In its current form Git scales up poorly for very large repositories that
>> + change a lot, as the history grows very large. For that you may want to look
>> + at shallow clone, sparse checkout or git-lfs.
>> + However you can also use submodules to e.g. hold large binary assets
>> + and these repositories are then shallowly cloned such that you do not
>> + have a large history locally.
>
> In other words, a better way to list these may be
>
> 1. using another project that stands on its own.
>
> 2. artificially split a (logically single) project into multiple
> repositories and tying them back together.
>
> The access control and performance reasons are subclasses of 2.
> IOW, if Git had per-path ACL and infinite scaling, you wouldn't be
> splitting your project into submodules for 2. You would still want
> to use somebody else's project by binding it as a subproject, instead
> of merging its history into yours.
Looking at the big picture with a logical view is better indeed.
>
>> +When working with submodules, you can think of them as in a state machine.
>> +So each submodule can be in a different state, the following indicators are used:
>> +
>> +* the existence of the setting of 'submodule.<name>.url' in the
>> + superprojects configuration
>> +* the existence of the submodules working tree within the
>> + working tree of the superproject
>> +* the existence of the submodules git directory within the superprojects
>> + git directory at $GIT_DIR/modules/<name> or within the submodules working
>> + tree
>> +
>> + State URL config working tree git dir
>> + -----------------------------------------------------
>> + uninitialized no no no
>> + initialized yes no no
>> + populated yes yes yes
>> + depopulated yes no yes
>> + deinitialized no no yes
>> + uninteresting no yes yes
>> +
>> + invalid no yes no
>> + invalid yes yes no
>
> I do not have strong opinions on these labels; are submodule folks
> happy with the above vocabulary?
Brandon suggested (in)active instead of (un)initialized, which is better as
it decouples the current process from the actual states. Once we reintroduce
[1], then the user would not need to run "init" (whether it is 'git
submodule init'
or implicit as e.g. 'git submodule update --init') any more, but the selection
of active submodules would be done via config.
[1] https://public-inbox.org/git/20161110203428.30512-35-sbeller@google.com/
>
> "uninteresting" is not explained in the below?
will fix.
>
>> ...
>> +SEE ALSO
>> +--------
>> +linkgit:git-submodule[1], linkgit:gitmodules[1].
>
> Ditto.
^ permalink raw reply
* [PATCH 8/7] grep: treat revs the same for --untracked as for --no-index
From: Jeff King @ 2017-02-14 21:54 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, gitster
In-Reply-To: <82212eaa-76d2-3357-8e06-5e4d56028c2e@google.com>
On Tue, Feb 14, 2017 at 10:19:56AM -0800, Jonathan Tan wrote:
> > I wasn't sure if we wanted to treat "untracked" in the same way.
> > Certainly we can catch the error here for the seen_dashdash case, but is
> > it also the case that:
> >
> > echo content >master
> > git grep --untracked pattern master
> >
> > should treat "master" as a path?
>
> It is already the case that it cannot be treated as a rev:
>
> $ git grep --untracked pattern master
> fatal: --no-index or --untracked cannot be used with revs.
>
> So I think it would be better if it was treated as a path, for consistency
> with --no-index. I'm OK either way, though.
Right, it's always been disallowed. But the early detection changes a
few user-visible behaviors like the exact error message, and how
disambiguation works (see below). I think the arguments for making that
change for --no-index are stronger than for --untracked. But it probably
makes sense for --untracked, too.
Here's a patch on top of my series that lumps the two together again. It
_could_ be squashed into the earlier patch, but I think I prefer keeping
it separate.
-- >8 --
Subject: [PATCH] grep: treat revs the same for --untracked as for --no-index
git-grep has always disallowed grepping in a tree (as
opposed to the working directory) with both --untracked
and --no-index. But we traditionally did so by first
collecting the revs, and then complaining when any were
provided.
The --no-index option recently learned to detect revs
much earlier. This has two user-visible effects:
- we don't bother to resolve revision names at all. So
when there's a rev/path ambiguity, we always choose to
treat it as a path.
- likewise, when you do specify a revision without "--",
the error you get is "no such path" and not "--untracked
cannot be used with revs".
The rationale for doing this with --no-index is that it is
meant to be used outside a repository, and so parsing revs
at all does not make sense.
This patch gives --untracked the same treatment. While it
_is_ meant to be used in a repository, it is explicitly
about grepping the non-repository contents. Telling the user
"we found a rev, but you are not allowed to use revs" is
not really helpful compared to "we treated your argument as
a path, and could not find it".
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/grep.c | 10 +++++-----
t/t7810-grep.sh | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 1454bef49..9304c33e7 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -967,6 +967,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
int dummy;
int use_index = 1;
int pattern_type_arg = GREP_PATTERN_TYPE_UNSPECIFIED;
+ int allow_revs;
struct option options[] = {
OPT_BOOL(0, "cached", &cached,
@@ -1165,6 +1166,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
* to it must resolve as a rev. If not, then we stop at the first
* non-rev and assume everything else is a path.
*/
+ allow_revs = use_index && !untracked;
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
unsigned char sha1[20];
@@ -1176,9 +1178,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
break;
}
- if (!use_index) {
+ if (!allow_revs) {
if (seen_dashdash)
- die(_("--no-index cannot be used with revs"));
+ die(_("--no-index or --untracked cannot be used with revs"));
break;
}
@@ -1201,7 +1203,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (!seen_dashdash) {
int j;
for (j = i; j < argc; j++)
- verify_filename(prefix, argv[j], j == i && use_index);
+ verify_filename(prefix, argv[j], j == i && allow_revs);
}
parse_pathspec(&pathspec, 0,
@@ -1273,8 +1275,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (!use_index || untracked) {
int use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;
- if (list.nr)
- die(_("--no-index or --untracked cannot be used with revs."));
hit = grep_directory(&opt, &pathspec, use_exclude, use_index);
} else if (0 <= opt_exclude) {
die(_("--[no-]exclude-standard cannot be used for tracked contents."));
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 0ff9f6cae..cee42097b 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1032,7 +1032,7 @@ test_expect_success 'grep --no-index pattern -- path' '
test_expect_success 'grep --no-index complains of revs' '
test_must_fail git grep --no-index o master -- 2>err &&
- test_i18ngrep "no-index cannot be used with revs" err
+ test_i18ngrep "cannot be used with revs" err
'
test_expect_success 'grep --no-index prefers paths to revs' '
--
2.12.0.rc1.479.g59880b11e
^ permalink raw reply related
* Re: [RFC-PATCHv2] submodules: add a background story
From: Junio C Hamano @ 2017-02-14 21:56 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kb2jZ9fgct6gncDqmWFsbY4MRiboFXPvw7AMcU2KanyfQ@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> If we were to redesign the .gitmodules file, we might have it as
>
> [submodule "path"]
> url = git://example.org
> branch = .
> ...
>
> and the "path -> name/UID" mapping would be inside $GIT_DIR.
I am not sure how you are going to keep track of that mapping,
though. If .gitmodules file does not have a way to tell that what
used to be at "path" in its v1.0 is now at "htap" (instead the above
seems to assume there will just be an entry for [submodule "htap"]
in the newer version, without anything that links the old one with
the new one), how would the mapping inside $GIT_DIR know? Don't
forget that name was introduced as the identity because we cannot
assume that URL for a single project will never change.
I fully agree that our documentation and user education should
stress that names must be unique and immultable throughout the
history of a superproject, though.
^ permalink raw reply
* Re: [PATCH 8/7] grep: treat revs the same for --untracked as for --no-index
From: Junio C Hamano @ 2017-02-14 21:58 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Tan, git
In-Reply-To: <20170214215436.kqca4c7gv2kwevw7@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ...
> The rationale for doing this with --no-index is that it is
> meant to be used outside a repository, and so parsing revs
> at all does not make sense.
>
> This patch gives --untracked the same treatment. While it
> _is_ meant to be used in a repository, it is explicitly
> about grepping the non-repository contents. Telling the user
> "we found a rev, but you are not allowed to use revs" is
> not really helpful compared to "we treated your argument as
> a path, and could not find it".
Yup, both sounds very sensible. Thanks.
^ permalink raw reply
* Re: [PATCH v2 00/19] object_id part 6
From: Junio C Hamano @ 2017-02-14 22:02 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Jeff King, Michael Haggerty
In-Reply-To: <20170214023141.842922-1-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> This is another series in the continuing conversion to struct object_id.
>
> This series converts more of the builtin directory and some of the refs
> code to use struct object_id. Additionally, it implements an
> nth_packed_object_oid function which provides a struct object_id version
> of the nth_packed_object function, and a parse_oid_hex function that
> makes parsing easier.
>
> The patch to use parse_oid_hex in the refs code has been split out into
> its own patch, just because I'm wary of that code and potentially
> breaking things, and I want it to be easy to revert in case things go
> wrong. I have no reason to believe it is anything other than fully
> functional, however.
Thanks. Will queue.
There are a few hunks in builtin/merge.c that ends up getting discarded
when merged to 'pu' as is-old-style-invocation will just be removed,
but the conflict resolution was trivial.
^ permalink raw reply
* Re: [PATCH 0/5] A series of performance enhancements in the memihash and name-cache area
From: Jeff King @ 2017-02-14 22:03 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano, Jeff Hostetler
In-Reply-To: <cover.1487071883.git.johannes.schindelin@gmx.de>
On Tue, Feb 14, 2017 at 12:31:46PM +0100, Johannes Schindelin wrote:
> On Windows, calls to memihash() and maintaining the istate.name_hash and
> istate.dir_hash HashMaps take significant time on very large
> repositories. This series of changes reduces the overall time taken for
> various operations by reducing the number calls to memihash(), moving
> some of them into multi-threaded code, and etc.
>
> Note: one commenter in https://github.com/git-for-windows/git/pull/964
> pointed out that memihash() only handles ASCII correctly. That is true.
> And fixing this is outside the purview of this patch series.
Out of curiosity, do you have numbers? Bonus points if the speedup can
be shown via a t/perf script.
We have a read-cache perf-test already, but I suspect you'd want
something more like "git status" or "ls-files -o" that calls into
read_directory().
-Peff
^ permalink raw reply
* Re: [RFC-PATCHv2] submodules: add a background story
From: Stefan Beller @ 2017-02-14 22:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <xmqq4lzw8mim.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 14, 2017 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> If we were to redesign the .gitmodules file, we might have it as
>>
>> [submodule "path"]
>> url = git://example.org
>> branch = .
>> ...
>>
>> and the "path -> name/UID" mapping would be inside $GIT_DIR.
>
> I am not sure how you are going to keep track of that mapping,
> though. If .gitmodules file does not have a way to tell that what
> used to be at "path" in its v1.0 is now at "htap" (instead the above
> seems to assume there will just be an entry for [submodule "htap"]
> in the newer version, without anything that links the old one with
> the new one), how would the mapping inside $GIT_DIR know?
It depends. Maybe git-mv could have rewritten the internal mapping
as well.
Maybe it would work similar to a rename detection
utilizing a bloomfilter that includes all recorded sha1s at a given path
and then we can take the sha1 from the a given path and check for each
absorbed submodule git dir if that commit belongs to this repo.
I did not quite think it through, but I was pointing out this is brittle.
I guess a quick way would be to follow the .git file inside the submodule
if that exists and if not build up an internal cache that can map
"path -> potential git dirs".
Of course we can argue that the same problem applies to e.g. remotes:
If I have
remote.origin.url = git://kernel.org and
remote.mirror.url = kernel.googlesource.com
then swapping the urls will of course yield different behavior
for 'origin' and 'mirror'. But in this case it is obvious because
"origin" is not the same string as "kernel.org".
So long term, maybe we should come up with a better default name
for submodules, e.g. just a hash of say the URL being used when
adding the submodule.
> Don't
> forget that name was introduced as the identity because we cannot
> assume that URL for a single project will never change.
Yes, URL and path can both change over time, which is why it is
a good idea to have them versioned as well as having a way to
overwrite the URL in the config later on.
> I fully agree that our documentation and user education should
> stress that names must be unique and immultable throughout the
> history of a superproject, though.
This would be a good paragraph in this "background story" that this
patch tries to write. I'll add that.
Thanks,
Stefan
^ permalink raw reply
* Re: [RFC PATCH] show decorations at the end of the line
From: Junio C Hamano @ 2017-02-14 22:11 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <xmqq7f4tdcua.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Thanks. We'd need to update the tests that expects the old style
> output, though.
The updates to the expectation look like this (already squashed).
The --source decorations in 4202 are also shown at the end, which
probably is in line with the way --show-decorations adds them at the
end of the line, but was somewhat surprising from reading only the
log message.
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 48b55bfd27..dea2d449ab 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1353,9 +1353,9 @@ test_expect_success 'set up --source tests' '
test_expect_success 'log --source paints branch names' '
cat >expect <<-\EOF &&
- 09e12a9 source-b three
- 8e393e1 source-a two
- 1ac6c77 source-b one
+ 09e12a9 three source-b
+ 8e393e1 two source-a
+ 1ac6c77 one source-b
EOF
git log --oneline --source source-a source-b >actual &&
test_cmp expect actual
@@ -1364,9 +1364,9 @@ test_expect_success 'log --source paints branch names' '
test_expect_success 'log --source paints tag names' '
git tag -m tagged source-tag &&
cat >expect <<-\EOF &&
- 09e12a9 source-tag three
- 8e393e1 source-a two
- 1ac6c77 source-tag one
+ 09e12a9 three source-tag
+ 8e393e1 two source-a
+ 1ac6c77 one source-tag
EOF
git log --oneline --source source-tag source-a >actual &&
test_cmp expect actual
diff --git a/t/t4207-log-decoration-colors.sh b/t/t4207-log-decoration-colors.sh
index b972296f06..08236a83e7 100755
--- a/t/t4207-log-decoration-colors.sh
+++ b/t/t4207-log-decoration-colors.sh
@@ -44,15 +44,15 @@ test_expect_success setup '
'
cat >expected <<EOF
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_HEAD}HEAD ->\
+${c_commit}COMMIT_ID${c_reset} B${c_commit} (${c_reset}${c_HEAD}HEAD ->\
${c_reset}${c_branch}master${c_reset}${c_commit},\
${c_reset}${c_tag}tag: v1.0${c_reset}${c_commit},\
- ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset} B
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\
- ${c_reset}${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset} A1
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}\
- On master: Changes to A.t
-${c_commit}COMMIT_ID${c_reset}${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset} A
+ ${c_reset}${c_tag}tag: B${c_reset}${c_commit})${c_reset}
+${c_commit}COMMIT_ID${c_reset} A1${c_commit} (${c_reset}${c_tag}tag: A1${c_reset}${c_commit},\
+ ${c_reset}${c_remoteBranch}other/master${c_reset}${c_commit})${c_reset}
+${c_commit}COMMIT_ID${c_reset} On master: Changes to A.t\
+${c_commit} (${c_reset}${c_stash}refs/stash${c_reset}${c_commit})${c_reset}
+${c_commit}COMMIT_ID${c_reset} A${c_commit} (${c_reset}${c_tag}tag: A${c_reset}${c_commit})${c_reset}
EOF
# We want log to show all, but the second parent to refs/stash is irrelevant
^ permalink raw reply related
* Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: Cornelius Weig @ 2017-02-14 22:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, szeder.dev, bitte.keine.werbung.einwerfen, j6t
In-Reply-To: <xmqq8tp88nnj.fsf@gitster.mtv.corp.google.com>
On 02/14/2017 10:31 PM, Junio C Hamano wrote:
> cornelius.weig@tngtech.com writes:
>
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> Git-checkout completes words starting with '--' as options and other
>> words as refs. Even after specifying a ref, further words not starting
>> with '--' are completed as refs, which is invalid for git-checkout.
>>
>> This commit ensures that after specifying a ref, further non-option
>> words are completed as paths. Four cases are considered:
>>
>> - If the word contains a ':', do not treat it as reference and use
>> regular revlist completion.
>> - If no ref is found on the command line, complete non-options as refs
>> as before.
>> - If the ref is HEAD or @, complete only with modified files because
>> checking out unmodified files is a noop.
>> This case also applies if no ref is given, but '--' is present.
>
> Please at least do not do this one; a completion that is or pretends
> to be more clever than the end users will confuse them at best and
> annoy them. Maybe the user does not recall if she touched the path
> or not, and just trying to be extra sure that it matches HEAD or
> index by doing "git checkout [HEAD] path<TAB>". Leave the "make it
> a noop" to Git, but just allow her do so.
Hmm.. I'm a bit reluctant to let go of this just yet, because it was my
original motivation for the whole patch. I admit that it may be
confusing to not get completion in your example. However, I think that
once acquainted with the new behavior, a user who wants some files
cleaned would start by having a look at the list of files from "git
checkout HEAD <TAB><TAB>". That's probably faster than spelling the
first few characters and hit <TAB> for a file that's already clean.
Let's hear if anybody else has an opinion about this.
> I personally feel that "git checkout <anything>... foo<TAB>" should
> just fall back to the normal "path on the filesystem" without any
> cleverness, instead of opening a tree object or peek into the index.
I was thinking about that as well, but it's not what happens for "git
checkout topic:path<TAB>". And given that "git checkout topic path<TAB>"
refers to the same object, consistency kind of demands that the tree
objects are opened in the latter case as well. However, because the
differences to filesystem path completion are somewhat corner cases, I'm
fine with that as long as I'm not offered ref names any longer...
^ permalink raw reply
* Re: [RFC-PATCHv2] submodules: add a background story
From: Junio C Hamano @ 2017-02-14 22:17 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kbjSLaUsJH_KuT6EiC+Kt-87+GjONt08hCytXULecMijA@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Tue, Feb 14, 2017 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>> If we were to redesign the .gitmodules file, we might have it as
>>>
>>> [submodule "path"]
>>> url = git://example.org
>>> branch = .
>>> ...
>>>
>>> and the "path -> name/UID" mapping would be inside $GIT_DIR.
>>
>> I am not sure how you are going to keep track of that mapping,
>> though. If .gitmodules file does not have a way to tell that what
>> used to be at "path" in its v1.0 is now at "htap" (instead the above
>> seems to assume there will just be an entry for [submodule "htap"]
>> in the newer version, without anything that links the old one with
>> the new one), how would the mapping inside $GIT_DIR know?
>
> It depends. Maybe git-mv could have rewritten the internal mapping
> as well.
And then after doing the "git mv" you have pushed the result, which
I pulled. Now, how will your "internal mapping" propagate to me?
I also do not think "this is similar to file renames" holds water.
Moving the path a submodule bound to from one path to another is
done as a whole, and it is not like the blob contents where we need
to handle patch application that expresses a move as creation and
deletion of similar contents at two different paths. We can afford
to be precise (after all, we are recording other information about
submodules by having an extra .gitmodules file).
In short, "name" is not a design mistake at all. That needs to be
excised from the "background story".
^ permalink raw reply
* Re: [RFC-PATCHv2] submodules: add a background story
From: Stefan Beller @ 2017-02-14 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <xmqqmvdo76yw.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 14, 2017 at 2:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Tue, Feb 14, 2017 at 1:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Stefan Beller <sbeller@google.com> writes:
>>>
>>>> If we were to redesign the .gitmodules file, we might have it as
>>>>
>>>> [submodule "path"]
>>>> url = git://example.org
>>>> branch = .
>>>> ...
>>>>
>>>> and the "path -> name/UID" mapping would be inside $GIT_DIR.
>>>
>>> I am not sure how you are going to keep track of that mapping,
>>> though. If .gitmodules file does not have a way to tell that what
>>> used to be at "path" in its v1.0 is now at "htap" (instead the above
>>> seems to assume there will just be an entry for [submodule "htap"]
>>> in the newer version, without anything that links the old one with
>>> the new one), how would the mapping inside $GIT_DIR know?
>>
>> It depends. Maybe git-mv could have rewritten the internal mapping
>> as well.
>
> And then after doing the "git mv" you have pushed the result, which
> I pulled. Now, how will your "internal mapping" propagate to me?
The "name" inside your superprojects git dir may be different from mine,
after all the name only serves the purpose to not have duplicate
git repositories when renaming a submodule.
>
> I also do not think "this is similar to file renames" holds water.
> Moving the path a submodule bound to from one path to another is
> done as a whole, and it is not like the blob contents where we need
> to handle patch application that expresses a move as creation and
> deletion of similar contents at two different paths. We can afford
> to be precise (after all, we are recording other information about
> submodules by having an extra .gitmodules file).
>
> In short, "name" is not a design mistake at all. That needs to be
> excised from the "background story".
I am not saying it was a design mistake per se.
I claim that the exposure into .gitmodules combined with
the extreme similarity to its path is confusing. Maybe this
can be fixed by a different default name.
^ permalink raw reply
* Re: [RFC-PATCHv2] submodules: add a background story
From: Junio C Hamano @ 2017-02-14 22:39 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Brandon Williams
In-Reply-To: <CAGZ79kaeVrW3_kUWWxBMztOPuWY_V6XP2SUDyw8mmQ6peFZwdw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
>> And then after doing the "git mv" you have pushed the result, which
>> I pulled. Now, how will your "internal mapping" propagate to me?
>
> The "name" inside your superprojects git dir may be different from mine,
> after all the name only serves the purpose to not have duplicate
> git repositories when renaming a submodule.
That is true, but you still need to convey "what I used to have at
'path' is now at 'htap'". It is clear how to do so if we use "name"
in .gitmodules (you say "what we collectively call module A is now
at 'htap'"). I do not know how you do so without having a name.
^ permalink raw reply
* Re: [PATCH v2 2/2] completion: checkout: complete paths when ref given
From: Junio C Hamano @ 2017-02-14 22:45 UTC (permalink / raw)
To: Cornelius Weig; +Cc: git, szeder.dev, bitte.keine.werbung.einwerfen, j6t
In-Reply-To: <9be8b988-f5b3-7365-ae7f-b46888253f4c@tngtech.com>
Cornelius Weig <cornelius.weig@tngtech.com> writes:
> Hmm.. I'm a bit reluctant to let go of this just yet, because it was my
> original motivation for the whole patch. I admit that it may be
> confusing to not get completion in your example. However, I think that
> once acquainted with the new behavior, a user who wants some files
> cleaned would start by having a look at the list of files from "git
> checkout HEAD <TAB><TAB>". That's probably faster than spelling the
> first few characters and hit <TAB> for a file that's already clean.
I understand that "git checkout HEAD frotz<TAB>" that gives 47 other
paths that all begin with "foo", when "frotz27" is the only one
among them that you know you changed, is not very useful to narrow
things down.
But it is equally irritating when you know "frotz27" is the only
path that begins with "frotz" (after all, that is why you decided to
stop typing after saying "frotz" and letting the comletion kick in)
but you are not certain if you touched it. The completion being
silent may be an indication that it is not modified, OR it may be an
indication that you mistyped the leading part "frotz", and leaves
you wondering.
^ permalink raw reply
* [PATCH/RFC] git-gui: Fix author name encoding in Amend Last Commit.
From: bernhardu @ 2017-02-14 22:46 UTC (permalink / raw)
To: git; +Cc: Bernhard Übelacker
From: Bernhard Übelacker <bernhardu@mailbox.org>
In "New Commit" author name is set correctly.
But when doing "Amend Last Commit" the encoding gets wrong.
This patch adds in load_last_commit a similar assignment converting
to tcl encoding for commit_author as already exists for msg.
---
lib/commit.tcl | 3 +++
1 file changed, 3 insertions(+)
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 83620b7..f5357f2 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -46,6 +46,9 @@ You are currently in the middle of a merge that has not been fully completed. Y
set msg [encoding convertfrom $enc $msg]
}
set msg [string trim $msg]
+ if {$enc ne {}} {
+ set commit_author [encoding convertfrom $enc $commit_author]
+ }
} err]} {
error_popup [strcat [mc "Error loading commit data for amend:"] "\n\n$err"]
return
--
2.11.0
^ permalink raw reply related
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