* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Max Horn @ 2012-12-12 23:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, git
In-Reply-To: <7vwqwm27az.fsf@alter.siamese.dyndns.org>
On 13.12.2012, at 00:13, Junio C Hamano wrote:
> Max Horn <max@quendi.de> writes:
>
>> Of course I can also re-roll, if that is necessary/preferred.
>
> No, you can't. The topic has been cooking in 'next' for some days
> now already.
Ah, right, I somehow missed that :-/. Well, I guess it's at most a tiny minor cleanup anyway and thus not important :-).
Cheers,
Max
^ permalink raw reply
* Re: Weird problem with git-submodule.sh
From: Phil Hord @ 2012-12-12 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefano Lattarini, Marc Branchaud, Git Mailing List
In-Reply-To: <7vehiv3vjm.fsf@alter.siamese.dyndns.org>
On Wed, Dec 12, 2012 at 2:44 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Phil Hord <phil.hord@gmail.com> writes:
>> [2] https://bugs.launchpad.net/ubuntu/+source/dash/+bug/141481
>
> None of the ones listed seems to me a bug. Rather, I see it as a
> sign that the reporter does not know POSIX shell well and only
> learned his/her shell through bash.
You're probably right. I run into enough problems with 'dash' as
/bin/sh that I always have to disable it early after a new install.
In particular, an third-party embedded linux kernel build script fails
in cryptic ways with dash. But it is probably the third-party's poor
understanding of POSIX shell which is to blame.
I think git's 'make test' previously would also fail under dash, but
it seems to be happy with it atm.
Phil
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Junio C Hamano @ 2012-12-12 23:13 UTC (permalink / raw)
To: Max Horn; +Cc: Felipe Contreras, git
In-Reply-To: <A0E9390A-58CE-4E3E-A1A6-2D5CDB62FE06@quendi.de>
Max Horn <max@quendi.de> writes:
> Of course I can also re-roll, if that is necessary/preferred.
No, you can't. The topic has been cooking in 'next' for some days
now already.
^ permalink raw reply
* Re: [PATCH v2] submodule: add 'deinit' command
From: W. Trevor King @ 2012-12-12 23:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jens Lehmann, Michael J Gruber, Phil Hord, Git, Heiko Voigt,
Jeff King, Shawn Pearce, Nahor
In-Reply-To: <7vlid23nnc.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 958 bytes --]
On Wed, Dec 12, 2012 at 02:34:47PM -0800, Junio C Hamano wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
> > So unless people agree that deinit should also remove the work
> > tree I'll prepare some patches teaching all git commands to
> > consistently ignore deinitialized submodules. Opinions?
>
> While I agree that consistency is good, "deinit" that does not
> remove the working tree of the submodule the user explicitly said he
> no longer wants to have the checkout for is a bug, and I think these
> two are orthogonal issues.
Should `deinit` remove the submodule checkout, replace it with the
original gitlink, and clear the .git/config information then? That
would restore the user to the state they'd be in if they were never
interested in the submodule.
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [BUG] Hardcoded python and install on solaris
From: Junio C Hamano @ 2012-12-12 23:08 UTC (permalink / raw)
To: P Fudd; +Cc: git
In-Reply-To: <1870c3d3587281b436cddb33cca1e822.squirrel@www.pkts.ca>
"P Fudd" <pfudd@mailinator.com> writes:
> When compiling git-1.8.0.2 on a moderately old OpenIndiana machine, I had
> to install a few things (m4, autoconf, coreutils, xz, python).
>
> Even though I started the configuration fresh (make distclean; configure),
> the makefile still wanted to use /usr/bin/python (instead of
> /usr/local/bin/python) and /usr/usb/install (instead of
> /usr/local/bin/install).
You need to specify PYTHON_PATH on the build command line, something
like:
$ make PYTHON_PATH=/usr/local/bin/python
$ make PYTHON_PATH=/usr/local/bin/python install
We don't really rely on configure, and it sometimes is the case
where ./configure output does not know some knobs you can tweak in
the Makefile, but this one is not.
I think you can use --with-python=/usr/local/bin/python when running
the ./configure script. ./configure --help may tell you more about
what you can tweak.
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Max Horn @ 2012-12-12 23:05 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano
In-Reply-To: <CAMP44s1xetknwdOT5EseuASQE_2WFP1e1-Ao2RWYeya+EJ9SfQ@mail.gmail.com>
On 13.12.2012, at 00:00, Felipe Contreras wrote:
[...]
>>>
>>> I find all this text a bit confusing. First argument, second argument,
>>> etc. Personally, I would describe everything in the terms of alias
>>> (1st arg), and URL (2nd arg).
>>
>> Yeah, I also thought about that, but as above, deliberately did not touch it here, but only moved it around. I'll be happy to revisit this on a future date, though.
>
> Oh, in that case it's fine, but I would have named it "move invocation
> before input format", or something that has *move*, or *shuffle*.
Agreed. It explicit says move in the body of the commit message, but not in the summary line.. That would be an improvement, I gueess. Junio, if you want, feel free to reword the summary line of the patch accordingly, e.g. changing it from
git-remote-helpers.txt: document invocation before input format
to something like
git-remote-helpers.txt: move 'invocation' section before 'input format'
Of course I can also re-roll, if that is necessary/preferred.
Cheers,
Max
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Junio C Hamano @ 2012-12-12 23:03 UTC (permalink / raw)
To: Max Horn; +Cc: Felipe Contreras, git
In-Reply-To: <839EECE2-4459-4358-B7E8-5D64374A0540@quendi.de>
Max Horn <max@quendi.de> writes:
> Worth a thought yeah -- but beyond the scope of this patch: I
> merely moved this text around, but did not touch it otherwise.
> ...
> Yeah, I also thought about that, but as above, deliberately did
> not touch it here, but only moved it around. I'll be happy to
> revisit this on a future date, though.
That sounds like a sensible approach. So what's been cooking on
'next' is OK, it seems.
As this is merely a doc update, I may be tempted to merge it down to
the 'master' branch before the next -rc.
Thanks.
^ permalink raw reply
* Re: [PATCH v7 2/3] submodule update: add --remote for submodule's upstream changes
From: W. Trevor King @ 2012-12-12 23:02 UTC (permalink / raw)
To: Phil Hord
Cc: Git, Junio C Hamano, Heiko Voigt, Jeff King, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <CABURp0oLmSjiZAOJxEzwSmL+jimpVj8DcDi-odPTzCpCcyH8yA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4633 bytes --]
On Wed, Dec 12, 2012 at 12:43:23PM -0500, Phil Hord wrote:
> On Tue, Dec 11, 2012 at 1:58 PM, W. Trevor King <wking@tremily.us> wrote:
> > diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
> > …
> > +--remote::
> > [snip some --remote documentation]
> > +In order to ensure a current tracking branch state, `update --remote`
> > +fetches the submodule's remote repository before calculating the
> > +SHA-1. This makes `submodule update --remote --merge` similar to
> > +running `git pull` in the submodule. If you don't want to fetch (for
> > +something closer to `git merge`), you should use `submodule update
> > +--remote --no-fetch --merge`.
>
> I assume the same can be said for 'submodue update --remote --rebase',
> right?
Yes.
> I wonder if this can be made merge/rebase-agnostic. Is it still
> true if I word it like this?:
>
> In order to ensure a current tracking branch state, `update --remote`
> fetches the submodule's remote repository before calculating the
> SHA-1. If you don't want to fetch, you should use `submodule update
> --remote --no-fetch`.
Works for me. Will change in v8 (which I'll base on 'master').
> > diff --git a/git-submodule.sh b/git-submodule.sh
> > index f969f28..1395079 100755
> > --- a/git-submodule.sh
> > +++ b/git-submodule.sh
> > @@ -8,7 +8,8 @@ dashless=$(basename "$0" | sed -e 's/-/ /')
> > USAGE="[--quiet] add [-b branch] [-f|--force] [--reference <repository>] [--] <repository> [<path>]
> > or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]
> > or: $dashless [--quiet] init [--] [<path>...]
> > - or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> > + or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference <repository>] [--merge] [--recursive] [--] [<path>...]
> > +ges
>
> I think there's an unintentionally added line here with "ges".
That is embarrassing :p. Will fix in v8.
> > + if test -n "$remote"
> > + then
> > + if test -z "$nofetch"
> > + then
> > + # Fetch remote before determining tracking $sha1
> > + (clear_local_git_env; cd "$sm_path" && git-fetch) ||
>
> You should 'git fetch $remote_name' here, and of course, initialize
> remote_name before this. But how can we know the remote_name in the
> first place? Is it safe to assume the submodule remote names will
> match those in the superproject?
The other git-fetch call from git-submodule.sh is also bare (i.e. no
specified remote). When the remote needs to be specified, other
portions of git-submodule.sh use $(get_default_remote), which is (I
think) what the user should expect. v6 of this series had a
configurable remote name, but Junio wasn't keen on the additional
configuration option. I don't really mind either way.
>
> > + die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> > + fi
> > + remote_name=$(get_default_remote)
>
> This get_default_remote finds the remote for the remote-tracking
> branch for HEAD in the superproject. It is possible that HEAD !=
> $branch, so we have very few clues to go on here to get a more
> reasonable answer, so I do not have any good suggestions to improve
> this.
For detached HEADs, get_default_remote should fall back to 'origin',
which seems sane. If the user wants a different default, they've
likely checkout out a branch in the submodule, setup that branch's
remote, and will be using --merge or --rebase. If anyone expects
users who will be using detached heads to *want* to specify a
different remote than 'origin', that would be a good argument for
reinstating my submodule.<name>.remote patch from v6.
> > + sha1=$(clear_local_git_env; cd "$sm_path" &&
> > + git rev-parse --verify "${remote_name}/${branch}") ||
>
> This does assume the submodule remote names will match those in the
> superproject. Is this safe?
Another good catch. I should be calling get_default_remote after
cd-ing into the submodule. Will change in v8.
Thanks for the feedback :)
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Felipe Contreras @ 2012-12-12 23:00 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <839EECE2-4459-4358-B7E8-5D64374A0540@quendi.de>
On Wed, Dec 12, 2012 at 4:58 PM, Max Horn <max@quendi.de> wrote:
>
> On 12.12.2012, at 23:14, Felipe Contreras wrote:
>
>> On Tue, Nov 27, 2012 at 5:03 PM, Max Horn <max@quendi.de> wrote:
>>
>>> index 5ce4cda..9a7e583 100644
>>> --- a/Documentation/git-remote-helpers.txt
>>> +++ b/Documentation/git-remote-helpers.txt
>>> @@ -35,6 +35,37 @@ transport protocols, such as 'git-remote-http', 'git-remote-https',
>>> 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
>>> 'fetch', 'option', and 'push'.
>>>
>>> +INVOCATION
>>> +----------
>>> +
>>> +Remote helper programs are invoked with one or (optionally) two
>>> +arguments. The first argument specifies a remote repository as in git;
>>> +it is either the name of a configured remote or a URL. The second
>>> +argument specifies a URL; it is usually of the form
>>> +'<transport>://<address>', but any arbitrary string is possible.
>>> +The 'GIT_DIR' environment variable is set up for the remote helper
>>> +and can be used to determine where to store additional data or from
>>> +which directory to invoke auxiliary git commands.
>>> +
>>> +When git encounters a URL of the form '<transport>://<address>', where
>>> +'<transport>' is a protocol that it cannot handle natively, it
>>> +automatically invokes 'git remote-<transport>' with the full URL as
>>> +the second argument. If such a URL is encountered directly on the
>>> +command line, the first argument is the same as the second, and if it
>>> +is encountered in a configured remote, the first argument is the name
>>> +of that remote.
>>
>> Maybe it's worth mentioning that if the alias of the remote is not
>> specified, the URL is used instead.
>
> Worth a thought yeah -- but beyond the scope of this patch: I merely moved this text around, but did not touch it otherwise.
>
>>
>>> +A URL of the form '<transport>::<address>' explicitly instructs git to
>>> +invoke 'git remote-<transport>' with '<address>' as the second
>>> +argument. If such a URL is encountered directly on the command line,
>>> +the first argument is '<address>', and if it is encountered in a
>>> +configured remote, the first argument is the name of that remote.
>>> +
>>> +Additionally, when a configured remote has 'remote.<name>.vcs' set to
>>> +'<transport>', git explicitly invokes 'git remote-<transport>' with
>>> +'<name>' as the first argument. If set, the second argument is
>>> +'remote.<name>.url'; otherwise, the second argument is omitted.
>>
>> I find all this text a bit confusing. First argument, second argument,
>> etc. Personally, I would describe everything in the terms of alias
>> (1st arg), and URL (2nd arg).
>
> Yeah, I also thought about that, but as above, deliberately did not touch it here, but only moved it around. I'll be happy to revisit this on a future date, though.
Oh, in that case it's fine, but I would have named it "move invocation
before input format", or something that has *move*, or *shuffle*.
--
Felipe Contreras
^ permalink raw reply
* [BUG] Hardcoded python and install on solaris
From: P Fudd @ 2012-12-12 22:34 UTC (permalink / raw)
To: git
Hi;
When compiling git-1.8.0.2 on a moderately old OpenIndiana machine, I had
to install a few things (m4, autoconf, coreutils, xz, python).
Even though I started the configuration fresh (make distclean; configure),
the makefile still wanted to use /usr/bin/python (instead of
/usr/local/bin/python) and /usr/usb/install (instead of
/usr/local/bin/install).
Thanks for a great SCM!
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Max Horn @ 2012-12-12 22:58 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <CAMP44s3vO9b4-XxqatEc2w3KJLqLGgyjPuKpQkAXHQwTJJEQTg@mail.gmail.com>
On 12.12.2012, at 23:14, Felipe Contreras wrote:
> On Tue, Nov 27, 2012 at 5:03 PM, Max Horn <max@quendi.de> wrote:
>
>> index 5ce4cda..9a7e583 100644
>> --- a/Documentation/git-remote-helpers.txt
>> +++ b/Documentation/git-remote-helpers.txt
>> @@ -35,6 +35,37 @@ transport protocols, such as 'git-remote-http', 'git-remote-https',
>> 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
>> 'fetch', 'option', and 'push'.
>>
>> +INVOCATION
>> +----------
>> +
>> +Remote helper programs are invoked with one or (optionally) two
>> +arguments. The first argument specifies a remote repository as in git;
>> +it is either the name of a configured remote or a URL. The second
>> +argument specifies a URL; it is usually of the form
>> +'<transport>://<address>', but any arbitrary string is possible.
>> +The 'GIT_DIR' environment variable is set up for the remote helper
>> +and can be used to determine where to store additional data or from
>> +which directory to invoke auxiliary git commands.
>> +
>> +When git encounters a URL of the form '<transport>://<address>', where
>> +'<transport>' is a protocol that it cannot handle natively, it
>> +automatically invokes 'git remote-<transport>' with the full URL as
>> +the second argument. If such a URL is encountered directly on the
>> +command line, the first argument is the same as the second, and if it
>> +is encountered in a configured remote, the first argument is the name
>> +of that remote.
>
> Maybe it's worth mentioning that if the alias of the remote is not
> specified, the URL is used instead.
Worth a thought yeah -- but beyond the scope of this patch: I merely moved this text around, but did not touch it otherwise.
>
>> +A URL of the form '<transport>::<address>' explicitly instructs git to
>> +invoke 'git remote-<transport>' with '<address>' as the second
>> +argument. If such a URL is encountered directly on the command line,
>> +the first argument is '<address>', and if it is encountered in a
>> +configured remote, the first argument is the name of that remote.
>> +
>> +Additionally, when a configured remote has 'remote.<name>.vcs' set to
>> +'<transport>', git explicitly invokes 'git remote-<transport>' with
>> +'<name>' as the first argument. If set, the second argument is
>> +'remote.<name>.url'; otherwise, the second argument is omitted.
>
> I find all this text a bit confusing. First argument, second argument,
> etc. Personally, I would describe everything in the terms of alias
> (1st arg), and URL (2nd arg).
Yeah, I also thought about that, but as above, deliberately did not touch it here, but only moved it around. I'll be happy to revisit this on a future date, though.
Thanks for the feedback,
Max
^ permalink raw reply
* Re: [PATCH v7 0/3] submodule update: add --remote for submodule's upstream changes
From: W. Trevor King @ 2012-12-12 22:44 UTC (permalink / raw)
To: Junio C Hamano
Cc: Git, Heiko Voigt, Jeff King, Phil Hord, Shawn Pearce,
Jens Lehmann, Nahor
In-Reply-To: <7vzk1j3zgr.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 839 bytes --]
On Wed, Dec 12, 2012 at 10:19:32AM -0800, Junio C Hamano wrote:
> In any case, I ended up applying them by editing the patches, and I
> should have a good copy in 'pu'. Please double check the result.
Your 'pu' branch looks good to me. Most of the differences with my
initial patch are due to irrelevant context lines. I would change
patch 3 (commit 2f507f9a in 'pu') to use
git config -f .gitmodules submodule."$sm_name".branch "$branch"
^
instead of
git config -f .gitmodules submodule."$sm_path".branch "$branch"
^
to match the nearby changes from 73b0898d.
Trevor
--
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Junio C Hamano @ 2012-12-12 22:43 UTC (permalink / raw)
To: Andrew Ardill
Cc: Jeff King, Eric S. Raymond, Sitaram Chamarty, Patrick Donnelly,
Nguyen Thai Ngoc Duy, Michael Haggerty, Felipe Contreras,
git@vger.kernel.org
In-Reply-To: <CAH5451nVqnS0UFBVDW5=Xmaj_6geiw7D7J4mR7922U+074W2qQ@mail.gmail.com>
Andrew Ardill <andrew.ardill@gmail.com> writes:
> On 13 December 2012 04:49, Junio C Hamano <gitster@pobox.com> wrote:
>> "bisect" with "<used-to-be, now-is> vs
>> <good, bad>" issue unsettled
>
> Would you want to see this issue resolved in-script before a porting
> attempt was started?
Honestly, I do not care too much either way, but for the people who
want to work either on the rewrite-to-C or on the semantics issue,
it would be easier to manage it that way.
And that "issue resolved in-script" does not have to be "implemented
in-script". The resolution could be to declare that it is not worth
it and a promise to call the two states <good, bad> and with no
other names. It would give a semantics for the rewriters-to-C can
start working on that is stable enough ;-).
^ permalink raw reply
* Re: Fwd: possible Improving diff algoritm
From: Andrew Ardill @ 2012-12-12 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Morten Welinder, Kevin, git
In-Reply-To: <7vpq2f2az4.fsf@alter.siamese.dyndns.org>
On 13 December 2012 08:53, Junio C Hamano <gitster@pobox.com> wrote:
> The output being "a correct patch" is not the only thing we need to
> consider, though, as I mentioned in another response to Kevin
> regarding the "consequences".
The main benefit of picking a more 'natural' diff is a usability one.
I know that when a chunk begins and ends one line after the logical
break point (typically with braces in my experience) mentally parsing
the diff becomes significantly harder. If there was a way to teach git
where it should try and break out a chunk (potentially per filetype?)
this is a good thing for readability, and I think would outweigh any
temporary pain with regards to cached rerere and diff data.
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCH v2] submodule: add 'deinit' command
From: Junio C Hamano @ 2012-12-12 22:34 UTC (permalink / raw)
To: Jens Lehmann
Cc: Michael J Gruber, Phil Hord, W. Trevor King, Git, Heiko Voigt,
Jeff King, Shawn Pearce, Nahor
In-Reply-To: <50C90469.8080303@web.de>
Jens Lehmann <Jens.Lehmann@web.de> writes:
> So unless people agree that deinit should also remove the work
> tree I'll prepare some patches teaching all git commands to
> consistently ignore deinitialized submodules. Opinions?
While I agree that consistency is good, "deinit" that does not
remove the working tree of the submodule the user explicitly said he
no longer wants to have the checkout for is a bug, and I think these
two are orthogonal issues.
In other words, "Ignore deinitialized submodules even when an
earlier bug in deinit failed to remove the working tree" is a
robustness issue for the other recursing commands, not an excuse for
"deinit" to have such a bug in the first place, I think.
^ permalink raw reply
* Re: [PATCH v2] If `egrep` is aliased, temporary disable it in bash.completion
From: Felipe Contreras @ 2012-12-12 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Adam Tkac, git, SZEDER Gábor
In-Reply-To: <7v4njzjbzo.fsf@alter.siamese.dyndns.org>
On Thu, Dec 6, 2012 at 12:01 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Adam Tkac <atkac@redhat.com> writes:
>
>> On Thu, Nov 29, 2012 at 09:33:53AM -0800, Junio C Hamano wrote:
>> ...
>>> IOW, something along this line?
>>
>> This won't work, unfortunately, because shopt settings aren't inherited by
>> subshell (and for example egrep is called in subshell).
>>
>> I discussed this issue with colleagues and we found basically two "fixes":
>>
>> 1. Tell people "do not use aliases which breaks completion script"
>> 2. Prefix all commands with "command", i.e. `command egrep` etc.
>>
>> In my opinion "2." is better long time solution, what do you think?
>
> Judging from what is in /etc/bash_completion.d/ (I am on Debian), I
> think that others are divided. Many but not all prefix "command" in
> front of "grep", but nobody does the same for "egrep", "cut", "tr",
> "sed", etc.
>
> If it were up to me, I would say we pick #1, but I cc'ed the people
> who have been more involved in our bash-completion code because they
> are in a better position to argue between the two than I am.
Why not both? I do prefer #1, but I don't see why we wouldn't prefix
some commonly problematic ones (\egrep), prefixing all of them seems
overkill for me.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] completion: add option --recurse-submodules to "git push"
From: Felipe Contreras @ 2012-12-12 22:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Jaeckel, git, Heiko Voigt
In-Reply-To: <7vehj1ixr6.fsf@alter.siamese.dyndns.org>
On Fri, Dec 7, 2012 at 11:21 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Steffen Jaeckel <steffen.jaeckel@stzedn.de> writes:
>
>> Signed-off-by: Steffen Jaeckel <steffen.jaeckel@stzedn.de>
>> ---
>> contrib/completion/git-completion.bash | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 0b77eb1..5b4d2e1 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -1434,6 +1434,10 @@ _git_pull ()
>> __git_complete_remote_or_refspec
>> }
>>
>> +__git_push_recurse_submodules_options="
>> + check on-demand
>> +"
>
> Most of the existing completion functions do not seem to define
> separate variables like this; instead, they literally embed their
> choices at the point of use.
>
> Is it expected that the same set of choices will appear in the
> completion of many other subcommand options? [jc: Cc'ed Heiko so
> that we can sanity check the answer to this question]. If so, the
> variable may be justified; otherwise, not.
>
>> _git_push ()
>> {
>> case "$prev" in
>> @@ -1446,10 +1450,15 @@ _git_push ()
>> __gitcomp_nl "$(__git_remotes)" "" "${cur##--repo=}"
>> return
>> ;;
>> + --recurse-submodules=*)
>> + __gitcomp "$__git_push_recurse_submodules_options" "" "${cur##--recurse-submodules=}"
>> + return
>> + ;;
>
> Owners of the completion script, does this look reasonable?
> [jc: Cc'ed Felipe for this]
>
> This is a tangent, but why is it a double-hash "##" not a
> single-hash "#", other than "because all others use ##"?
Seems OK by me, but I agree, the options should be inline.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH v2] submodule: add 'deinit' command
From: Jens Lehmann @ 2012-12-12 22:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: Michael J Gruber, Phil Hord, W. Trevor King, Git, Heiko Voigt,
Jeff King, Shawn Pearce, Nahor
In-Reply-To: <7vr4mv3w2x.fsf@alter.siamese.dyndns.org>
Am 12.12.2012 20:32, schrieb Junio C Hamano:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> Especially as I suspect the number of submodule users having
>> customized those in .git/config is not that high ...
>
> I thought the point of "deinit" was to say "I am not interested in
> having a checkout of these submodules in my working tree anymore".
Yes. (But I'm not sure users expect that command to also remove
the work tree)
> The user could do "rm -fr submodule && mkdir submodule" to remove it
> locally and keep "diff" and "status" from noticing the removal, but
> the primary reason the user needs an explicit "deinit" is because
> many subcommands of "git submodule" are documented to operate on all
> submodules that have been "init"ed when given no explicit submodule
> names [*1*].
The real reason we need deinit is that the next run of "submodule
update" will otherwise happily recreate the submodule checkout you
just removed as long as it finds the url setting in .git/config.
> Your "deinit" is documented not to actually remove the submodule
> checkout, but that very much goes against my intuition. What is the
> justification behind that choice?
I thought it should match what "submodule init" does, which is to do
nothing to the work tree until the next "submodule update" is run.
(But I agree that analogy is somewhat flawed until we teach "update"
to remove a deinitialized submodule - or maybe teach it the --deinit
option which could do both). On the other hand with current git
submodule work trees always stay around anyway until you remove them
by hand (e.g. when you switch to a branch that doesn't have it), so
I'm not sure what would surprise people more here. So I just left
the work tree unchanged.
> "We'll remove the configuration,
> you remove the tree yourself" will invite the mistake of running
> "git rm" on it, which you wanted to avoid with the addition to the
> "git rm" documentation, no?
I think that'll happen only if git would remind them that they
still have a populated work tree, which I believe it shouldn't.
> [Footnote]
>
> *1* In reality, the code looks at presense of .git in the submodule
> path to decide if it has been "init"ed (cf. cmd_update), but this
> implementation of "deinit" does not seem to cause that .git to be
> removed, leaving the submodule in "init"ed state from these other
> command's perspective.
Nope, cmd_update() checks first if the url is found in .git/config
and skips the submodule if not. I rechecked and only "summary" and
"foreach" still recurse into a deinitialized submodule, which they
shouldn't. But a quick test shows that "git status" and "git diff"
also still inspect a deinitialized submodule, so there's some work
left to do to handle the case where the work tree is not removed.
So unless people agree that deinit should also remove the work
tree I'll prepare some patches teaching all git commands to
consistently ignore deinitialized submodules. Opinions?
^ permalink raw reply
* Re: [PATCH 0/6] Improve remote helper documentation
From: Felipe Contreras @ 2012-12-12 22:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Max Horn, git, Sverre Rabbelier, Florian Achleitner
In-Reply-To: <7v1uf1he6m.fsf@alter.siamese.dyndns.org>
On Fri, Dec 7, 2012 at 1:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Max Horn <max@quendi.de> writes:
>
>> Various remote helper capabilities and commands were not
>> documented, in particular 'export', or documented in a misleading
>> way (e.g. 'for-push' was listed as a ref attribute understood by
>> git, which is not the case). This patch series changes that, and
>> also address some other things in the remote helper documentation
>> that I found jarring when reading through it.
>>
>> Note that the description of export and (im|ex)port-marks probably can be
>> improved, and I hope that somebody who knows more about them
>> than me and/or is better at writing documentation will do just that.
>> But I felt it was better to provide something than to do nothing
>> and only complain, as I did previously on this subject ;-).
>
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments. I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
>
> Except for a minor nit in 6/6; I think "defined options" should be
> "defined attributes".
I think the wording in 1/6 can be improved. Other than that I don't
have any comments, what I'm familiar with looks good to me.
--
Felipe Contreras
^ permalink raw reply
* Re: Python extension commands in git - request for policy change
From: Andrew Ardill @ 2012-12-12 22:21 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Eric S. Raymond, Sitaram Chamarty, Patrick Donnelly,
Nguyen Thai Ngoc Duy, Michael Haggerty, Felipe Contreras,
git@vger.kernel.org
In-Reply-To: <7vpq2f5ffu.fsf@alter.siamese.dyndns.org>
On 13 December 2012 04:49, Junio C Hamano <gitster@pobox.com> wrote:
> "bisect" with "<used-to-be, now-is> vs
> <good, bad>" issue unsettled
Would you want to see this issue resolved in-script before a porting
attempt was started?
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCH v2 1/6] git-remote-helpers.txt: document invocation before input format
From: Felipe Contreras @ 2012-12-12 22:14 UTC (permalink / raw)
To: Max Horn; +Cc: git
In-Reply-To: <1354057407-83151-2-git-send-email-max@quendi.de>
On Tue, Nov 27, 2012 at 5:03 PM, Max Horn <max@quendi.de> wrote:
> index 5ce4cda..9a7e583 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -35,6 +35,37 @@ transport protocols, such as 'git-remote-http', 'git-remote-https',
> 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
> 'fetch', 'option', and 'push'.
>
> +INVOCATION
> +----------
> +
> +Remote helper programs are invoked with one or (optionally) two
> +arguments. The first argument specifies a remote repository as in git;
> +it is either the name of a configured remote or a URL. The second
> +argument specifies a URL; it is usually of the form
> +'<transport>://<address>', but any arbitrary string is possible.
> +The 'GIT_DIR' environment variable is set up for the remote helper
> +and can be used to determine where to store additional data or from
> +which directory to invoke auxiliary git commands.
> +
> +When git encounters a URL of the form '<transport>://<address>', where
> +'<transport>' is a protocol that it cannot handle natively, it
> +automatically invokes 'git remote-<transport>' with the full URL as
> +the second argument. If such a URL is encountered directly on the
> +command line, the first argument is the same as the second, and if it
> +is encountered in a configured remote, the first argument is the name
> +of that remote.
Maybe it's worth mentioning that if the alias of the remote is not
specified, the URL is used instead.
> +A URL of the form '<transport>::<address>' explicitly instructs git to
> +invoke 'git remote-<transport>' with '<address>' as the second
> +argument. If such a URL is encountered directly on the command line,
> +the first argument is '<address>', and if it is encountered in a
> +configured remote, the first argument is the name of that remote.
> +
> +Additionally, when a configured remote has 'remote.<name>.vcs' set to
> +'<transport>', git explicitly invokes 'git remote-<transport>' with
> +'<name>' as the first argument. If set, the second argument is
> +'remote.<name>.url'; otherwise, the second argument is omitted.
I find all this text a bit confusing. First argument, second argument,
etc. Personally, I would describe everything in the terms of alias
(1st arg), and URL (2nd arg).
--
Felipe Contreras
^ permalink raw reply
* Re: Fwd: possible Improving diff algoritm
From: Junio C Hamano @ 2012-12-12 21:53 UTC (permalink / raw)
To: Morten Welinder; +Cc: Kevin, git
In-Reply-To: <CANv4PNm45xGBn2veKi1o0wB4K9NgsbtCsiymHNO4xbCDpJ5tDg@mail.gmail.com>
Morten Welinder <mwelinder@gmail.com> writes:
> Is there a reason why picking among the choices in a sliding window
> must be contents neutral?
Sorry, you might be getting at something interesting but I do not
understand the question. I have no idea what you mean by "contents
neutral".
Picking between these two choices
/** + /**
+ * Default parent + * Default parent
+ * + *
+ * @var int + * @var int
+ * @access protected + * @access protected
+ * @index + * @index
+ */ + */
+ protected $defaultParent; + protected $defaultParent;
+ +
+ /** /**
would not affect the correctness of the patch. You may pick
whatever you deem the most desirable, but your answer must be a
correct patch (the definition of "correct" here is "applying that
patch to the preimage produces the intended postimage").
And I think if you inserted a block of text B after a context C
where the tail of B matches the tail of C like the above, you can
shift what you treat as "inserted" up and still come up with a
correct patch.
The output being "a correct patch" is not the only thing we need to
consider, though, as I mentioned in another response to Kevin
regarding the "consequences".
^ permalink raw reply
* Re: Fwd: possible Improving diff algoritm
From: Morten Welinder @ 2012-12-12 21:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin, git
In-Reply-To: <7vvcc73yzh.fsf@alter.siamese.dyndns.org>
> So I think with s/Regularly/About half the time/, your observation
> above is correct.
>
> I think the reason you perceived this as "Regularly" is that you do
> not notice nor appreciate it when things go right (half the time),
> but you tend to notice and remember only when a wrong side happened
> to have been picked (the other half).
Is there a reason why picking among the choices in a sliding window
must be contents neutral?
I see these "illogical" (== stylistically not matching user intent) diffs
quite often. C comments (as in the example given) and #ifdef blocks
are typical cases.
Purely anecdotically, I have seen more trouble applying "illogical"
diffs than I would have expected from the corresponding "logical" diffs.
Morten
^ permalink raw reply
* Re: Fwd: possible Improving diff algoritm
From: Junio C Hamano @ 2012-12-12 20:29 UTC (permalink / raw)
To: Kevin; +Cc: git
In-Reply-To: <7vvcc73yzh.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Kevin <ikke@ikke.info> writes:
>
>> Regularly I notice that the diffs that are provided (through diff, or
>> add -p) tend to disconnect changes that belong to each other and
>> report lines being changed that are not changed.
>>
>> An example for this is:
>>
>> /**
>> + * Default parent
>> + *
>> + * @var int
>> + * @access protected
>> + * @index
>> + */
>> + protected $defaultParent;
>> +
>> + /**
>>
>> I understand this is a valid view of what is changed, but not a very
>> logical view from the point of the user.
>>
>> I wondered if there is a way to improve this, or would that have other
>> consequences.
I forgot to mention consequences. Changing it obviously changes the
shape of the diff, hence changes the patch id. Anything that caches
output from "git cherry" to match up "identical patches" will need
to discard and repopulate its cache. Your "rerere" database will go
stale.
Also "kup" tool used at k.org allows an uploader to pretend to
upload an incremental diff between two known commits by only sending
the GPG signature of the diff the uploader generates. The actual
diff is generated on the k.org machine locally and deposited next to
the GPG signature file, with the expectation that the signture
matches the diff. Changing the output from diff between two
versions will break the optimization and force the uploader to
upload the diff over the wire.
^ permalink raw reply
* Re: git-prompt.sh vs leading white space in __git_ps1()::printf_format
From: Simon Oosthoek @ 2012-12-12 20:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Piotr Krukowiecki, git
In-Reply-To: <7vlid35fe4.fsf@alter.siamese.dyndns.org>
On 12/12/12 18:50, Junio C Hamano wrote:
> Simon Oosthoek <s.oosthoek@xs4all.nl> writes:
>
>> This removes most of the ambiguities :-)
>> Ack from me!
>
> OK, as this is a low-impact finishing touch for a new feature, I'll
> fast-track this to 'master' before the final release.
>
Ok, wonderful!
BTW, I tried the thing I mentioned and it was safe to do:
PS1='blabla$(__git_ps1 x y)blabla'
will not eat your prompt, although I'd recommend putting something
useful instead of blabla ;-)
Cheers
Simon
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox