Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration
Jeff King writes: >> It was a bit more painful than necessary to make sure I have >> something that can be merged for 2.14.x maintenance track, but I >> think the topic is now in a reasonable shape, and I've merged it to >> 'next'. On the first-parent chain from 'master' to 'pu', the merge >> of this topic is the very first one, and after reading it over once >> again, I think this is OK. > > Hmm. I think you would just want the top two commits for maint-2.14 > (reverting 136c8c8b8f and fixing up git-tag to check color config). But > of course you can't do a partial merge because they come on top of the > other dead-end/revert pair. You'd have to cherry-pick (and even then fix > up a few bits, like adding in the "add -p" test). > > Though if we take all of jk/ui-color-always-to-auto-maint, and then do > the whole reversion on top of that, I think that should work. It just > doesn't look like that topic ever made it to "maint" (I see mention of a > jk/ref-filter-colors-fix-maint in the log for master, but there's no > such branch). Yeah, that is what ended up to be jk/ref-filter-colors-fix; the branch merges cleanly to 'master', but also to 'maint' without dragging the rest of the recent development along with it---I did a rebase before sending out the message you are responding to.
Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration
On Mon, Oct 16, 2017 at 11:51:01PM -0700, Jonathan Nieder wrote: > > OK, so it seems we both have slight preference for the "peel back" > > approach. Adding Jonathan to Cc: > > Which approach is "harder but right" / "peel back"? "peel back" is reverting back to the pre-v2.14.2 state (Junio has the patches queued in jk/ref-filter-colors-fix). > I agree with the goal of making color.ui=always a synonym for auto in > file-based config. Peff found some problems with the warning patch > (scripted commands produce too many warnings), which are not an issue > for $dayjob but may be for upstream, so I see the value of holding off > on the warning for now. > > I'm also fine with "revert the proximate cause of the latest > complaints" as a stepping stone toward making color.ui=always a > synonym for auto in file-based config in a later release. I do think "color.ui=always" is a foot-gun, but I wasn't happy with the number of weird hacks we ended up with in trying to fix that (like "it means one thing in the on-disk config and another thing with "git -c"). We can take it up as a topic post-release. At the very least, I think we will want to change the documentation to make it more clear that you almost certainly _don't_ want to use "always". -Peff
Re: [PATCH 1/2] color: downgrade "always" to "auto" only for on-disk configuration
On Tue, Oct 17, 2017 at 03:26:25PM +0900, Junio C Hamano wrote: > Junio C Hamano writes: > > > Jeff King writes: > > > >> After pondering over it, I have a slight preference for that, too. But > >> I'm also happy to hear more input. > > > > OK, so it seems we both have slight preference for the "peel back" > > approach. Adding Jonathan to Cc: > > It was a bit more painful than necessary to make sure I have > something that can be merged for 2.14.x maintenance track, but I > think the topic is now in a reasonable shape, and I've merged it to > 'next'. On the first-parent chain from 'master' to 'pu', the merge > of this topic is the very first one, and after reading it over once > again, I think this is OK. Hmm. I think you would just want the top two commits for maint-2.14 (reverting 136c8c8b8f and fixing up git-tag to check color config). But of course you can't do a partial merge because they come on top of the other dead-end/revert pair. You'd have to cherry-pick (and even then fix up a few bits, like adding in the "add -p" test). Though if we take all of jk/ui-color-always-to-auto-maint, and then do the whole reversion on top of that, I think that should work. It just doesn't look like that topic ever made it to "maint" (I see mention of a jk/ref-filter-colors-fix-maint in the log for master, but there's no such branch). I started to prepare a patch directly on v2.14.2 just to see what it would look like. The first one (the revert) is fine, but we then have to fixup tag and for-each-ref. And since they don't have --color added by the dead-end fixups, the tests get harder... -Peff
Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands
On Wed, Oct 18, 2017 at 05:55:18AM +0900, Junio C Hamano wrote: > I'll take these three to replace what I tentatively queued, not > necessarily because I like the change in 1/3 better, but because > these are explained much better; besides I prefer a version that at > least two people deeply looked at. These look good to me. Between your two versions, I think the code in Jonathan's is slightly preferable. It's possible that some other caller of strbuf_check_branch_name() may run into the same thing and be fixed (I am trying to think of a hypothetical caller that would be inconvenienced by the new behavior, but I can't come up with one; and certainly the existing code would BUG(), so this is an improvement). -Peff
Re: Minor man page weirdness?
On Wed, Oct 18, 2017 at 12:41:03PM +0900, Junio C Hamano wrote: > Jeff King writes: > > > It does make me wonder if there are other instances of the missing-space > > problem lurking. Grepping for backtick followed by single-quote shows > > only one more case in user-manual.txt. I have no idea if that one hits > > the same problem on older docbook versions. > > I had an impression that this is only for roff, but we do not > produce user-manual.[0-9], do we? Yeah, I think you're right. We do produce XML of it, but it goes to other formats like texi and pdf (and if this is a docbook bug, it may or may not be present in those other output formats). -Peff
Re: Minor man page weirdness?
Jeff King writes: > It does make me wonder if there are other instances of the missing-space > problem lurking. Grepping for backtick followed by single-quote shows > only one more case in user-manual.txt. I have no idea if that one hits > the same problem on older docbook versions. I had an impression that this is only for roff, but we do not produce user-manual.[0-9], do we? > Or if it's a problem for any > other punctuation combinations. I'm happy to punt on it until somebody > with an affected docbook produces more bug reports. :) Yup, we cannot _fix_ what we do not know are broken ;-) Thanks.
Re: Minor man page weirdness?
On Wed, Oct 18, 2017 at 11:34:31AM +0900, Junio C Hamano wrote: > -- >8 -- > branch doc: sprinkle a few commas for readability > > The "--force" option can also be used when the named branch does not > yet exist, and the point of the option is the user can (re)point the > branch to the named commit even if it does. Add 'even' before 'if' > to clarify. Also, insert another comma after "Without -f" before > "the command refuses..." to make the text easier to parse. > > Incidentally, this change should help certain versions of > docbook-xsl-stylesheets that renders the original without any > whitespace between "-f" and "git". Thanks, this looks good. It does make me wonder if there are other instances of the missing-space problem lurking. Grepping for backtick followed by single-quote shows only one more case in user-manual.txt. I have no idea if that one hits the same problem on older docbook versions. Or if it's a problem for any other punctuation combinations. I'm happy to punt on it until somebody with an affected docbook produces more bug reports. :) -Peff
Re: Minor man page weirdness?
Junio C Hamano writes: > Andreas Schwab writes: > >> On Okt 16 2017, Jeff King wrote: >> >>> We do have some hacks/workarounds for broken versions of the toolchain. >>> You can try tweaking various knobs you find in Documentation/Makefile). >>> DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the >>> opposite (removes extra spaces). >> >> An easy workaround would be to add a comma between `-f` and 'git >> branch'. > > That sounds like a good idea, regardless of the mark-up issue. -- >8 -- branch doc: sprinkle a few commas for readability The "--force" option can also be used when the named branch does not yet exist, and the point of the option is the user can (re)point the branch to the named commit even if it does. Add 'even' before 'if' to clarify. Also, insert another comma after "Without -f" before "the command refuses..." to make the text easier to parse. Incidentally, this change should help certain versions of docbook-xsl-stylesheets that renders the original without any whitespace between "-f" and "git". Noticed-by: Lars Schneider Helped-by: Jeff King Helped-by: Andreas Schwab Signed-off-by: Junio C Hamano --- Documentation/git-branch.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index fe029ac6fc..d6587c5e96 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -104,8 +104,8 @@ OPTIONS -f:: --force:: - Reset to if exists - already. Without `-f` 'git branch' refuses to change an existing branch. + Reset to , even if exists + already. Without `-f`, 'git branch' refuses to change an existing branch. In combination with `-d` (or `--delete`), allow deleting the branch irrespective of its merged status. In combination with `-m` (or `--move`), allow renaming the branch even if the new
Re: Fwd: Has git-gui repo moved from location?
Weird, it was not working for me earlier today, but now it works. Thank you, Gilberto On Tue, Oct 17, 2017 at 5:57 PM, Junio C Hamano wrote: > Gilberto Stankiewicz writes: > >> I am trying to clone git://repo.or.cz/git-gui.git as described at >> https://github.com/git/git/blob/master/Documentation/SubmittingPatches >> but it seems the repo does not exist. > > $ git fetch -v git-gui > Looking up repo.or.cz ... done. > Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done. > From git://repo.or.cz/git-gui > = [up to date]master -> git-gui/master > = [up to date]pu -> git-gui/pu > = [up to date]todo -> git-gui/todo > > $ git fetch -v git://repo.or.cz/git-gui.git > Looking up repo.or.cz ... done. > Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done. > From git://repo.or.cz/git-gui > * branch HEAD -> FETCH_HEAD >
Re: [PATCH] Added support for new configuration parameter push.pushOption
Junio C Hamano writes:
> base to show an incremental improvement, but here is how I would do
> the "code" part.
>
> builtin/push.c | 22 ++
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/push.c b/builtin/push.c
> index 03846e8379..89ef029c67 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -32,6 +32,8 @@ static const char **refspec;
> static int refspec_nr;
> static int refspec_alloc;
>
> +static struct string_list push_options_config = STRING_LIST_INIT_DUP;
> +
> static void add_refspec(const char *ref)
> {
> refspec_nr++;
> @@ -503,6 +505,13 @@ static int git_push_config(const char *k, const char *v,
> void *cb)
> int val = git_config_bool(k, v) ?
> RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
> recurse_submodules = val;
> + } else if (!strcmp(k, "push.pushoption")) {
> + if (!v)
> + return config_error_nonbool(k);
> + else if (!*v)
> + string_list_clear(&push_options_config, 0);
> + else
> + string_list_append(&push_options_config, v);
Here needs to be
return 0;
as there is no point in falling through to call
git_default_config().
> }
>
> return git_default_config(k, v, NULL);
Re: [PATCH] Added support for new configuration parameter push.pushOption
Marius Paliga writes:
> builtin/push.c: add push.pushOption config
This is a good title for this change; it would be perfect if it were
on the Subject: header ;-)
> Currently push options need to be given explicitly, via
> the command line as "git push --push-option".
>
> The UX of Git would be enhanced if push options could be
> configured instead of given each time on the command line.
>
> Add the config option push.pushOption, which is a multi
> string option, containing push options that are sent by default.
s/Currently p/P/ would be sufficient; we describe the status quo in
present tense, and give an order to the codebase to "become like so"
(or command a patch monkey to "make the codebase like so") by using
imperative mood.
I think something shorter like this would be sufficient:
Push options need to be given explicitly, via the command line as "git
push --push-option ". Add the config option push.pushOption,
which is a multi-valued option, containing push options that are sent
by default.
When push options are set in the lower-priority configulation file
(e.g. /etc/gitconfig, or $HOME/.gitconfig), they can be unset later in
the more specific repository config by the empty string.
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
> index 3e76e99f3..da9b17624 100644
> --- a/Documentation/git-push.txt
> +++ b/Documentation/git-push.txt
> @@ -161,6 +161,9 @@ already exists on the remote side.
> Transmit the given string to the server, which passes them to
> the pre-receive as well as the post-receive hook. The given string
> must not contain a NUL or LF character.
> +Default push options can also be specified with configuration
> +variable `push.pushOption`. String(s) specified here will always
> +be passed to the server without need to specify it using `--push-option`
"will always be passed" is not a "Default".
If we really need to say it, say something like
When no `--push-option ` is given from the command
line, the values of this configuration variable are used
instead.
But that is what "Default" means, so dropping the second sentence
altogether would be more concise and better.
> diff --git a/builtin/push.c b/builtin/push.c
> index 2ac810422..10e520c8f 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -32,6 +32,8 @@ static const char **refspec;
> static int refspec_nr;
> static int refspec_alloc;
>
> +static struct string_list push_options_from_config = STRING_LIST_INIT_DUP;
> +
> static void add_refspec(const char *ref)
> {
> refspec_nr++;
> @@ -503,6 +505,14 @@ static int git_push_config(const char *k, const
> char *v, void *cb)
> int val = git_config_bool(k, v) ?
> RECURSE_SUBMODULES_ON_DEMAND : RECURSE_SUBMODULES_OFF;
> recurse_submodules = val;
> +} else if (!strcmp(k, "push.pushoption")) {
> +if (v == NULL)
> +return config_error_nonbool(k);
> +else
> +if (v[0] == '\0')
Make this "else if (!*v)" on a single line and dedent the remainder.
> +string_list_clear(&push_options_from_config, 0);
> +else
> +string_list_append(&push_options_from_config, v);
> }
> @@ -562,6 +572,8 @@ int cmd_push(int argc, const char **argv, const
> char *prefix)
> packet_trace_identity("push");
> git_config(git_push_config, &flags);
> argc = parse_options(argc, argv, prefix, options, push_usage, 0);
> +if (!push_options.nr)
> +push_options = push_options_from_config;
We encourage our developers to think twice before doing a structure
assignment.
I think this is a bad idea, primarily because at the point where
string_list_clear() is later called on &push_options, it is unclear
how to release the resources held by push_options_from_config(). It
also is bad to depend on the implementation detail that overwriting
the structure do not leak push_options.item when push_options.nr is
zero, but it is conceivable that STRING_LIST_INIT_* could later be
modified to pre-allocate a small array, at which point this will
become a memory leak.
> diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh
> index 90a4b0d2f..463783789 100755
> --- a/t/t5545-push-options.sh
> +++ b/t/t5545-push-options.sh
> @@ -140,6 +140,83 @@ test_expect_success 'push options and submodules' '
> ...
> +git -c push.pushOption=default1 -c push.pushOption=default2
> push up master
> ...
> +test_expect_success 'push option from command line overrides
> from-config push option' '
It appears that your MUA is expanding tabs to spaces and wrapping
long lines in your patch? Please double check and make sure that
will not happen.
I couldn't use your patch (because it was broken by your MUA) as a
base to show an incremental improvement, but here is how I would do
the "code" part.
builtin/push.c | 22 ++
1 file changed, 18 insertions(+), 4 de
Re: Fwd: Has git-gui repo moved from location?
Gilberto Stankiewicz writes: > I am trying to clone git://repo.or.cz/git-gui.git as described at > https://github.com/git/git/blob/master/Documentation/SubmittingPatches > but it seems the repo does not exist. $ git fetch -v git-gui Looking up repo.or.cz ... done. Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done. >From git://repo.or.cz/git-gui = [up to date]master -> git-gui/master = [up to date]pu -> git-gui/pu = [up to date]todo -> git-gui/todo $ git fetch -v git://repo.or.cz/git-gui.git Looking up repo.or.cz ... done. Connecting to repo.or.cz (port 9418) ... 195.113.20.142 done. >From git://repo.or.cz/git-gui * branch HEAD -> FETCH_HEAD
Re: [PATCH v2] doc: list filter-branch subdirectory-filter first
David Glasser writes: > From: David Glasser > > The docs claim that filters are applied in the listed order, so > subdirectory-filter should come first. > > For consistency, apply the same order to the SYNOPSIS and the script's usage, > as > well as the switch while parsing arguments. > > Add missing --prune-empty to the script's usage. > > Signed-off-by: David Glasser > --- Thanks for being extra careful. Will queue.
Re: [PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch
Stefan Beller writes: > On Mon, Oct 16, 2017 at 6:59 AM, Heiko Voigt wrote: >> To make extending this logic later easier. >> >> Signed-off-by: Heiko Voigt > > Thanks for this readability fix! > > Stefan Thanks, both.
Re: [PATCH v4 2/3] implement fetching of moved submodules
Stefan Beller writes:
>> + /* make sure name does not collide with existing one
>> */
>> + submodule = submodule_from_name(commit_oid, name);
>> + if (submodule) {
>> + warning("Submodule in commit %s at path: "
>> + "'%s' collides with a submodule
>> named "
>> + "the same. Skipping it.",
>> + oid_to_hex(commit_oid), name);
>> + name = NULL;
>> + }
>
> This is the ugly part of using one string list and storing names or
> path in it. I wonder if we could omit this warning if we had 2 string lists?
We are keying off of 'name', because that is what will give a module
its identity. If we have a gitlink whose path is not in .gitmodules
in the same tree, then we are seeing an unregistered submodule. If
we were to "git add" it, then we'd use its path as the default name,
but if we already have a submodule with that name (the most likely
explanation for its existence is because it started its life there
and then later moved), and the submodule is bound to a different
path, then that is a different submodule. Skipping and warning both
are sensible thing to do.
I do not know what you see as ugly here, and more importantly, I am
not sure how having two lists would help.
Fwd: Has git-gui repo moved from location?
Hello, I am trying to clone git://repo.or.cz/git-gui.git as described at https://github.com/git/git/blob/master/Documentation/SubmittingPatches but it seems the repo does not exist. Has the repo changed from location? Thank you, Gilberto
Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads
René Scharfe writes: > Stop advertising -h as the short equivalent of --heads, because it's > used for showing a short help text for almost all other git commands. > Since the ba5f28bf79 (ls-remote: use parse-options api) it has only > been working when used together with other parameters anyway. > > Signed-off-by: Rene Scharfe > --- > That would be step on the way towards more consistent command line > switches, in the same vein as d69155119 (t0012: test "-h" with > builtins). Sorry, but I am not sure whom this and the other approach are trying to help. The rule we have currently seems to be that "git cmd -h" (no other arguments) consistently gives a short help, and if a subcommand supports an option "-h" specific to it that is not about giving a short help, the caller needs to be aware of it to invoke the option, by making sure that there is some other arguments on the command line if "-h" form of that subcommand specific option is used, by doing e.g. "git ls-remote -h origin" or "git ls-remote --head". I can see that this "alternative" approach makes it less convenent for folks who have followed that rule by hiding "-h" (with the intention of deprecating and possibly removing it in the future) and encouraging (and later foring) "--head" to be used instead. The other approach burdens new users by changing the rule to "some subcommands that have their own '-h' option cannot be asked for a brief usage with 'git cmd -h'". But the thing is, these new users who do not know which subcommands do have their own '-h' and which ones do not are the ones that benefit most from the consistent "'git cmd -h' with no other argument gets short help" rule. When making a backward incompatible change, it is asking us to go to those who are inconvenienced and say "I am sorry that we are making things less convenient for you, but by doing so we can gain this greater good which is ...". I can explain how this and the other approach make things less convenient for existing or new users, but I am having a hard time formulating what the greater good is. In short, I cannot sell this change to our users. Please help me do so if you want this (or the other) change made to our system.
Re: [PATCH 2/1] mention git stash push first in the man page
On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote: > > Seems reasonable, though if we are deprecating "save" should we demote > > it from being in the synopsis entirely? > > I saw that as a next step, with the "official" deprecation of "save". > I thought we might want to advertise "push" a bit more before actually > officially deprecating "save" and sending the deprecation notice out > in the release notes. Right, my thinking was that it would still be documented in the manpage, just lower down. And that would probably say something like "save is a historical synonym for push, except that it differs in these ways...". > OTOH, dropping it from the synopsis in the man page probably wouldn't > cause much issue, as "push" directly replaces it, and is easily > visible. Not sure how slow we want to take the deprecation? I don't think there's any reason to go slow in marking something as deprecated. It's the part where we follow up and remove or change the feature that must take a while. -Peff
Re: [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)
On 10/17, Jeff King wrote:
> On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote:
>
> > Since you were asking :) With the introduction of 'git stash push',
> > my hope was always that we could eventually get rid of 'git stash
> > save' and only keep one interface around.
> >
> > As there still many references to it around on the internet, it
> > probably requires a bit of a longer deprecation plan. What do you
> > think about the following:
> >
> > - Change docs/man pages to use 'git stash push' everywhere instead of
> > 'git stash save'.
> > - Mention the deprecation in the release notes and in the man page.
> > - Print a warning when 'git stash save' is used.
> > - Eventually get rid of it (maybe something for 3.0?)
> >
> > The steps above would all occur sequentially with a few releases
> > between each of them.
>
> That sounds like a pretty good plan.
>
> > A patch for the first step below. I think that even makes sense if we
> > don't want to follow through with the deprecation.
>
> Agreed. The patch mostly looks good, except:
Thanks for the review.
> > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> > index b4d88af133..1c4e44892d 100644
> > --- a/Documentation/user-manual.txt
> > +++ b/Documentation/user-manual.txt
> > @@ -1556,7 +1556,7 @@ so on a different branch and then coming back),
> > unstash the
> > work-in-progress changes.
> >
> >
> > -$ git stash save "work in progress for foo feature"
> > +$ git stash push "work in progress for foo feature"
> >
>
> This needs "-m", doesn't it?
Yeah, this definitely needs "-m". Will change.
> >
> > This command will save your changes away to the `stash`, and
> > diff --git a/git-stash.sh b/git-stash.sh
> > index 8b2ce9afda..8ce6929d7f 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -267,11 +267,11 @@ push_stash () {
> > # 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'"
> > + #$ git stash push --blah-blah 2>&1 | head -n 2
> > + #error: unknown option for 'stash push': --blah-blah
> > + # To provide a message, use git stash push --
> > '--blah-blah'
> > + eval_gettextln "error: unknown option for 'stash push':
> > \$option
> > + To provide a message, use git stash push -- '\$option'"
>
> And here, too?
Indeed, thanks for catching these. Will change and re-roll the
patches.
> -Peff
Re: [PATCH 2/1] mention git stash push first in the man page
On 10/17, Jeff King wrote: > On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote: > > > Because 'stash push' and 'stash save' are so closely related they share one > > section in the man page. Currently 'stash save' comes first, as that > > was the command that people were historically using. However this makes > > the newer, more feature rich git stash push very easy to overlook. > > Change the order to give the newer interface for creating a stash the > > more prominent position. > > Seems reasonable, though if we are deprecating "save" should we demote > it from being in the synopsis entirely? I saw that as a next step, with the "official" deprecation of "save". I thought we might want to advertise "push" a bit more before actually officially deprecating "save" and sending the deprecation notice out in the release notes. OTOH, dropping it from the synopsis in the man page probably wouldn't cause much issue, as "push" directly replaces it, and is easily visible. Not sure how slow we want to take the deprecation? > -Peff
Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick
On 17/10/2017 19:30, Johannes Sixt wrote: > Am 17.10.2017 um 01:01 schrieb Rafael Ascensao: >>> This is worth discussing, though not my preference. The picture to "pick >>> cherries" has become quite common, and now that we use it for the name of >>> the command, "cherry-pick", the direction of flow is quite obvious and >>> strongly implied: from somewhere else to me (and not to somebody else). >> >> What if we borrow '--onto' from rebase and make it cherry-pick --onto >> ? > > I actually like this. Although I would miss the convenience that the > source defaults to HEAD. Unless we make a special case for --onto, > that is, of course. I second this. >From my point of view, I would say that "cherry-pick" in general seems to just imply "taking" commits (picking "cherries"), where direction flow is irrelevant, or at least not strongly implied. For me, it`s more "from somewhere else to somebody else", indeed. The fact that it currently "posts" its picks "to me" (on top of current HEAD) could very well be justified/explained as the most obvious case (at time of implementation, at least), where usually one does work "here" (HEAD), thus cares to have commits picked over "here" as well - but it doesn`t have to be a restriction, where other destination besides HEAD could/should be allowed. Anyway, I`d say current behaviour without "--onto" should stay, indeed, where HEAD destination is implied if not otherwise specified (and that seems to align well with some other Git commands, too). Also, in case "--onto" _is_ provided, maybe commit to be picked could be allowed to be omitted, defaulting to HEAD again, but as source this time. That said, might be 'git commit' looks like a nice candidate for being taught this new trick as well (optionally commit to a revision other than HEAD, what Ævar already mentioned), following the same logic... ;) It would even be a better (more straightforward) fit for one of your previous examples: > Another use case is when you receive a patch to be applied on a > different branch while you are in the middle of some work. If it can be > applied on the current branch, then you can post it to the destination, > rewind, and continue with your work. p.s. I`m very interested in this functionality, and more - I have a beginner`s attempt in addressing a similar use-case this topic is concerned with (symptomatically using a variation of "--onto" as well :P), hopefully I`ll sent out something soon, for the sake of discussion/opinion, at least :( Regards, Buga
Re: Re* Is t5601 flaky for anybody else?
On Wed, Oct 18, 2017 at 06:02:59AM +0900, Junio C Hamano wrote: > Brandon Williams writes: > > > I haven't noticed any issues myself but maybe this has something to do > > with my changes to this test in the 'bw/protocol-v1' topic? > > As I've seen this on 'master', too, I suspect the topic has nothing > to do with it. > > Here is what I have on 'pu'. FWIW, I can't replicate the problem on either "master" or "pu". I wonder why. -Peff
Re: Multiple paths in GIT_EXEC_PATH
Kevin Daudt writes: > The commit that changed what you described is 1073094f3 (git-sh-setup: > be explicit where to dot-source git-sh-i18n from., 2016-10-29). That > commit claims there were already scripts that assumed GIT_EXEC_PATH is > just a single entry. That commit was included in v2.11. > > There was also a recent thread[0] about it that discussed this issue, > where someone stated that indeed treating GIT_EXEC_PATH with the same > semantics as PATH has been broken for a while, but it seems there are no > real plans to fix it. The variable was never meant to have more than one path concatenated with ':' from day one. In C code we've used it as a leading directory path to tack a command name to form a path to give to exec(3), without any intention to have it a list of paths, which is split at ':'. sh-setup was doing "PATH=$GIT_EXEC_PATH:$PATH" without rejecting a value in GIT_EXEC_PATH with a colon in it but that was merely being lazy and made it "work" by accident.
Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands
Junio C Hamano wrote:
> Jonathan Nieder writes:
>> From: Junio C Hamano
>>
>> "git check-ref-format --branch $name" feature was originally
>> introduced (and was advertised) as a way for scripts to take any
>> end-user supplied string (like "master", "@{-1}" etc.) and see if it
>> is usable when Git expects to see a branch name, and also obtain the
>> concrete branch name that the at-mark magic expands to.
>>
>> Emphasize that "see if it is usable" role in the description and
>> clarify that the @{...} expansion only occurs when run from within a
>> repository.
>>
>> [jn: split out from a larger patch]
>>
>> Helped-by: Jeff King
>> Signed-off-by: Junio C Hamano
>> ---
>
> Missing sign-off, unlike the other two, intended?
Oops.
Signed-off-by: Jonathan Nieder
> I'll take these three to replace what I tentatively queued, not
> necessarily because I like the change in 1/3 better, but because
> these are explained much better; besides I prefer a version that at
> least two people deeply looked at.
Okay, thanks. I'll add the rest of the change as a patch on top. :)
Thanks,
Jonathan
Re* Is t5601 flaky for anybody else?
Brandon Williams writes:
> I haven't noticed any issues myself but maybe this has something to do
> with my changes to this test in the 'bw/protocol-v1' topic?
As I've seen this on 'master', too, I suspect the topic has nothing
to do with it.
Here is what I have on 'pu'.
-- >8 --
From: Junio C Hamano
Date: Tue, 17 Oct 2017 14:04:43 +0900
Subject: [PATCH] t5601: rm the target file of cp that could still be executing
"while sh t5601-clone.sh; do :; done" seems to fail sporadically at
around test #45 where fake-ssh wrapper is copied create plink.exe,
with an error message that says the "text is busy".
I have a mild suspicion that the root cause of the bug is that the
fake SSH process from the previous test is still running by the time
the next test wants to replace it with a new binary, but in the
meantime, removing the target that could still be executing before
copying something else over seems to work it around.
Signed-off-by: Junio C Hamano
---
t/t5601-clone.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 9c56f771b6..50e40abb11 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -308,6 +308,7 @@ test_expect_success 'clone checking out a tag' '
setup_ssh_wrapper () {
test_expect_success 'setup ssh wrapper' '
+ rm -f "$TRASH_DIRECTORY/ssh-wrapper$X" &&
cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
"$TRASH_DIRECTORY/ssh-wrapper$X" &&
GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
@@ -318,6 +319,7 @@ setup_ssh_wrapper () {
}
copy_ssh_wrapper_as () {
+ rm -f "${1%$X}$X" &&
cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
GIT_SSH="${1%$X}$X" &&
export GIT_SSH
--
2.15.0-rc1-178-ge1264d9eb8
Re: [PATCH] fetch doc: src side of refspec could be full SHA-1
Jonathan Nieder writes: > nits: > > s/most typically/typically/ > s/an fully/a fully/ > > With those tweaks, > Reviewed-by: Jonathan Nieder Thanks.
Re: Minor man page weirdness?
Andreas Schwab writes: > On Okt 16 2017, Jeff King wrote: > >> We do have some hacks/workarounds for broken versions of the toolchain. >> You can try tweaking various knobs you find in Documentation/Makefile). >> DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the >> opposite (removes extra spaces). > > An easy workaround would be to add a comma between `-f` and 'git > branch'. That sounds like a good idea, regardless of the mark-up issue.
Re: [PATCH 3/3] check-ref-format doc: --branch validates and expands
Jonathan Nieder writes:
> From: Junio C Hamano
>
> "git check-ref-format --branch $name" feature was originally
> introduced (and was advertised) as a way for scripts to take any
> end-user supplied string (like "master", "@{-1}" etc.) and see if it
> is usable when Git expects to see a branch name, and also obtain the
> concrete branch name that the at-mark magic expands to.
>
> Emphasize that "see if it is usable" role in the description and
> clarify that the @{...} expansion only occurs when run from within a
> repository.
>
> [jn: split out from a larger patch]
>
> Helped-by: Jeff King
> Signed-off-by: Junio C Hamano
> ---
Missing sign-off, unlike the other two, intended?
I'll take these three to replace what I tentatively queued, not
necessarily because I like the change in 1/3 better, but because
these are explained much better; besides I prefer a version that at
least two people deeply looked at.
Thanks.
Re: Minor man page weirdness?
On Okt 17 2017, Jeff King wrote: > One other thing to try: > > rm git-branch.1 > make NO_MAN_BOLD_LITERAL=1 git-branch.1 That doesn't help: Reset to if exists already\&. Without \-f\fIgit branch\fR refuses to change an existing branch\&. In combination with Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] fetch doc: src side of refspec could be full SHA-1
Hi Junio,
Junio C Hamano wrote:
> Since a9d34933 ("Merge branch 'fm/fetch-raw-sha1'", 2015-06-01) we
> allow to fetch by an object name when the other side accepts such a
> request, but we never updated the documentation to match.
>
> Signed-off-by: Junio C Hamano
> ---
> Documentation/pull-fetch-param.txt | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
Good catch.
> --- a/Documentation/pull-fetch-param.txt
> +++ b/Documentation/pull-fetch-param.txt
> @@ -23,9 +23,11 @@ ifdef::git-pull[]
> endif::git-pull[]
> +
> The format of a parameter is an optional plus
> -`+`, followed by the source ref , followed
> +`+`, followed by the source , followed
> by a colon `:`, followed by the destination ref .
> -The colon can be omitted when is empty.
> +The colon can be omitted when is empty. is most
> +typically a ref, but it can also be an fully spelled hex object
> +name.
nits:
s/most typically/typically/
s/an fully/a fully/
With those tweaks,
Reviewed-by: Jonathan Nieder
Thanks.
Re: Minor man page weirdness?
On Tue, Oct 17, 2017 at 07:52:03PM +0200, Andreas Schwab wrote: > > Yes, it's in that step, but xmlto is just driving the xslt > > transformation done by docbook. So the interesting version is probably > > docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable). > > docbook-xsl-stylesheets-1.78.1+svn9743 Hmm. That could be it, though I was unable to bisect on the docbook repo since I couldn't get their build to work reliably. One other thing to try: rm git-branch.1 make NO_MAN_BOLD_LITERAL=1 git-branch.1 It's possible our snippet to add in the bolding causes problems. -Peff
Re: [PATCH v4 3/3] submodule: simplify decision tree whether to or not to fetch
On Mon, Oct 16, 2017 at 6:59 AM, Heiko Voigt wrote: > To make extending this logic later easier. > > Signed-off-by: Heiko Voigt Thanks for this readability fix! Stefan
Re: Minor man page weirdness?
On Okt 16 2017, Jeff King wrote: > We do have some hacks/workarounds for broken versions of the toolchain. > You can try tweaking various knobs you find in Documentation/Makefile). > DOCBOOK_SUPPRESS_SP sounds promising, but I think it actually does the > opposite (removes extra spaces). An easy workaround would be to add a comma between `-f` and 'git branch'. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH v4 1/3] fetch: add test to make sure we stay backwards compatible
On Mon, Oct 16, 2017 at 6:57 AM, Heiko Voigt wrote: > The current implementation of submodules supports on-demand fetch if > there is no .gitmodules entry for a submodule. Let's add a test to > document this behavior. > > Signed-off-by: Heiko Voigt > --- > t/t5526-fetch-submodules.sh | 42 +- > 1 file changed, 41 insertions(+), 1 deletion(-) > > diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh > index 42251f7f3a..43a22f680f 100755 > --- a/t/t5526-fetch-submodules.sh > +++ b/t/t5526-fetch-submodules.sh > @@ -478,7 +478,47 @@ test_expect_success "don't fetch submodule when newly > recorded commits are alrea > git fetch >../actual.out 2>../actual.err > ) && > ! test -s actual.out && > - test_i18ncmp expect.err actual.err > + test_i18ncmp expect.err actual.err && > + ( > + cd submodule && > + git checkout -q master > + ) For few instructions inside another repo, I tend to use the -C option: git -C submodule checkout -q master That saves a shell, which is noticeable cost on Windows I was told. (also fewer lines to type). Oh, I see, that is consistent with the rest of the file. Oh well. (Otherwise I would have lobbied to even move it further up and put it inside a test_when_finished "" > +' > + > +test_expect_success "'fetch.recurseSubmodules=on-demand' works also without > .gitmodule entry" ' > + ( > + cd downstream && > + git fetch --recurse-submodules > + ) && This is consistent with the rest of the file as well, so I shall refrain from complaining. ;) > + add_upstream_commit && > + head1=$(git rev-parse --short HEAD) && > + git add submodule && > + git rm .gitmodules && > + git commit -m "new submodule without .gitmodules" && > + printf "" >expect.out && This could be just : >expect.out no need to invoke a function to print nothing. > + head2=$(git rev-parse --short HEAD) && > + echo "From $pwd/." >expect.err.2 && > + echo " $head1..$head2 master -> origin/master" >>expect.err.2 > && > + head -3 expect.err >>expect.err.2 && > + ( > + cd downstream && > + rm .gitmodules && > + git config fetch.recurseSubmodules on-demand && > + # fake submodule configuration to avoid skipping submodule > handling > + git config -f .gitmodules submodule.fake.path fake && > + git config -f .gitmodules submodule.fake.url fakeurl && > + git add .gitmodules && > + git config --unset submodule.submodule.url && > + git fetch >../actual.out 2>../actual.err && > + # cleanup > + git config --unset fetch.recurseSubmodules && > + git reset --hard > + ) && > + test_i18ncmp expect.out actual.out && > + test_i18ncmp expect.err.2 actual.err && > + git checkout HEAD^ -- .gitmodules && > + git add .gitmodules && > + git commit -m "new submodule restored .gitmodules" > ' Thanks for writing this test. With or without the nits addressed, this is Reviewed-by: Stefan Beller
Re: Minor man page weirdness?
On Okt 17 2017, Jeff King wrote: > On Tue, Oct 17, 2017 at 07:25:28PM +0200, Andreas Schwab wrote: > >> >> I see this in git-branch.1: >> >> >> >> Reset to if exists already\&. >> >> Without >> >> \fB\-f\fR\fIgit branch\fR >> >> refuses to change an existing branch\&. In combination with >> >> >> >> This is with asciidoc 8.6.9. >> > >> > Thanks, that seems a likely culprit, then. What's in your git-branch.xml >> > after you build the documentation? >> >> Same as yours, with the space. I'd guess it's rather xmlto (version >> 0.0.25) that's the culprit here. > > Yes, it's in that step, but xmlto is just driving the xslt > transformation done by docbook. So the interesting version is probably > docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable). docbook-xsl-stylesheets-1.78.1+svn9743 Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH v4 2/3] implement fetching of moved submodules
On Mon, Oct 16, 2017 at 6:58 AM, Heiko Voigt wrote:
> We store the changed submodules paths to calculate which submodule needs
> fetching. This does not work for moved submodules since their paths do
> not stay the same in case of a moved submodules. In case of new
> submodules we do not have a path in the current checkout, since they
> just appeared in this fetch.
>
> It is more general to collect the submodule names for changes instead of
> their paths to include the above cases. If we do not have a
> configuration for a gitlink we rely on constructing a default name from
> the path if a git repository can be found at its path. We skip
> non-configured gitlinks whose default name collides with a configured
> one.
Thanks for working on this!
As detailed below, I wonder if it is easier (in maintenance, explaining
correctness, reviewing) if we'd rather keep two lists around. One for
based on names, and if we cannot lookup a name for a submodule, we
rather use the second path based list as a fallback. That would avoid
potential namespace collisions between names and paths, as well as
not having the confusion of mapping back and forth.
Most functions would then need to operate on path, as the name->path
mapping can be looked up for the first list, but the path->name mapping
cannot be looked up for the second list.
Cheers,
Stefan
> With the change described above we implement 'on-demand' fetching of
> changes in moved submodules.
>
> Signed-off-by: Heiko Voigt
> ---
> submodule-config.h | 3 +
> submodule.c | 138
>
> t/t5526-fetch-submodules.sh | 35 +++
> 3 files changed, 138 insertions(+), 38 deletions(-)
>
> diff --git a/submodule-config.h b/submodule-config.h
> index e3845831f6..a5503a5d17 100644
> --- a/submodule-config.h
> +++ b/submodule-config.h
> @@ -22,6 +22,9 @@ struct submodule {
> int recommend_shallow;
> };
>
> +#define SUBMODULE_INIT { NULL, NULL, NULL, RECURSE_SUBMODULES_NONE, \
> + NULL, NULL, SUBMODULE_UPDATE_STRATEGY_INIT, {0}, -1 };
> +
> struct submodule_cache;
> struct repository;
>
> diff --git a/submodule.c b/submodule.c
> index 63e7094e16..71d1773e2e 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -21,7 +21,7 @@
> #include "parse-options.h"
>
> static int config_update_recurse_submodules = RECURSE_SUBMODULES_OFF;
> -static struct string_list changed_submodule_paths = STRING_LIST_INIT_DUP;
> +static struct string_list changed_submodule_names = STRING_LIST_INIT_DUP;
> static int initialized_fetch_ref_tips;
> static struct oid_array ref_tips_before_fetch;
> static struct oid_array ref_tips_after_fetch;
> @@ -674,11 +674,11 @@ const struct submodule *submodule_from_ce(const struct
> cache_entry *ce)
> }
>
> static struct oid_array *submodule_commits(struct string_list *submodules,
> - const char *path)
> + const char *name)
> {
> struct string_list_item *item;
>
> - item = string_list_insert(submodules, path);
> + item = string_list_insert(submodules, name);
> if (item->util)
> return (struct oid_array *) item->util;
>
> @@ -687,39 +687,67 @@ static struct oid_array *submodule_commits(struct
> string_list *submodules,
> return (struct oid_array *) item->util;
> }
>
> +struct collect_changed_submodules_cb_data {
> + struct string_list *changed;
> + const struct object_id *commit_oid;
> +};
> +
> +/*
> + * this would normally be two functions: default_name_from_path() and
Please start the comment capitalised. (minor nit)
> + * path_from_default_name(). Since the default name is the same as
> + * the submodule path we can get away with just one function which only
> + * checks whether there is a submodule in the working directory at that
> + * location.
This is an interesting comment, as it hints that we ought to keep it that way.
Earlier I was wondering if we want to make the name distinctively different
than its path, as that will confuse users *less* IMHO. (I just remember
someone asking on the mailing list why their "rename" did not work, as
they just renamed everything in the .gitmodules that looked like the path)
As the path/name is confusing, I'd wish we'd be super concise, such that
errors are harder to sneak into. For example, the arguments name should
be "path" as that is the only thing we can look up using is_sub_pop_gently,
if a "name" is given, than it just works because the chosen default name
was its path.
> +static const char *default_name_or_path(const char *path_or_name)
> +{
> + int error_code;
> +
> + if (!is_submodule_populated_gently(path_or_name, &error_code))
> + return NULL;
> +
> + return path_or_name;
> +}
> +
> + if (submodule)
> + name = submodule->name;
> + else {
> + name = default_name_o
Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads
René Scharfe wrote: > Stop advertising -h as the short equivalent of --heads, because it's > used for showing a short help text for almost all other git commands. > Since the ba5f28bf79 (ls-remote: use parse-options api) it has only > been working when used together with other parameters anyway. > > Signed-off-by: Rene Scharfe > --- > Documentation/git-ls-remote.txt | 1 - > builtin/ls-remote.c | 4 +++- > 2 files changed, 3 insertions(+), 2 deletions(-) Yes, I think this is a good step. Reviewed-by: Jonathan Nieder I would be tempted to go even further and make "git ls-remote -h" print a warning to stderr to encourage people to update their scripts to use --heads instead. Thanks.
Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP
René Scharfe wrote:
> Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
> without any other options has shown the short help text of the command
> instead of acting as the short equivalent of --heads. Fix this
> regression by turning off internal handling of -h for repository setup,
> and option parsing, as well as the corresponding test in t0012.
>
> Reported-by: Thomas Rikl
> Analyzed-by: Martin Ågren
> Signed-off-by: Rene Scharfe
> ---
> builtin/ls-remote.c | 1 +
> git.c| 2 +-
> t/t0012-help.sh | 1 +
> t/t5512-ls-remote.sh | 6 ++
> 4 files changed, 9 insertions(+), 1 deletion(-)
Reviewed-by: Jonathan Nieder
[...]
> --- a/git.c
> +++ b/git.c
> @@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
> { "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
> { "log", cmd_log, RUN_SETUP },
> { "ls-files", cmd_ls_files, RUN_SETUP },
> - { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
> + { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },
There's only one other command that uses PARSE_OPT_NO_INTERNAL_HELP, and
that's "git archive". Does it need the same treatment?
More generally, is there a straightforward way for parse-options or
some compile-time analysis to catch if we forget to add
NO_INTERNAL_HELP to a command in the commands[] table?
Thanks,
Jonathan
Re: [PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP
On 17 October 2017 at 17:39, René Scharfe wrote: > Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h > without any other options has shown the short help text of the command > instead of acting as the short equivalent of --heads. Fix this > regression by turning off internal handling of -h for repository setup, > and option parsing, as well as the corresponding test in t0012. > > Reported-by: Thomas Rikl > Analyzed-by: Martin Ågren > Signed-off-by: Rene Scharfe > --- > builtin/ls-remote.c | 1 + > git.c| 2 +- > t/t0012-help.sh | 1 + > t/t5512-ls-remote.sh | 6 ++ > 4 files changed, 9 insertions(+), 1 deletion(-) Documentation/gitcli.txt might need updating. It says that "[c]ommands which have the enhanced option parser activated all understand ... -h". Of course, it already was in an incorrect state, since it pretends like no-one uses PARSE_OPT_NO_INTERNAL_HELP. Probably better to leave that until it's clear in which direction "git ls-remote -h" should go.
Re: [Alt. PATCH] ls-remote: deprecate -h as short for --heads
On 17 October 2017 at 17:39, René Scharfe wrote: > Stop advertising -h as the short equivalent of --heads, because it's > used for showing a short help text for almost all other git commands. > Since the ba5f28bf79 (ls-remote: use parse-options api) it has only > been working when used together with other parameters anyway. > > Signed-off-by: Rene Scharfe > --- > That would be step on the way towards more consistent command line > switches, in the same vein as d69155119 (t0012: test "-h" with > builtins). FWIW, my first inclination would be to go with a patch like this instead of the other two (where you introduce an exception so that git ls-remote -h does not display the usage). ba5f28bf79 was in 2.8.0. That's 18 months ago. Not an eternity, but still some time ago. Not fixing this regression has an obvious downside, but there's also a downside to adding the exception. As long as a lone -h may give the usage or do something else entirely, then -- put bluntly -- to know whether you can request the usage with git foo -h without risk of messing up your repo, you'll need to look up the usage some other way. At which point you've solved your original problem, without -h. Of course, we could promise that -h will give the usage or, in case of historical wart(s), do something harmless. But it seems a bit awkward, and might limit the perceived usefulness/safeness or -h. > diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt > index 5f2628c8f8..898836a111 100644 > --- a/Documentation/git-ls-remote.txt > +++ b/Documentation/git-ls-remote.txt > @@ -21,7 +21,6 @@ commit IDs. > > OPTIONS > --- > --h:: > --heads:: > -t:: > --tags:: Do we want to document -h as a deprecated alias, so that people have a slightly larger chance of noticing and adapting?
Re: [PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins
Hi,
René Scharfe wrote:
> Builtin commands have skipped repo setup when called with just a single
> option -h and thus shown their short help text even outside of
> repositories since 99caeed05d3 (Let 'git -h' show usage
> without a git dir).
>
> Add the flag NO_INTERNAL_HELP for builtins to opt out of this special
> treatment and provide a list of commands that are exempt from the
> corresponding test in t0012. That allows builtins to handle -h
> themselves.
>
> Signed-off-by: Rene Scharfe
> ---
> git.c | 6 +-
> t/t0012-help.sh | 9 -
> 2 files changed, 13 insertions(+), 2 deletions(-)
Makes sense.
[...]
> +++ b/git.c
[...]
> @@ -312,7 +313,10 @@ static int run_builtin(struct cmd_struct *p, int argc,
> const char **argv)
> const char *prefix;
>
> prefix = NULL;
> - help = argc == 2 && !strcmp(argv[1], "-h");
> + if (p->option & NO_INTERNAL_HELP)
> + help = 0;
> + else
> + help = argc == 2 && !strcmp(argv[1], "-h");
optional: this could be part of the same expression:
help = !(p->option & NO_INTERNAL_HELP) && argc == 2 && ...;
[...]
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -53,12 +53,19 @@ test_expect_success 'generate builtin list' '
> git --list-builtins >builtins
> '
>
> +cat >no_internal_help < +EOF
> +
> +test_expect_success 'generate list of builtins with internal help' '
> + grep -w -v -f no_internal_help builtins_internal_help
> +'
Hm, I don't see any instances of "grep -f" in the testsuite. Are
there portability pitfalls in it I don't know about? It's in POSIX,
so it looks pretty safe.
I would have been tempted to use comm, which is already used in
t6500-gc.sh:
comm -1 -3 no_internal_help builtins >builtins_internal_help
Other nits:
- preparatory 'cat' commands like the above tend to go inside
test_expect_success these days
- test that set up for later tests get marked as 'setup' or 'set up'
With whatever subset of the changes below looks good squashed in,
Reviewed-by: Jonathan Nieder
Thanks.
diff --git i/git.c w/git.c
index 9d1b8c4716..e4b340df7d 100644
--- i/git.c
+++ w/git.c
@@ -313,10 +313,8 @@ static int run_builtin(struct cmd_struct *p, int argc,
const char **argv)
const char *prefix;
prefix = NULL;
- if (p->option & NO_INTERNAL_HELP)
- help = 0;
- else
- help = argc == 2 && !strcmp(argv[1], "-h");
+ help = !(p->option & NO_INTERNAL_HELP) &&
+ argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
diff --git i/t/t0012-help.sh w/t/t0012-help.sh
index c5a748837c..73fdfd99ab 100755
--- i/t/t0012-help.sh
+++ w/t/t0012-help.sh
@@ -49,15 +49,11 @@ test_expect_success "--help does not work for guides" "
test_i18ncmp expect actual
"
-test_expect_success 'generate builtin list' '
- git --list-builtins >builtins
-'
-
-cat >no_internal_help no_internal_help <<-\EOF &&
+ EOF
+ git --list-builtins >builtins &&
+ comm -1 -3 no_internal_help builtins >builtins_internal_help
'
while read builtin
Re: [PATCH/RFC] git-post: the opposite of git-cherry-pick
Am 17.10.2017 um 01:01 schrieb Rafael Ascensao: This is worth discussing, though not my preference. The picture to "pick cherries" has become quite common, and now that we use it for the name of the command, "cherry-pick", the direction of flow is quite obvious and strongly implied: from somewhere else to me (and not to somebody else). What if we borrow '--onto' from rebase and make it cherry-pick --onto ? I actually like this. Although I would miss the convenience that the source defaults to HEAD. Unless we make a special case for --onto, that is, of course. -- Hannes
Re: Minor man page weirdness?
On Tue, Oct 17, 2017 at 07:25:28PM +0200, Andreas Schwab wrote: > >> I see this in git-branch.1: > >> > >> Reset to if exists already\&. > >> Without > >> \fB\-f\fR\fIgit branch\fR > >> refuses to change an existing branch\&. In combination with > >> > >> This is with asciidoc 8.6.9. > > > > Thanks, that seems a likely culprit, then. What's in your git-branch.xml > > after you build the documentation? > > Same as yours, with the space. I'd guess it's rather xmlto (version > 0.0.25) that's the culprit here. Yes, it's in that step, but xmlto is just driving the xslt transformation done by docbook. So the interesting version is probably docbook. I have docbook-xsl 1.79.1+dfsg-2 (from Debian unstable). -Peff
Re: Minor man page weirdness?
On Okt 17 2017, Jeff King wrote: > On Tue, Oct 17, 2017 at 06:29:59PM +0200, Andreas Schwab wrote: > >> On Okt 16 2017, Jeff King wrote: >> >> > I get: >> > >> > Reset to if exists already\&. >> > Without >> > \fB\-f\fR >> > \fIgit branch\fR >> > refuses to change an existing branch\&. In combination with >> >> I see this in git-branch.1: >> >> Reset to if exists already\&. >> Without >> \fB\-f\fR\fIgit branch\fR >> refuses to change an existing branch\&. In combination with >> >> This is with asciidoc 8.6.9. > > Thanks, that seems a likely culprit, then. What's in your git-branch.xml > after you build the documentation? Same as yours, with the space. I'd guess it's rather xmlto (version 0.0.25) that's the culprit here. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: Is t5601 flaky for anybody else?
On 10/17, Junio C Hamano wrote:
> I seem to be seeing sporadic errors with this test, and today I got
> annoyed enough to do
>
> cd t && while sh t5601-clone.sh -i -v; do :; done
>
> I saw an error from "cp" saying "plink.exe - text file busy" or
> something like that at around test #45; here is an workaround that
> seems to work (the above loop is spinning without problem for
> several minutes now).
I haven't noticed any issues myself but maybe this has something to do
with my changes to this test in the 'bw/protocol-v1' topic?
>
> t/t5601-clone.sh | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 9c56f771b6..50e40abb11 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -308,6 +308,7 @@ test_expect_success 'clone checking out a tag' '
>
> setup_ssh_wrapper () {
> test_expect_success 'setup ssh wrapper' '
> + rm -f "$TRASH_DIRECTORY/ssh-wrapper$X" &&
> cp "$GIT_BUILD_DIR/t/helper/test-fake-ssh$X" \
> "$TRASH_DIRECTORY/ssh-wrapper$X" &&
> GIT_SSH="$TRASH_DIRECTORY/ssh-wrapper$X" &&
> @@ -318,6 +319,7 @@ setup_ssh_wrapper () {
> }
>
> copy_ssh_wrapper_as () {
> + rm -f "${1%$X}$X" &&
> cp "$TRASH_DIRECTORY/ssh-wrapper$X" "${1%$X}$X" &&
> GIT_SSH="${1%$X}$X" &&
> export GIT_SSH
--
Brandon Williams
Re: Minor man page weirdness?
On Tue, Oct 17, 2017 at 06:29:59PM +0200, Andreas Schwab wrote: > On Okt 16 2017, Jeff King wrote: > > > I get: > > > > Reset to if exists already\&. > > Without > > \fB\-f\fR > > \fIgit branch\fR > > refuses to change an existing branch\&. In combination with > > I see this in git-branch.1: > > Reset to if exists already\&. Without > \fB\-f\fR\fIgit branch\fR > refuses to change an existing branch\&. In combination with > > This is with asciidoc 8.6.9. Thanks, that seems a likely culprit, then. What's in your git-branch.xml after you build the documentation? -Peff
Re: Minor man page weirdness?
On Okt 16 2017, Jeff King wrote: > I get: > > Reset to if exists already\&. Without > \fB\-f\fR > \fIgit branch\fR > refuses to change an existing branch\&. In combination with I see this in git-branch.1: Reset to if exists already\&. Without \fB\-f\fR\fIgit branch\fR refuses to change an existing branch\&. In combination with This is with asciidoc 8.6.9. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
[PATCH] l10n: de.po: translate 70 new messages
Translate 70 new messages came from git.pot update in 25eab542b (l10n: git.pot: v2.15.0 round 1 (68 new, 36 removed)) and 9c07fab78 (l10n: git.pot: v2.15.0 round 2 (2 new, 2 removed)). Signed-off-by: Ralf Thielow --- po/de.po | 253 ++- 1 file changed, 106 insertions(+), 147 deletions(-) diff --git a/po/de.po b/po/de.po index c76fe575c..0619c4988 100644 --- a/po/de.po +++ b/po/de.po @@ -92,21 +92,20 @@ msgid "" "\n" " git checkout -b \n" "\n" msgstr "" "Hinweis: Checke '%s' aus.\n" "\n" -"Sie befinden sich im Zustand eines 'lösgelösten HEAD'. Sie können sich\n" +"Sie befinden sich im Zustand eines 'losgelösten HEAD'. Sie können sich\n" "umschauen, experimentelle Änderungen vornehmen und diese committen, und\n" "Sie können alle möglichen Commits, die Sie in diesem Zustand machen,\n" "ohne Auswirkungen auf irgendeinen Branch verwerfen, indem Sie einen\n" "weiteren Checkout durchführen.\n" "\n" "Wenn Sie einen neuen Branch erstellen möchten, um Ihre erstellten Commits\n" -"zu behalten, können Sie das (jetzt oder später) durch einen weiteren " -"Checkout\n" +"zu behalten, können Sie das (jetzt oder später) durch einen weiteren Checkout\n" "mit der Option -b tun. Beispiel:\n" "\n" " git checkout -b \n" "\n" #: apply.c:58 @@ -1066,52 +1065,50 @@ msgstr "" #: branch.c:67 #, c-format msgid "Not setting branch %s as its own upstream." msgstr "Branch %s kann nicht sein eigener Upstream-Branch sein." #: branch.c:93 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track remote branch '%s' from '%s' by rebasing." -msgstr "" -"Branch %s konfiguriert zum Folgen von Remote-Branch %s von %s durch Rebase." +msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Branch '%s' von '%s' durch Rebase." #: branch.c:94 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track remote branch '%s' from '%s'." -msgstr "Branch %s konfiguriert zum Folgen von Remote-Branch %s von %s." +msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Branch '%s' von '%s'." #: branch.c:98 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track local branch '%s' by rebasing." -msgstr "Branch %s konfiguriert zum Folgen von lokalem Branch %s durch Rebase." +msgstr "Branch '%s' konfiguriert zum Folgen von lokalem Branch '%s' durch Rebase." #: branch.c:99 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track local branch '%s'." -msgstr "Branch %s konfiguriert zum Folgen von lokalem Branch %s." +msgstr "Branch '%s' konfiguriert zum Folgen von lokalem Branch '%s'." #: branch.c:104 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track remote ref '%s' by rebasing." -msgstr "Branch %s konfiguriert zum Folgen von Remote-Referenz %s durch Rebase." +msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Referenz '%s' durch Rebase." #: branch.c:105 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track remote ref '%s'." -msgstr "Branch %s konfiguriert zum Folgen von Remote-Referenz %s." +msgstr "Branch '%s' konfiguriert zum Folgen von Remote-Referenz '%s'." #: branch.c:109 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track local ref '%s' by rebasing." -msgstr "" -"Branch %s konfiguriert zum Folgen von lokaler Referenz %s durch Rebase." +msgstr "Branch '%s' konfiguriert zum Folgen von lokaler Referenz '%s' durch Rebase." #: branch.c:110 -#, fuzzy, c-format +#, c-format msgid "Branch '%s' set up to track local ref '%s'." -msgstr "Branch %s konfiguriert zum Folgen von lokaler Referenz %s." +msgstr "Branch '%s' konfiguriert zum Folgen von lokaler Referenz '%s'." #: branch.c:119 msgid "Unable to write upstream branch configuration" msgstr "Konnte Konfiguration zu Upstream-Branch nicht schreiben." #: branch.c:156 @@ -1623,13 +1620,13 @@ msgid " Unknown dirstat parameter '%s'\n" msgstr " Unbekannter \"dirstat\" Parameter '%s'\n" #: diff.c:281 msgid "" "color moved setting must be one of 'no', 'default', 'zebra', 'dimmed_zebra', " "'plain'" -msgstr "" +msgstr "\"color moved\"-Einstellung muss eines von diesen sein: 'no', 'default', 'zebra', 'dimmed_zebra', 'plain'" #: diff.c:341 #, c-format msgid "Unknown value for 'diff.submodule' config variable: '%s'" msgstr "Unbekannter Wert in Konfigurationsvariable 'diff.dirstat': '%s'" @@ -1706,15 +1703,14 @@ msgstr "Konnte Verzeichnisse für '%s' nicht erstellen." #: dir.c:2915 #, c-format msgid "could not migrate git directory from '%s' to '%s'" msgstr "Konnte Git-Verzeichnis nicht von '%s' nach '%s' migrieren." #: entry.c:176 -#, fuzzy msgid "Filtering content" -msgstr "Tag-Inhalte ausgeben" +msgstr "Filtere Inhalt" #: entry.c:433 #, c-format msgid "could not stat file '%s'" msgstr "konnte Datei '%s' nicht lesen" @@ -2335,13 +2331,12 @@ msgstr "" #: merge-recursive.c:1917 #, c-format msgid "Adding %s" msgstr "Füge %s hinzu" #: merge-recursive.c:1954 -#, fuzzy msgid
[PATCH] sequencer.c: unify an error message
Change an error message in sequencer.c for the case that
we could not write to a file to match other instances.
Signed-off-by: Ralf Thielow
---
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 75f5356f6..f2a10cc4f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2948,7 +2948,7 @@ int rearrange_squash(void)
if (fd < 0)
res = error_errno(_("could not open '%s'"), todo_file);
else if (write(fd, buf.buf, buf.len) < 0)
- res = error_errno(_("could not write '%s'"), todo_file);
+ res = error_errno(_("could not write to '%s'"),
todo_file);
else if (ftruncate(fd, buf.len) < 0)
res = error_errno(_("could not truncate '%s'"),
todo_file);
--
2.15.0.rc1.299.gda03b47c3
Re: Multiple paths in GIT_EXEC_PATH
On Tue, Oct 17, 2017 at 06:21:22PM +0300, Nikolay Yakimov wrote: > Hello. After updating to a recent git release (2.14, I believe, but > possibly earlier), setting GIT_EXEC_PATH to multiple directories > stopped working. It did work before, and I believe the culprit is > 'git-sh-setup', which uses 'git --exec-path' output directly, while > most other git components observe PATH syntax, i.e. multiple paths > with colon separator. Is this intended, or is it an oversight? > > For why I need that is another matter. Long story short, I need git to > look for '.gitignore' in a particular non-standard location, since I > have multiple git repositories in the same workdir (that workdir being > $HOME and git repositories being stores for my different configs) The commit that changed what you described is 1073094f3 (git-sh-setup: be explicit where to dot-source git-sh-i18n from., 2016-10-29). That commit claims there were already scripts that assumed GIT_EXEC_PATH is just a single entry. That commit was included in v2.11. There was also a recent thread[0] about it that discussed this issue, where someone stated that indeed treating GIT_EXEC_PATH with the same semantics as PATH has been broken for a while, but it seems there are no real plans to fix it. [0]:https://public-inbox.org/git/20170928223134.GA30744@varnish/
[PATCH 2/2] ls-remote: use PARSE_OPT_NO_INTERNAL_HELP
Since ba5f28bf79 (ls-remote: use parse-options api) git ls-remote -h
without any other options has shown the short help text of the command
instead of acting as the short equivalent of --heads. Fix this
regression by turning off internal handling of -h for repository setup,
and option parsing, as well as the corresponding test in t0012.
Reported-by: Thomas Rikl
Analyzed-by: Martin Ågren
Signed-off-by: Rene Scharfe
---
builtin/ls-remote.c | 1 +
git.c| 2 +-
t/t0012-help.sh | 1 +
t/t5512-ls-remote.sh | 6 ++
4 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..5f27d252b4 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -68,6 +68,7 @@ int cmd_ls_remote(int argc, const char **argv, const char
*prefix)
};
argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
+PARSE_OPT_NO_INTERNAL_HELP |
PARSE_OPT_STOP_AT_NON_OPTION);
dest = argv[0];
diff --git a/git.c b/git.c
index 9d1b8c4716..056a123506 100644
--- a/git.c
+++ b/git.c
@@ -421,7 +421,7 @@ static struct cmd_struct commands[] = {
{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
{ "log", cmd_log, RUN_SETUP },
{ "ls-files", cmd_ls_files, RUN_SETUP },
- { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
+ { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY | NO_INTERNAL_HELP },
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
{ "mailsplit", cmd_mailsplit },
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 74eeead168..05d8cf9b49 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -54,6 +54,7 @@ test_expect_success 'generate builtin list' '
'
cat >no_internal_help long &&
+ test_cmp long short
+'
+
test_done
--
2.14.2
[Alt. PATCH] ls-remote: deprecate -h as short for --heads
Stop advertising -h as the short equivalent of --heads, because it's
used for showing a short help text for almost all other git commands.
Since the ba5f28bf79 (ls-remote: use parse-options api) it has only
been working when used together with other parameters anyway.
Signed-off-by: Rene Scharfe
---
That would be step on the way towards more consistent command line
switches, in the same vein as d69155119 (t0012: test "-h" with
builtins).
Documentation/git-ls-remote.txt | 1 -
builtin/ls-remote.c | 4 +++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
index 5f2628c8f8..898836a111 100644
--- a/Documentation/git-ls-remote.txt
+++ b/Documentation/git-ls-remote.txt
@@ -21,7 +21,6 @@ commit IDs.
OPTIONS
---
--h::
--heads::
-t::
--tags::
diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index c4be98ab9e..840deedbef 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -56,7 +56,9 @@ int cmd_ls_remote(int argc, const char **argv, const char
*prefix)
N_("path of git-upload-pack on the remote host"),
PARSE_OPT_HIDDEN },
OPT_BIT('t', "tags", &flags, N_("limit to tags"), REF_TAGS),
- OPT_BIT('h', "heads", &flags, N_("limit to heads"), REF_HEADS),
+ OPT_BIT(0, "heads", &flags, N_("limit to heads"), REF_HEADS),
+ { OPTION_BIT, 'h', NULL, &flags, NULL, N_("limit to heads"),
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, REF_HEADS },
OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"),
REF_NORMAL),
OPT_BOOL(0, "get-url", &get_url,
N_("take url..insteadOf into account")),
--
2.14.2
[PATCH 1/2] git: add NO_INTERNAL_HELP flag for builtins
Builtin commands have skipped repo setup when called with just a single
option -h and thus shown their short help text even outside of
repositories since 99caeed05d3 (Let 'git -h' show usage
without a git dir).
Add the flag NO_INTERNAL_HELP for builtins to opt out of this special
treatment and provide a list of commands that are exempt from the
corresponding test in t0012. That allows builtins to handle -h
themselves.
Signed-off-by: Rene Scharfe
---
git.c | 6 +-
t/t0012-help.sh | 9 -
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/git.c b/git.c
index 9e96dd4090..9d1b8c4716 100644
--- a/git.c
+++ b/git.c
@@ -298,6 +298,7 @@ static int handle_alias(int *argcp, const char ***argv)
#define NEED_WORK_TREE (1<<3)
#define SUPPORT_SUPER_PREFIX (1<<4)
#define DELAY_PAGER_CONFIG (1<<5)
+#define NO_INTERNAL_HELP (1<<6)
struct cmd_struct {
const char *cmd;
@@ -312,7 +313,10 @@ static int run_builtin(struct cmd_struct *p, int argc,
const char **argv)
const char *prefix;
prefix = NULL;
- help = argc == 2 && !strcmp(argv[1], "-h");
+ if (p->option & NO_INTERNAL_HELP)
+ help = 0;
+ else
+ help = argc == 2 && !strcmp(argv[1], "-h");
if (!help) {
if (p->option & RUN_SETUP)
prefix = setup_git_directory();
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index 487b92a5de..74eeead168 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -53,12 +53,19 @@ test_expect_success 'generate builtin list' '
git --list-builtins >builtins
'
+cat >no_internal_help output 2>&1 &&
test_i18ngrep usage output
'
-done
Multiple paths in GIT_EXEC_PATH
Hello. After updating to a recent git release (2.14, I believe, but possibly earlier), setting GIT_EXEC_PATH to multiple directories stopped working. It did work before, and I believe the culprit is 'git-sh-setup', which uses 'git --exec-path' output directly, while most other git components observe PATH syntax, i.e. multiple paths with colon separator. Is this intended, or is it an oversight? For why I need that is another matter. Long story short, I need git to look for '.gitignore' in a particular non-standard location, since I have multiple git repositories in the same workdir (that workdir being $HOME and git repositories being stores for my different configs)
Attendees:- Society for Neuroscience - SfN-2017
Hi , Would you be interested in the "Attendees list of Society for Neuroscience - SfN-2017" Let me know your interest to send you the number of Attendees and cost . Regards, Donald Charles, If you do not wish to receive future emails from us, please reply as 'leave out'. <>
[PATCH] status: do not get confused by submodules in excluded directories
We meticulously pass the `exclude` flag to the `treat_directory()`
function so that we can indicate that files in it are excluded rather
than untracked when recursing.
But we did not yet treat submodules the same way.
Signed-off-by: Johannes Schindelin
---
Published-As: https://github.com/dscho/git/releases/tag/submodule-in-excluded-v1
Fetch-It-Via: git fetch https://github.com/dscho/git submodule-in-excluded-v1
dir.c | 2 +-
t/t7061-wtstatus-ignore.sh | 14 ++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c
index 1d17b800cf3..9987011da57 100644
--- a/dir.c
+++ b/dir.c
@@ -1392,7 +1392,7 @@ static enum path_treatment treat_directory(struct
dir_struct *dir,
if (!(dir->flags & DIR_NO_GITLINKS)) {
unsigned char sha1[20];
if (resolve_gitlink_ref(dirname, "HEAD", sha1) == 0)
- return path_untracked;
+ return exclude ? path_excluded : path_untracked;
}
return path_recurse;
}
diff --git a/t/t7061-wtstatus-ignore.sh b/t/t7061-wtstatus-ignore.sh
index fc6013ba3c8..8c849a4cd2f 100755
--- a/t/t7061-wtstatus-ignore.sh
+++ b/t/t7061-wtstatus-ignore.sh
@@ -272,4 +272,18 @@ test_expect_success 'status ignored tracked directory with
uncommitted file in t
test_cmp expected actual
'
+cat >expected <<\EOF
+!! tracked/submodule/
+EOF
+
+test_expect_success 'status ignores submodule in excluded directory' '
+ git init tracked/submodule &&
+ (
+ cd tracked/submodule &&
+ test_commit initial
+ ) &&
+ git status --porcelain --ignored -u tracked/submodule >actual &&
+ test_cmp expected actual
+'
+
test_done
base-commit: 111ef79afe185f8731920569450f6a65320f5d5f
--
2.14.2.windows.3
Re: Deleting a branch after merging it results in "there may be uncommitted changes"
On 10/05/2017 05:04 AM, Joshua Lamusga wrote: Anyway, I follow a very simple merging model for this one-person project. Recently, I made a new local branch off of develop called feature-printing. After checking out feature-printing, making my changes, and committing changes, I merged it with develop. I then immediately tried to delete feature-printing, which resulted in a prompt asking if I was sure since it might contain uncommitted changes. Though I've seen this problem many times on the internet, I haven't seen it in the context of literally just merging. There are 0 steps between merging and deleting the old branch. What's the exact warning? It seems unlikely that your Git interface would complain about *uncommitted* changes in this context. All of this is done in Visual Studio's GUI for Git. Any ideas? Maybe ask on a Visual Studio forum instead, in case this is something generated by the front end? Florian
Salut mon bon ami!.
Salut mon bon ami! Vous avez été désigné comme le plus proche parent de mon client en retard qui a déposé les 7,3 millions de dollars avant sa mort et votre profil a été ajouté à notre base de données. Vous êtes maintenant ravi de répondre avec effet immédiat pour plus information. Voici votre numéro de badge d'inscription: 678378
[PATCH v2] doc: list filter-branch subdirectory-filter first
From: David Glasser
The docs claim that filters are applied in the listed order, so
subdirectory-filter should come first.
For consistency, apply the same order to the SYNOPSIS and the script's usage, as
well as the switch while parsing arguments.
Add missing --prune-empty to the script's usage.
Signed-off-by: David Glasser
---
Documentation/git-filter-branch.txt | 20 ++--
git-filter-branch.sh| 20 ++--
2 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-filter-branch.txt
b/Documentation/git-filter-branch.txt
index 9e5169aa64f4f..394f74451a659 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -8,11 +8,11 @@ git-filter-branch - Rewrite branches
SYNOPSIS
[verse]
-'git filter-branch' [--setup ] [--env-filter ]
- [--tree-filter ] [--index-filter ]
- [--parent-filter ] [--msg-filter ]
- [--commit-filter ] [--tag-name-filter ]
- [--subdirectory-filter ] [--prune-empty]
+'git filter-branch' [--setup ] [--subdirectory-filter ]
+ [--env-filter ] [--tree-filter ]
+ [--index-filter ] [--parent-filter ]
+ [--msg-filter ] [--commit-filter ]
+ [--tag-name-filter ] [--prune-empty]
[--original ] [-d ] [-f | --force]
[--] [...]
@@ -89,6 +89,11 @@ OPTIONS
can be used or modified in the following filter steps except
the commit filter, for technical reasons.
+--subdirectory-filter ::
+ Only look at the history which touches the given subdirectory.
+ The result will contain that directory (and only that) as its
+ project root. Implies <>.
+
--env-filter ::
This filter may be used if you only need to modify the environment
in which the commit will be performed. Specifically, you might
@@ -167,11 +172,6 @@ be removed, buyer beware. There is also no support for
changing the
author or timestamp (or the tag message for that matter). Tags which point
to other tags will be rewritten to point to the underlying commit.
---subdirectory-filter ::
- Only look at the history which touches the given subdirectory.
- The result will contain that directory (and only that) as its
- project root. Implies <>.
-
--prune-empty::
Some filters will generate empty commits that leave the tree untouched.
This option instructs git-filter-branch to remove such commits if they
diff --git a/git-filter-branch.sh b/git-filter-branch.sh
index 3a74602ef3771..b7827e745a92a 100755
--- a/git-filter-branch.sh
+++ b/git-filter-branch.sh
@@ -81,12 +81,12 @@ set_ident () {
finish_ident COMMITTER
}
-USAGE="[--setup ] [--env-filter ]
- [--tree-filter ] [--index-filter ]
- [--parent-filter ] [--msg-filter ]
- [--commit-filter ] [--tag-name-filter ]
- [--subdirectory-filter ] [--original ]
- [-d ] [-f | --force]
+USAGE="[--setup ] [--subdirectory-filter ]
+ [--env-filter ] [--tree-filter ]
+ [--index-filter ] [--parent-filter ]
+ [--msg-filter ] [--commit-filter ]
+ [--tag-name-filter ] [--prune-empty]
+ [--original ] [-d ] [-f | --force]
[--] [...]"
OPTIONS_SPEC=
@@ -153,6 +153,10 @@ do
--setup)
filter_setup="$OPTARG"
;;
+ --subdirectory-filter)
+ filter_subdir="$OPTARG"
+ remap_to_ancestor=t
+ ;;
--env-filter)
filter_env="$OPTARG"
;;
@@ -174,10 +178,6 @@ do
--tag-name-filter)
filter_tag_name="$OPTARG"
;;
- --subdirectory-filter)
- filter_subdir="$OPTARG"
- remap_to_ancestor=t
- ;;
--original)
orig_namespace=$(expr "$OPTARG/" : '\(.*[^/]\)/*$')/
;;
--
https://github.com/git/git/pull/415
Hi Dear,
Hi Dear, how are you today I hope that everything is OK with you as it is my great pleasure to contact you in having communication with you starting from today, i was just going through the Internet search when i found your email address, I want to make a very new and special friend, so i decided to contact you to see how we can make it work if we can. Please i wish you will have the desire with me so that we can get to know each other better and see what happens in future. My name is Lisa Williams, I am an American presently I live in the UK, I will be very happy if you can write me through my private email address(lisa.williams...@yahoo.com) for easy communication so that we can know each other, I will give you my pictures and details about me. bye Lisa
Re: [ANNOUNCE] Git for Windows 2.14.2(3)
Hi Steve, On Mon, 16 Oct 2017, Steve Hoelzer wrote: > On Mon, Oct 16, 2017 at 5:57 AM, Johannes Schindelin > wrote: > > Hi Steve, > > > > On Sun, 15 Oct 2017, Johannes Schindelin wrote: > > > >> On Fri, 13 Oct 2017, Steve Hoelzer wrote: > >> > >> > On Thu, Oct 12, 2017 at 5:53 PM, Johannes Schindelin > >> > wrote: > >> > > > >> > > It is my pleasure to announce that Git for Windows 2.14.2(3) is > >> > > available from: > >> > > > >> > > https://git-for-windows.github.io/ > >> > > > >> > > Changes since Git for Windows v2.14.2(2) (October 5th 2017) > >> > > > >> > > New Features > >> > > > >> > > * Comes with Git LFS v2.3.3. > >> > > >> > I just ran "git update" and afterward "git version" reported > >> > 2.14.2(3), but "git lfs version" still said 2.3.2. > >> > > >> > I also uninstalled/reinstalled Git for Windows and LFS is still 2.3.2. > >> > >> Ah bummer. I forgot to actually update it in the VM where I build the > >> releases :-( > >> > >> Will work on it tomorrow. > > > > I'll actually use this opportunity to revamp a part of Git for Windows' > > release engineering process to try to prevent similar things from > > happening in the future. > > > > Also, cURL v7.56.1 is slated to be released in exactly one week, and I > > have some important installer work to do this week, so I'll just defer the > > new Git for Windows version tentatively to Monday, 23rd. > > > > Git LFS users can always install Git LFS v2.3.3 specifically in the > > meantime, or use Git for Windows' snapshot versions > > (https://wingit.blob.core.windows.net/files/index.html). > > Sounds like a good plan. > > I think I have successfully updated to LFS 2.3.3 by copying the new > git-lfs.exe into C:\Program Files\Git\mingw64\bin. Is that right way > to do it? That should be enough in your case, as the config is already written by the Git for Windows installer. In general, the best way to install a new Git LFS version seems to be to download and run the Git LFS installer: https://github.com/git-lfs/git-lfs/releases/download/v2.3.3/git-lfs-windows-2.3.3.exe Ciao, Johannes
Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch
Jonathan Nieder writes: > Junio C Hamano wrote: >> Jonathan Nieder writes: > >>> Handles the nongit case in strbuf_check_branch_ref instead of >>> introducing a new check_branch_ref_format helper. >> >> I view that as a regression, actually. Don't we want a function >> that does not require a strbuf when asking a simple question: "I >> have a string, and I want to see if that is a valid name"? > > *shrug* I found the change easier to read, and it also sidesteps the > which-header question. It also ensures that other > strbuf_check_branch_ref callers are safe without having to audit them. Please ignore the above, which was merely an impression _without_ and before having received any patch to comment on ;-) Quite frankly, this is a Meh topic that won't have to hit even 'next' before the final. The color.ui=always thing has a lot more urgency, and this was merely what I did while waiting for others to react to that topic.
Re: [PATCH] check-ref-format: require a repository for --branch
Junio C Hamano wrote: > Jonathan Nieder writes: >> And in that spirit, I think the patch you replied with aims to go in >> the right direction, by providing the core functionality when in a >> repository while avoiding breaking such a script outside of one >> (though I do not understand it fully yet). > > Given that, is it safe for me to ignore this earlier one > >> For what it's worth, I don't agree with this repurposing of >> "check-ref-format --branch" at all. > > as reacting to the patch without reading what it does? On second reading, yes, I was reacting to the discussion leading up to the patch instead of the patch. The patch looks good. Sorry for the noise, Jonathan
Re: [L10N] Kickoff of translation for Git 2.15.0 round 2
2017-10-17 8:53 GMT+02:00 Junio C Hamano :
> Jiang Xin writes:
>
>> Git v2.15.0-rc1 released with a typo fix from commit dfab1eac23
>> ("i18n: add a missing space in message", Sun Oct 8 14:18:39 2017 +0200).
>> This time there are 2 updated messages need to be translated since last
>> update. Let's start new round of translation for Git 2.15.0.
>
> Today's pushout included 3247edbb ("sequencer.c: fix and unify error
> messages in rearrange_squash()", 2017-10-15), which unfortunately
> added this to the mix:
>
> error_errno(_("could not write '%s'"), todo_file);
>
> There is another one "could not truncate '%s'" added by the same
> commit, but the codebase already had another instance of the same
> string, so it won't cause trouble.
>
> Regarding the "could not write" thing, there are quite a few
> instances of "coulld not write to '%s'" with %s set to the filename,
> and I strongly suspect that this new one should be further updated
> to match them (Ralf promoted from Cc: to To: for this). That would
> have a nice side effect of making today's last-minute merges a
> non-event from translators' point of view ;-)
>
>
>
I didn't expect this message to be a new one for translation. I'll send
a patch later to update the message to match the other instances.
Re: [PATCH] check-ref-format: require a repository for --branch
Junio C Hamano wrote: > Jonathan Nieder writes: >> And in that spirit, I think the patch you replied with aims to go in >> the right direction, by providing the core functionality when in a >> repository while avoiding breaking such a script outside of one >> (though I do not understand it fully yet). > > Given that, is it safe for me to ignore this earlier one > >> For what it's worth, I don't agree with this repurposing of >> "check-ref-format --branch" at all. > > as reacting to the patch without reading what it does? Are you saying I should have ignored the commit message? Recording future plans via the commit message is part of what the patch does, after all. >> Junio C Hamano wrote: >>>(e.g. a wrapper to "git >>> clone" that wants to verify the name it is going to give to the "-b" >>> option), and a check desired in such a context is different from >>> (and is stricter than) feeding refs/heads/$name to the same command >>> without the "--branch" option. >> >> Can you say more about this example? E.g. why is this hypothetical >> wrapper unable to rely on "git clone -b"'s own error handling? > > If I have to guess what you meant, perhaps the Porcelain wanted to > diagnose bad parameter that would be rejected _before_ letting clone > spend cycles and possibly network bandwidth? > > But this was my way of rephrasing an earlier example you used while > objecting to Peff's change [...] > so my answer to the question in the message I am directly responding > to would be "You tell me." ;-) Hm. Does the example in https://public-inbox.org/git/20171017070808.plddffhzdobye...@aiede.mtv.corp.google.com/ make it clearer? The goal of such a script is *not* error handling --- that is just a pleasant side-benefit. It is to be able to handle all branch specifiers from the user (and even a default branch that is not from the user) uniformly. Thanks, Jonathan
Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch
Junio C Hamano wrote: > Jonathan Nieder writes: >> Handles the nongit case in strbuf_check_branch_ref instead of >> introducing a new check_branch_ref_format helper. > > I view that as a regression, actually. Don't we want a function > that does not require a strbuf when asking a simple question: "I > have a string, and I want to see if that is a valid name"? *shrug* I found the change easier to read, and it also sidesteps the which-header question. It also ensures that other strbuf_check_branch_ref callers are safe without having to audit them. But feel free to tweak that back if you like, or I can tomorrow. Jonathan
Re: [PATCH] patch reply
I just sent a patch to a new thread which is not what I wanted. However I already sent the same patch a few days ago: https://public-inbox.org/git/CAK7vU=2ePR3jQsgu=rxsmrxytaahqxc0sfrn5yozlzqzp2z...@mail.gmail.com/ 2017-10-17 6:01 GMT+02:00 Junio C Hamano : > Thais Diniz writes: > >> +Just to clarify I did not see Marius patch. >> +Did see Marius' comment saying he would look it in the leftoverbits list, >> +but since i didn't see any patch i thought i could work on it and did so >> based on Stephan's comment >> +(which i suppose Mario also did and that is why the code resulted to be >> similar). > > In any case, both versions share exactly the same "don't call > get_multi() to grab the same configuration values from inside the > callback of git_config()" issue, so whoever works on it to complete > the topic, it needs further work.
[PATCH 3/3] check-ref-format doc: --branch validates and expands
From: Junio C Hamano
"git check-ref-format --branch $name" feature was originally
introduced (and was advertised) as a way for scripts to take any
end-user supplied string (like "master", "@{-1}" etc.) and see if it
is usable when Git expects to see a branch name, and also obtain the
concrete branch name that the at-mark magic expands to.
Emphasize that "see if it is usable" role in the description and
clarify that the @{...} expansion only occurs when run from within a
repository.
[jn: split out from a larger patch]
Helped-by: Jeff King
Signed-off-by: Junio C Hamano
---
Documentation/git-check-ref-format.txt | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-check-ref-format.txt
b/Documentation/git-check-ref-format.txt
index 92777cef25..cf0a0b7df2 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -77,7 +77,14 @@ reference name expressions (see linkgit:gitrevisions[7]):
. at-open-brace `@{` is used as a notation to access a reflog entry.
-With the `--branch` option, it expands the ``previous branch syntax''
+With the `--branch` option, the command takes a name and checks if
+it can be used as a valid branch name (e.g. when creating a new
+branch). The rule `git check-ref-format --branch $name` implements
+may be stricter than what `git check-ref-format refs/heads/$name`
+says (e.g. a dash may appear at the beginning of a ref component,
+but it is explicitly forbidden at the beginning of a branch name).
+When run with `--branch` option in a repository, the input is first
+expanded for the ``previous branch syntax''
`@{-n}`. For example, `@{-1}` is a way to refer the last branch you
were on. This option should be used by porcelains to accept this
syntax anywhere a branch name is expected, so they can act as if you
--
2.15.0.rc1.287.g2b38de12cc
Re: [PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch
Jonathan Nieder writes: > Handles the nongit case in strbuf_check_branch_ref instead of > introducing a new check_branch_ref_format helper. I view that as a regression, actually. Don't we want a function that does not require a strbuf when asking a simple question: "I have a string, and I want to see if that is a valid name"?
[PATCH 2/3] check-ref-format --branch: strip refs/heads/ using skip_prefix
From: Junio C Hamano
The expansion returned from strbuf_check_branch_ref always starts with
"refs/heads/" by construction, but there is nothing about its name or
advertised API making that obvious. This command is used to process
human-supplied input from the command line and is usually not the
inner loop, so we can spare some cycles to be more defensive. Instead
of hard-coding the offset strlen("refs/heads/") to skip, verify that
the expansion actually starts with refs/heads/.
[jn: split out from a larger patch, added explanation]
Signed-off-by: Junio C Hamano
Signed-off-by: Jonathan Nieder
---
builtin/check-ref-format.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 6c40ff110b..bc67d3f0a8 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -39,12 +39,14 @@ static char *collapse_slashes(const char *refname)
static int check_ref_format_branch(const char *arg)
{
struct strbuf sb = STRBUF_INIT;
+ const char *name;
int nongit;
setup_git_directory_gently(&nongit);
- if (strbuf_check_branch_ref(&sb, arg))
+ if (strbuf_check_branch_ref(&sb, arg) ||
+ !skip_prefix(sb.buf, "refs/heads/", &name))
die("'%s' is not a valid branch name", arg);
- printf("%s\n", sb.buf + 11);
+ printf("%s\n", name);
strbuf_release(&sb);
return 0;
}
--
2.15.0.rc1.287.g2b38de12cc
[PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository
From: Junio C Hamano
Running "git check-ref-format --branch @{-1}" from outside any
repository produces
$ git check-ref-format --branch @{-1}
BUG: environment.c:182: git environment hasn't been setup
This is because the expansion of @{-1} must come from the HEAD reflog,
which involves opening the repository. @{u} and @{push} (which are
more unusual because they typically would not expand to a local
branch) trigger the same assertion.
This has been broken since day one. Before v2.13.0-rc0~48^2
(setup_git_env: avoid blind fall-back to ".git", 2016-10-02), the
breakage was more subtle: Git would read reflogs from ".git" within
the current directory even if it was not a valid repository. Usually
that is harmless because Git is not being run from the root directory
of an invalid repository, but in edge cases such accesses can be
confusing or harmful. Since v2.13.0, the problem is easier to
diagnose because Git aborts with a BUG message.
Erroring out is the right behavior: when asked to interpret a branch
name like "@{-1}", there is no reasonable answer in this context.
But we should print a message saying so instead of an assertion failure.
We do not forbid "check-ref-format --branch" from outside a repository
altogether because it is ok for a script to pre-process branch
arguments without @{...} in such a context. For example, with
pre-2.13 Git, a script that does
branch='master'; # default value
parse_options
branch=$(git check-ref-format --branch "$branch")
to normalize an optional branch name provided by the user would work
both inside a repository (where the user could provide '@{-1}') and
outside (where '@{-1}' should not be accepted).
So disable the "expand @{...}" half of the feature when run outside a
repository, but keep the check of the syntax of a proposed branch
name. This way, when run from outside a repository, "git
check-ref-format --branch @{-1}" will gracefully fail:
$ git check-ref-format --branch @{-1}
fatal: '@{-1}' is not a valid branch name
and "git check-ref-format --branch master" will succeed as before:
$ git check-ref-format --branch master
master
restoring the usual pre-2.13 behavior.
[jn: split out from a larger patch; moved conditional to
strbuf_check_branch_ref instead of its caller; fleshed out commit
message; some style tweaks in tests]
Reported-by: Marko Kungla
Helped-by: Jeff King
Signed-off-by: Junio C Hamano
Signed-off-by: Jonathan Nieder
---
sha1_name.c | 5 -
t/t1402-check-ref-format.sh | 16
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index c7c5ab376c..603e667faa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1331,7 +1331,10 @@ void strbuf_branchname(struct strbuf *sb, const char
*name, unsigned allowed)
int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
{
- strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ if (startup_info->have_repository)
+ strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+ else
+ strbuf_addstr(sb, name);
if (name[0] == '-')
return -1;
strbuf_splice(sb, 0, 0, "refs/heads/", 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60d..98e4a8613b 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -144,6 +144,11 @@ test_expect_success "check-ref-format --branch @{-1}" '
refname2=$(git check-ref-format --branch @{-2}) &&
test "$refname2" = master'
+test_expect_success 'check-ref-format --branch -naster' '
+ test_must_fail git check-ref-format --branch -naster >actual &&
+ test_must_be_empty actual
+'
+
test_expect_success 'check-ref-format --branch from subdir' '
mkdir subdir &&
@@ -161,6 +166,17 @@ test_expect_success 'check-ref-format --branch from
subdir' '
test "$refname" = "$sha1"
'
+test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
+ nongit test_must_fail git check-ref-format --branch @{-1} >actual &&
+ test_must_be_empty actual
+'
+
+test_expect_success 'check-ref-format --branch master from non-repo' '
+ echo master >expect &&
+ nongit git check-ref-format --branch master >actual &&
+ test_cmp expect actual
+'
+
valid_ref_normalized() {
prereq=
case $1 in
--
2.15.0.rc1.287.g2b38de12cc
[PATCH 0/3] Re: [PATCH] check-ref-format: require a repository for --branch
Hi,
Junio C Hamano wrote:
> Subject: [PATCH] check-ref-format: --branch cannot grok @{-1} outside a
> repository
How about this? It is written to be more conservative than the patch
I am replying to, but except for the commit message, it should be
pretty much equivalent.
[...]
> --- a/builtin/check-ref-format.c
> +++ b/builtin/check-ref-format.c
> @@ -38,13 +38,22 @@ static char *collapse_slashes(const char *refname)
>
> static int check_ref_format_branch(const char *arg)
> {
> + int nongit, malformed;
> struct strbuf sb = STRBUF_INIT;
> - int nongit;
> + const char *name = arg;
>
> setup_git_directory_gently(&nongit);
> - if (strbuf_check_branch_ref(&sb, arg))
> +
> + if (!nongit)
> + malformed = (strbuf_check_branch_ref(&sb, arg) ||
> + !skip_prefix(sb.buf, "refs/heads/", &name));
> + else
> + malformed = check_branch_ref_format(arg);
Handles the nongit case in strbuf_check_branch_ref instead of
introducing a new check_branch_ref_format helper.
[...]
> --- a/cache.h
> +++ b/cache.h
> @@ -1444,6 +1444,20 @@ extern int parse_oid_hex(const char *hex, struct
> object_id *oid, const char **en
> #define INTERPRET_BRANCH_HEAD (1<<2)
> extern int interpret_branch_name(const char *str, int len, struct strbuf *,
>unsigned allowed);
> +
> +/*
> + * NEEDSWORK: declare strbuf_branchname() and strbuf_check_branch_ref()
> + * here, not in strbuf.h
> + */
As a result, it doesn't touch headers. I agree that these functions
don't belong in strbuf.h (sorry for not updating the headers at the
same time I moved their implementations) but suspect e.g. branch.h,
revision.h, or some new header like revision-syntax.h would be a
better place.
[...]
> --- a/strbuf.h
> +++ b/strbuf.h
> @@ -568,6 +568,12 @@ static inline void strbuf_complete_line(struct strbuf
> *sb)
> strbuf_complete(sb, '\n');
> }
>
> +/*
> + * NEEDSWORK: the following two functions should not be in this file;
> + * these are about refnames, and should be declared next to
> + * interpret_branch_name() in cache.h
> + */
Didn't touch headers.
[...]
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -161,6 +161,18 @@ test_expect_success 'check-ref-format --branch from
> subdir' '
> test "$refname" = "$sha1"
> '
>
> +test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
> + test_must_fail nongit git check-ref-format --branch @{-1}
> +'
Swapped test_must_fail and nongit to match existing tests.
Junio C Hamano (3):
check-ref-format --branch: do not expand @{...} outside repository
check-ref-format --branch: strip refs/heads/ using skip_prefix
check-ref-format doc: --branch validates and expands
Documentation/git-check-ref-format.txt | 9 -
builtin/check-ref-format.c | 6 --
sha1_name.c| 5 -
t/t1402-check-ref-format.sh| 16
4 files changed, 32 insertions(+), 4 deletions(-)
Re: [PATCH] check-ref-format: require a repository for --branch
Jonathan Nieder writes:
> And in that spirit, I think the patch you replied with aims to go in
> the right direction, by providing the core functionality when in a
> repository while avoiding breaking such a script outside of one
> (though I do not understand it fully yet).
Given that, is it safe for me to ignore this earlier one
> For what it's worth, I don't agree with this repurposing of
> "check-ref-format --branch" at all.
as reacting to the patch without reading what it does?
>>(e.g. a wrapper to "git
>> clone" that wants to verify the name it is going to give to the "-b"
>> option), and a check desired in such a context is different from
>> (and is stricter than) feeding refs/heads/$name to the same command
>> without the "--branch" option.
>
> Can you say more about this example? E.g. why is this hypothetical
> wrapper unable to rely on "git clone -b"'s own error handling?
If I have to guess what you meant, perhaps the Porcelain wanted to
diagnose bad parameter that would be rejected _before_ letting clone
spend cycles and possibly network bandwidth?
But this was my way of rephrasing an earlier example you used while
objecting to Peff's change
For example, if you have a script with an $opt_branch variable
that defaults to "master", it may do
resolved_branch=$(git check-ref-format --branch "$opt_branch")
even though it is in a mode that not going to have to use
$resolved_branch and it is not running from a repository.
so my answer to the question in the message I am directly responding
to would be "You tell me." ;-)
> --symbolic-full-name seems like a good fit. Do you remember why
> check-ref-format was introduced instead?
It really serves two purposes, and at this point, I do not think it
really matters why it also does the @{-1} expansion as well as name
validation. 604e0cb5 ("Documentation: describe check-ref-format
--branch", 2009-10-12) happened 8 years ago, and since then it has
been advertised long enough as if the 80% primary purpose of "c-r-f
--branch" were to expand @{-1} to a branch name; even though the
text hints that it also does the usual checks, by definition
validation of the result of expanding @{-1} ought to succeed; after
all that was the branch we _were_ on ;-).
