* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-05 13:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vmwvjzkcs.fsf@alter.siamese.dyndns.org>
On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> - do you want to support backslashed newlines?
JCH> What for? netrc/authinfo is not a line oriented file format at all,
JCH> and
JCH> machine k.org
JCH> login me
JCH> password mysecret
JCH> is a single entry; you do not need backslash at the end of any line.
Hmm. The parser I implemented only does single-line parsing, and I
misunderstood the format to be single-line (partly because I have never
seen anyone using the multi-line format you show). Looking at
Net::Netrc more carefully, it seems that the "machine" token is what
defines an entry, so a new entry starts with a new line that contains a
"machine" token. Is that acceptable and does it match your
understanding of the format? It matches Net::Netrc, at least.
I'll add this change to PATCHv4 with the assumption you agree.
>> - should all die() calls just print to STDERR and exit(0)?
JCH> Where "when unhandled, the helper should silently exit with 0" is
JCH> expected by the invoker, we shouldn't say anything to error stream,
JCH> and exit with zero. Please leave a comment to make it easy to
JCH> understand to the readers that is what is going on there.
JCH> If on the other hand it diagnosed an error (not a bug in the
JCH> implementation but a misconfiguration on the user's side), I _think_
JCH> it should loudly die() so that the user can notice and take
JCH> corrective action.
OK, I'll review these for PATCHv4 (also see below). Thanks.
>> - do you want to support multiple netrc files, as you and Peff suggested?
JCH> I didn't even suggest such thing IIRC---I expected it to iterate
JCH> from the most desirable (.authinfo.gpg) to the least (.netrc) and
JCH> stop at the first found one. There may be use cases people use more
JCH> than one and expect an entry to be found in any file, but I suspect
JCH> that might be more confusing than it is worth. But I do not care
JCH> very deeply myself either way.
After thinking about it, I agree with Peff multiple sources make sense
and will simplify the code flow (especially the default case, which
won't need to be handled separately). And the functionality doesn't
have to be confusing with the right debugging messages. So I'll add
them in PATCHv4.
The debugging messages will be fewer and simpler with this approach,
which makes it feel like the right track :)
Thanks
Ted
^ permalink raw reply
* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Ævar Arnfjörð Bjarmason @ 2013-02-05 14:27 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Michael J Gruber, git
In-Reply-To: <20130205112249.GE24973@sigill.intra.peff.net>
On Tue, Feb 5, 2013 at 12:22 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Feb 05, 2013 at 11:16:52AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> On Mon, Feb 4, 2013 at 5:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > "git log/diff-files -U8" do not need "-p" to enable textual patches,
>> > for example. It is "I already told you that I want 8-line context.
>> > For what else, other than showing textual diff, do you think I told
>> > you that for?" and replacing "8-line context" with various other
>> > options that affect patch generation will give us a variety of end
>> > user complaints that would tell us that C) is more intuitive to
>> > them.
>>
>> On a related note I think "--full-diff" should imply "-p" too.
>
> I don't think that is in the same class. --full-diff is quite useful for
> many other diff formats. E.g.:
>
> git log --full-diff --raw Makefile
>
> If you are proposing to default to "-p" when "--full-diff" is used but
> no format is given, that is a slightly different thing. The --full-diff
> in such a case is indeed useless, but I do not think it necessarily
> follows that "-p" was what the user wanted.
You're right. I didn't notice that it could work with other things besides -p.
On a related note then, it's a bit confusing that it's called
"--full-diff" when it doesn't actually show a diff.
^ permalink raw reply
* Re: Feature request: Allow extracting revisions into directories
From: Phil Hord @ 2013-02-05 15:11 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Robert Clausecker, git@vger.kernel.org
In-Reply-To: <510F03FC.6080501@gmail.com>
On Sun, Feb 3, 2013 at 7:42 PM, Sitaram Chamarty <sitaramc@gmail.com> wrote:
> On 02/03/2013 11:41 PM, Robert Clausecker wrote:
>>
>> Am Sonntag, den 03.02.2013, 21:55 +0530 schrieb Sitaram Chamarty:
>>> Could you help me understand why piping it to tar (actually 'tar -C
>>> /dest/dir -x') is not sufficient to achieve what you want?
>>
>> Piping the output of git archive into tar is of course a possible
>> solution; I just don't like the fact that you need to pipe the output
>> into a separate program to do something that should be possible with a
>> simple switch and not an extra command. It feels unintuitive and like a
>> workaround to make an archive just to unpack it on-the-fly. Also, adding
>> such a command (or at least documenting the way to do this using a pipe
>> to tar somewhere in the man pages) is a small and simple change that
>> improves usability.
>
> I realise it appears to be the fashion these days to get away from the
> Unix philosophy of having different tools do different things and
> combining them as needed.
>
> Ignoring the option-heavy GNU, and looking at the more traditional BSD
> tar manpage [1], I notice the following flags that could still be
> potentially needed by someone running 'git archive': '-t' (instead of
> '-x'), '-C dir', '--exclude/include', '-k', '-m', '--numeric-owner', -o,
> -P, -p, -q, -s, -T, -U, -v, -w, and -X.
OP did not ask about tar so I do not see why any of these tar options
are relevant. It seems like what he really wants is 'git archive
--format=native' , maybe. You can almost create this option by
saying
git config tar.native.command "tar -x"
except that you do not get the opportunity to specify a target directory.
But maybe he really wants a form of 'git checkout' instead.
> And I'm ignoring the esoteric ones like "--chroot" and "-S" (sparse mode).
>
> How many of these options would you like included in git? And if you
> say "I don't need any of those; I just need '-x'", that's not relevant.
> Someone else may need any or all of those flags, and if you accept "-x"
> you have to accept some of the others too.
This is only true if you cannot stop yourself from thinking about
'tar'. What about zip, for example?
I think none of these options is relevant.
> Also, I often want to deploy to a different host, and I might do that
> like so:
>
> git archive ... | ssh host tar -C /deploy/dir -x
>
> Why not put that ssh functionality into git also?
This slippery-slope argument is growing tiresome.
Phil
p.s. Conceded: OP set off this avalanche by disparaging the vaunted
PIPE operation.
^ permalink raw reply
* Re: [PATCH 11/13] contrib/subtree: Make each test self-contained
From: Junio C Hamano @ 2013-02-05 15:18 UTC (permalink / raw)
To: greened
Cc: James Nylen, git, Techlive Zheng, Wayne Walter, Avery Pennarun,
Jakub Suder, John Yani, David A. Greene
In-Reply-To: <87d2wfrnbe.fsf@waller.obbligato.org>
greened@obbligato.org writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> "David A. Greene" <greened@obbligato.org> writes:
>>
>>> +test_create_commit() (
>>> + repo=$1
>>> + commit=$2
>>> + cd "$repo"
>>> + mkdir -p "$(dirname "$commit")"
>>> + echo "$commit" > "$commit"
>>
>> Style.
>
> I need a little more explanation. :) Is there a style guide somewhere?
Documentation/CodingGuidelines?
>
>>> + git add "$commit"
>>> + git commit -m "$commit"
>>> +)
>>
>> Very nice, but don't we want to check for possible errors in any of
>> the above commands?
>
> I'll fix that. :)
> ...
> Ok. I'll rework this.
Thanks.
I also think it would be a good idea for you to learn to push back
to the original authors; fixing problems in patches by others, while
is a good way to learn how their thinking process went, is not
necessarily fun.
^ permalink raw reply
* Re: [PATCH v3 3/8] upload/receive-pack: allow hiding ref hierarchies
From: Junio C Hamano @ 2013-02-05 15:45 UTC (permalink / raw)
To: Jeff King; +Cc: git, Shawn Pearce
In-Reply-To: <20130205085047.GA24973@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 30, 2013 at 10:45:37AM -0800, Junio C Hamano wrote:
>
>> Teach upload-pack and receive-pack to omit some refs from their
>> initial advertisements by paying attention to the transfer.hiderefs
>> multi-valued configuration variable. Any ref that is under the
>> hierarchies listed on the value of this variable is excluded from
>> responses to requests made by "ls-remote", "fetch", "clone", "push",
>> etc.
>>
>> A typical use case may be
>>
>> [transfer]
>> hiderefs = refs/pull
>>
>> to hide the refs that are internally used by the hosting site and
>> should not be exposed over the network.
>
> In the earlier review, I mentioned making this per-service, but I see
> that is not the case here. Do you have an argument against doing so?
Perhaps then I misunderstood your intention. By reminding me of the
receive-pack side, I thought you were hinting to unify these two
into one, which I did. There is no argument against it.
> And I
> have not seen complaints about the current system.
Immediately after I added github to the set of places I push into,
which I think is long before you joined GitHub, I noticed that _my_
repository gets contaminated by second rate commits called pull
requests, and I may even have complained, but most likely I didn't,
as I could easily tell that, even though I know it is _not_ the only
way, nor even the best way [*1*], to implement the GitHub's pull
request workflow, I perfectly well understood that it would be the
most expedient way for GitHub folks to implement this feature.
I think you should take lack of complaints with a huge grain of
salt. It does not suggest much.
> Gerrit's refs/changes may be a different story, if they have a large
> enough number of them to make upload-pack's ref advertisement
> overwhelming.
This is probably a stale count, but platform/frameworks/base part of
AOSP has 3200+ refs; the corresponding repository internal to Google
has 60k+ refs (this is because there are many in-between states
recorded in the internal repository, even though the end result
published to the open source repository may be the same) and results
in ~4MB advertisement. Which is fairly significant when all you are
interested in doing is an "Am I up to date?" poll.
[Footnote]
*1* From the ownership point of view, objects that are only
reachable from these refs/pull/* refs do *not* belong to the
requestee, until the requestee chooses to accept the changes.
A malicious requestor can fork your repository, add an objectionable
blob to it, and throw a pull request at you. GitHub shows that the
blob now belongs to your repository, so the requestor turns around
and file a DMCA takedown complaint against your repository. A
clueful judge would then agree with the complaint after running a
"clone --mirror" and seeing the blob in your repository. Oops?
A funny thing is that you cannot "push :refs/pull/1/head" to remove
it anymore (I think in the early days, I took them out by doing this
a few times, but I may be misremembering), so you cannot make
yourself into compliance, even though you are not the offending
party. Your repository is held responsible for whatever the rogue
requestor added. That is not very nice, is it?
In an ideal world, I would have chosen to create a dedicated fork
managed by the hosting company (i.e. GitHub) for your repository
whose only purpose is to house these refs/pull/ refs (the hosting
site is ultimately who has to respond to DMCA notices anyway, and an
arrangement like this makes it clear who is reponsible for what).
The e-mail sent to you to let you know about outstanding pull
requests and the web UI could just point at that forked repository,
not your own (you also could choose to leave the outging pull
requests in the requestor's repository, but that is only OK if you
do not worry about (1) a requestor sending a pull request, then
updating the branch the pull request talks about later, to trick you
with bait-and-switch, or (2) a requestor sending a pull request,
thinks he is done with the topic and removes the repository).
^ permalink raw reply
* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Junio C Hamano @ 2013-02-05 15:46 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, git
In-Reply-To: <20130205093307.GC24973@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> git log --cc --stat
>
> the "--cc" is significant. So I don't think it is right for "--cc" to
> always imply "-p". But if the rule kicked in only when no other format
> had been specified, that might make sense.
Yes, it was my mistake that I left it unsaid. Obviously the
description of the "minor irritation" must be qualified with "unless
no other output options like --raw, --stat, etc. are given".
Thanks.
^ permalink raw reply
* Re: [RFC] Should "log --cc" imply "log --cc -p"?
From: Junio C Hamano @ 2013-02-05 15:51 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Jeff King, Michael J Gruber, git
In-Reply-To: <CACBZZX7JNwZYHn--Cp4g1rLPratJ9nEa-Q8HsYk65jjDYAkFqA@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> On a related note then, it's a bit confusing that it's called
> "--full-diff" when it doesn't actually show a diff.
It is too late to change the name of the option, but we could add a
synonym if it makes easier to understand what the option is for. It
is about saying "my pathspec are only to limit the commits to be
shown and do not limit the diff output". In that sense, the current
name that asks to give me the diff output fully is not that wrong,
but I wouldn't be surprised if other people can come up with better
names.
^ permalink raw reply
* Re: [PATCH v3 7/8] fetch: fetch objects by their exact SHA-1 object names
From: Junio C Hamano @ 2013-02-05 15:55 UTC (permalink / raw)
To: Jeff King; +Cc: git, Shawn Pearce
In-Reply-To: <20130205091938.GB24973@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Jan 30, 2013 at 10:45:41AM -0800, Junio C Hamano wrote:
>
>> Teach "git fetch" to accept an exact SHA-1 object name the user may
>> obtain out of band on the LHS of a pathspec, and send it on a "want"
>> message when the server side advertises the allow-tip-sha1-in-want
>> capability.
>
> Hmm. The UI on this is a little less nice than I would have hoped.
Naming with unadvertised *refname*, not object name, needs protocol
extension for the serving side to expand the name to object name;
otherwise the receiving end wouldn't know what tip what it asked
resulted in.
And that belongs to a separate "expand refs" extension (aka
"delaying ref advertisement") that is outside the scope of this
series but can be built on top, as I said in the cover letter.
^ permalink raw reply
* Re: Is anyone working on a next-gen Git protocol (Re: [PATCH v3 0/8] Hiding refs)
From: Junio C Hamano @ 2013-02-05 16:03 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Jeff King, Shawn Pearce, Johannes Schindelin
In-Reply-To: <CACBZZX7G=V9L+zEA2MQFjQW8tRA0i1Gk3nUYdg5dDy35v74SEw@mail.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Do you have any plans for something that *does* have the reduction of
> network bandwidth as a primary goal?
Uncluttering gives reduction of bandwidth anyway, so I do not see
much point in the distinction you seem to be making.
> Is this what you've been working on? Because if so I misunderstood you
> thinking you were going to work on something that gave clients the
> ability specify what they wanted before the initial ref advertisement.
> ...
> 4. http://thread.gmane.org/gmane.comp.version-control.git/207190
"Who speaks first" mentioned in 4. above, was primarily about
"delaying ref advertisement", which would be a larger protocol
change. Nobody seems to have attacked it since it was discussed,
and I was tired of hearing nothing but complaints and whines. This
"hiding refs" series was done as a cheaper way to solve a related
issue, without having to wait for the solution of "delaying
advertisement", which is an orthogonal issue.
^ permalink raw reply
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05 16:07 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87sj5ariar.fsf@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> On Mon, 04 Feb 2013 16:15:47 -0800 Junio C Hamano <gitster@pobox.com> wrote:
>
> JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>
>>> - do you want to support backslashed newlines?
>
> JCH> What for? netrc/authinfo is not a line oriented file format at all,
> JCH> and
>
> JCH> machine k.org
> JCH> login me
> JCH> password mysecret
>
> JCH> is a single entry; you do not need backslash at the end of any line.
>
> Hmm. The parser I implemented only does single-line parsing, and I
> misunderstood the format to be single-line (partly because I have never
> seen anyone using the multi-line format you show). Looking at
> Net::Netrc more carefully, it seems that the "machine" token is what
> defines an entry, so a new entry starts with a new line that contains a
> "machine" token. Is that acceptable and does it match your
> understanding of the format? It matches Net::Netrc, at least.
I thought I've given a more concrete outline than "I'll read
Net::Netrc and do whatever I think it does" in a separate message.
It would be better to read "man netrc" carefully at least once ;-)
^ permalink raw reply
* Re: [PATCH v3 2/8] git_remote_helpers: fix input when running under Python 3
From: Erik Faye-Lund @ 2013-02-05 16:07 UTC (permalink / raw)
To: Brandon Casey
Cc: Junio C Hamano, Sverre Rabbelier, Dennis Kaarsemaker,
John Keeping, Git List
In-Reply-To: <CA+sFfMf2R6+qzrLR9rwhtcM=ABZ8aWUJw-3riF98B3XWVGm54w@mail.gmail.com>
On Fri, Jan 25, 2013 at 9:23 PM, Brandon Casey <drafnel@gmail.com> wrote:
> On Wed, Jan 23, 2013 at 12:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Sverre Rabbelier <srabbelier@gmail.com> writes:
>>
>>> On Wed, Jan 23, 2013 at 11:47 AM, John Keeping <john@keeping.me.uk> wrote:
>>>>> When did we last revisit what minimal python version we are ok with requiring?
>>>>
>>>> I was wondering if people would weigh in discussing that in response to
>>>> [1] but no one has commented on that part of it. As another datapoint,
>>>> Brandon Casey was suggesting patching git-p4.py to support Python 2.4
>>>> [2].
>>>>
>>>> [1] http://article.gmane.org/gmane.comp.version-control.git/213920
>>>> [2] http://article.gmane.org/gmane.comp.version-control.git/214048
>>>
>>> I for one would be happy to kill off support for anything older than
>>> 2.6 (which had it's latest release on October 1st, 2008).
>>>
>>> Junio, how have we decided in the past which version of x to support?
>>
>> I do not think there was any conclusion. $gmane/212215 claiming 2.4
>> support matters for RHEL 5.x users was the last on the topic as far
>> as I can tell, so it boils down to another question: do users on
>> RHEL 5.x matter?
>>
>> I can read from $gmane/212215 that users of the said platform can
>> safely keep using Python 2.4 under their vendor support contract
>> until 2017. But let's focus on what do these users expect of their
>> system and software they run on it a bit.
>>
>> When they want to run a piece software that is not shipped with
>> RHEL, either by writing their own or by importing from elsewhere,
>> that needs 2.6 features, what are their options?
>>
>> (a) The platform vendor optionally supplies 2.6 with or without
>> support;
>>
>> (b) The users can and do install 2.6 as /usr/local/bin/python2.6,
>> which may even be community-supported, but the vendor does not
>> support it; or
>>
>> (c) The vendor terminates the support contract for users who choose
>> to go (b).
>>
>> I think we can safely discard (c); if that is the case, the users on
>> the said platform will not choose to update Git either, so it does
>> not matter where the future versions of Git sets the lower bound of
>> Python version at.
>>
>> If we are not talking about the situation (c), then the users can
>> choose to use 2.6, and more importantly, Python being a popular
>> software, I would imagine that there are reputable sources of
>> prepackaged RPMs for them to do so without going too much hassle of
>> configuring, compiling and installing.
>>
>> Now how does the decision we make today for releases of Git that
>> haven't yet happened will affect these users? As these versions of
>> newer Git were not shipped with RHEL 5.x, and also I am assuming
>> that Git is a more niche product than Python is, I would imagine
>> that it is very unlikely that the vendor gives it the users as an
>> optional package. The users will have to do the same thing to be
>> able to use such versions of Git as whatever they do in order to use
>> Python 2.6.
>>
>> Given that, what the vendor originally shipped and officially
>> supports does not affect the choices we would make today for newer
>> versions of Git. The users in a shop where additional third-party
>> software in /usr/local/bin is strictly forbidden, they are stuck
>> with the version of Git that the vendor shipped anyway, because they
>> won't be able to install an updated Git in /usr/local/bin, either.
>>
>> That is, unless installing 2.6 as /usr/local/bin/python2.6 (or if
>> you are really paranoid, /usr/local/only-for-git/bin/python2.6 where
>> nobody's $PATH points at) is impossible.
>>
>> So personally I do not think dropping 2.4 is a huge problem for
>> future versions of Git, but I'd like to hear from those working in
>> IT support for large and slow-moving organizations (aka RHEL 5
>> customers).
>
> I'm not really in the demographic that you asked to hear from, but
> I'll give my 2 cents anyway. :)
>
> Firstly, I defer to those with more knowledge and experience with
> python to decide which version should be the minimum version
> supported. Python 2.6 seems to be the consensus and that's fine with
> me.
>
> With respect to older platforms like RHEL 5.X that don't ship with
> Python 2.6 or later, I suspect most people who work in an organization
> with a dedicated IT staff can request that a more recent version of
> python be installed. So, I don't think a python 2.6 requirement (if
> there was one) would be a blocker for them, and I don't think it would
> be a major pain for the sysadmin to install.
>
Just a datapoint: I'm working with customers on RHEL 5.X that
unfortunately has an extremely lengthy (>3 months) process of
approving non-standard packages for install. Yeah, it's horrible, but
some times that's reality.
We are currently not using Git with that client, but we are in the
process of changing that. Said customer already have an exception for
all versions of Git.
I doubt this will end up being a problem in reality or not, but if it
will be, I'm sure it can be worked around out. I'm just pointing out
that the above suspicion might not be accurate.
^ permalink raw reply
* Re: [PATCH] Get correct column with for options in command usage
From: Junio C Hamano @ 2013-02-05 16:09 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jiang Xin, Git List
In-Reply-To: <CACsJy8Ci05C1mKDGp_YzP0H6G0Y40K8MUUq+D6NzjtPGRNms0g@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Tue, Feb 5, 2013 at 7:15 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> + fprintf(outfile, sb.buf);
>
> Use fputs instead. I looked up fputs man page but somehow still left
> fprintf there.
Once the streams of "oops that was wrong" comments are done, I'd
appreciate if one of you guys send a finished patch for application,
instead of forcing me to follow all the messages in the discussion
and picking up pieces.
Thanks.
^ permalink raw reply
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05 16:15 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <87mwvjsqjc.fsf_-_@lifelogs.com>
Ted Zlatanov <tzz@lifelogs.com> writes:
> +# build reverse token map
> +my %rmap;
> +foreach my $k (keys %{$options{tmap}}) {
> + push @{$rmap{$options{tmap}->{$k}}}, $k;
> +}
Mental note: "$rmap{foo} -eq 'bar'" means that what Git calls 'bar'
is found as 'foo' in the netrc/authinfo file. Keys in %rmap are
what we expect to read from the netrc/authinfo file.
> +# there are CPAN modules to do this better, but we want to avoid
> +# dependencies and generally, complex netrc-style files are rare
> +
> +if ($debug) {
> + printf STDERR "searching for %s = %s\n", $_, $q{$_} || '(any value)'
> + foreach sort keys %q;
> +}
> +
> +LINE: foreach my $line (@data) {
> +
> + print STDERR "line [$line]\n" if $debug;
> + my @tok;
> + # gratefully stolen from Net::Netrc
> + while (length $line &&
> + $line =~ s/^("((?:[^"]+|\\.)*)"|((?:[^\\\s]+|\\.)*))\s*//) {
> + (my $tok = $+) =~ s/\\(.)/$1/g;
> + push(@tok, $tok);
> + }
> +
> + # skip blank lines, comments, etc.
> + next LINE unless scalar @tok;
> +
> + my %tokens;
> + my $num_port;
> + while (@tok) {
> + my ($k, $v) = (shift @tok, shift @tok);
> + next unless defined $v;
> + next unless exists $options{tmap}->{$k};
> + $tokens{$options{tmap}->{$k}} = $v;
> + $num_port = $v if $k eq 'port' && $v =~ m/^\d+$/;
> + }
So you grabbed one line of input, split them into token pairs, and
built %tokens = ('key Git may want to see' => 'value read from file')
mapping.
> + # for "host X port Y" where Y is an integer (captured by
> + # $num_port above), set the host to "X:Y"
> + $tokens{host} = join(':', $tokens{host}, $num_port)
> + if defined $tokens{host} && defined $num_port;
What happens when 'host' does not exist? netrc/authinfo should be a
stream of SP/HT/LF delimited tokens and 'machine' token (or
'default') begins a new entry, so it would mean the input file is
corrupt if we do not have $tokens{host} when we get here, I think.
Oh, another thing. 'default' is like 'machine' followed by any
machine name, so the above while loop that reads two tokens
pair-wise needs to be aware that 'default' is not followed by a
value. I think the loop will fail to parse this:
default login anonymous password me@home
machine k.org login me password mysecret
> + foreach my $check (sort keys %q) {
Hmph, aren't you checking what you read a bit too early? This is a
valid input:
default
login anonymous
password me@home
machine k.org
login me
password mysecret
but does this loop gives mysecret back to me when asked for
host=k.org and user=me?
> + if (exists $tokens{$check} && defined $q{$check}) {
> + print STDERR "comparing [$tokens{$check}] to [$q{$check}] in line [$line]\n" if $debug;
> + next LINE unless $tokens{$check} eq $q{$check};
> + }
> + else {
> + print STDERR "we could not find [$check] but it's OK\n" if $debug;
> + }
> + }
I would probably structure this part like this:
%pending = ();
split the whole input into tokens, regardless of lines;
iterate over the tokens {
peek the token
if (it is not "default") {
take (token, value) pair;
} else {
take "default" as token; value does not matter.
}
if (token is "default" or "machine") {
# finished reading one entry and we are
# at the beginning of the next entry.
# see if this entry matches
if (%pending is not empty &&
%pending matches %q) {
found a match; use %pending;
}
# done with that entry. now start a new one.
%pending = ();
}
$pending{token} = value;
}
^ permalink raw reply
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Junio C Hamano @ 2013-02-05 16:18 UTC (permalink / raw)
To: Ted Zlatanov; +Cc: Jeff King, git
In-Reply-To: <7v7gmmvj5p.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I thought I've given a more concrete outline than "I'll read
> Net::Netrc and do whatever I think it does" in a separate message.
And it turns out that the message was sitting in my outbox. Sorry.
I just told the outbox to send it out.
^ permalink raw reply
* [PATCH v2 1/2] Get correct column with for options in command usage
From: Jiang Xin @ 2013-02-05 16:16 UTC (permalink / raw)
To: Junio C Hamano, Duy Nguyen; +Cc: Git List, Jiang Xin
In-Reply-To: <20130205121552.GA16601@lanh>
Command usage would not align well if command options are translated,
especially to CJK. Call utf8_strwidth in function usage_argh, so that
the caller will get correct column width.
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 67e98..cd029f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -3,6 +3,7 @@
#include "cache.h"
#include "commit.h"
#include "color.h"
+#include "utf8.h"
static int parse_options_usage(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
@@ -473,7 +474,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
static int usage_argh(const struct option *opts, FILE *outfile)
{
- const char *s;
+ const char *s, *p;
int literal = (opts->flags & PARSE_OPT_LITERAL_ARGHELP) || !opts->argh;
if (opts->flags & PARSE_OPT_OPTARG)
if (opts->long_name)
@@ -482,7 +483,10 @@ static int usage_argh(const struct option *opts, FILE *outfile)
s = literal ? "[%s]" : "[<%s>]";
else
s = literal ? " %s" : " <%s>";
- return fprintf(outfile, s, opts->argh ? _(opts->argh) : _("..."));
+ p = opts->argh ? _(opts->argh) : _("...");
+ fprintf(outfile, s, p);
+ /* Remove extra 2 chars ("%s" in s) to get column width of utf8 string */
+ return utf8_strwidth(p) + strlen(s) - 2;
}
#define USAGE_OPTS_WIDTH 24
--
1.8.1.1.368.g6034fad.dirty
^ permalink raw reply
* [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Jiang Xin @ 2013-02-05 16:16 UTC (permalink / raw)
To: Junio C Hamano, Duy Nguyen; +Cc: Git List, Jiang Xin
In-Reply-To: <883efc2358a0deb48bee48134d45ddd528a732d3.1360080194.git.worldhello.net@gmail.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
parse-options.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index cd029f..be916 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -497,6 +497,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
const struct option *opts, int full, int err)
{
FILE *outfile = err ? stderr : stdout;
+ const char *opt_num_buff = _("-NUM");
+ int opt_num_size = utf8_strwidth(opt_num_buff);
if (!usagestr)
return PARSE_OPT_HELP;
@@ -544,8 +546,10 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
pos += fprintf(outfile, ", ");
if (opts->long_name)
pos += fprintf(outfile, "--%s", opts->long_name);
- if (opts->type == OPTION_NUMBER)
- pos += fprintf(outfile, "-NUM");
+ if (opts->type == OPTION_NUMBER) {
+ fputs(opt_num_buff, outfile);
+ pos += opt_num_size;
+ }
if ((opts->flags & PARSE_OPT_LITERAL_ARGHELP) ||
!(opts->flags & PARSE_OPT_NOARG))
--
1.8.1.1.368.g6034fad.dirty
^ permalink raw reply
* Re: [WIP/RFH/RFD/PATCH] grep: allow to use textconv filters
From: Michael J Gruber @ 2013-02-05 16:21 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20130205111353.GD24973@sigill.intra.peff.net>
Jeff King venit, vidit, dixit 05.02.2013 12:13:
> On Mon, Feb 04, 2013 at 04:27:31PM +0100, Michael J Gruber wrote:
>
>> Recently and not so recently, we made sure that log/grep type operations
>> use textconv filters when a userfacing diff would do the same:
>>
>> ef90ab6 (pickaxe: use textconv for -S counting, 2012-10-28)
>> b1c2f57 (diff_grep: use textconv buffers for add/deleted files, 2012-10-28)
>> 0508fe5 (combine-diff: respect textconv attributes, 2011-05-23)
>>
>> "git grep" currently does not use textconv filters at all, that is
>> neither for displaying the match and context nor for the actual grepping.
>>
>> Introduce a binary mode "--textconv" (in addition to "--text" and "-I")
>> which makes git grep use any configured textconv filters for grepping
>> and output purposes.
>
> Sounds like a reasonable goal.
>
>> The difficulty is in getting the different cases (blob/sha1 vs.
>> worktree) right, and in making the changes minimally invasive. It seems
>> that some more refactoring could help: "git show --textconv" does not
>> use textconv filters when used on blobs either. (It does for diffs, of
>> course.) Most existing helper functions are tailored for diffs.
>
> I think "git show" with blobs originally did not because we have no
> filename with which to look up the attributes. IIRC, the patches to
> support "cat-file --textconv" taught get_sha1_with_context to report the
> path at which we found a blob. I suspect it is mostly a matter of
> plumbing that information from the revision parser through to
> show_blob_object.
>
>> Nota bene: --textconv does not affect "diff --stat" either...
>
> Yeah, though I wonder if it should be on by default. The diffstat for a
> binary file, unlike the diff, is already useful. The diffstat of the
> textconv'd data may _also_ be useful, of course.
>
>> @@ -659,6 +659,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>> OPT_SET_INT('I', NULL, &opt.binary,
>> N_("don't match patterns in binary files"),
>> GREP_BINARY_NOMATCH),
>> + OPT_SET_INT(0, "textconv", &opt.binary,
>> + N_("process binary files with textconv filters"),
>> + GREP_BINARY_TEXTCONV),
>
> Is this really a new form of GREP_BINARY_*? What happens when a file
> does not have a textconv filter?
>
> I would expect this to be more like diff's "--textconv" flag, which is
> really "allow textconv to be respected". Then you could do:
>
> git grep -I --textconv foo
>
> to grep in the text version of files which support it, and ignore the
> rest.
>
>> -static int grep_source_load(struct grep_source *gs);
>> -static int grep_source_is_binary(struct grep_source *gs);
>> +static int grep_source_load(struct grep_source *gs, struct grep_opt *opt);
>> +static int grep_source_is_binary(struct grep_source *gs, struct grep_opt *opt);
>
> Hmm. grep_source_load is more or less the analogue of
> diff_populate_filespec, which does not know about textconv at all. So I
> feel like this might be going in at the wrong layer...
>
>> @@ -1354,14 +1356,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
>>
>> switch (opt->binary) {
>> case GREP_BINARY_DEFAULT:
>> - if (grep_source_is_binary(gs))
>> + if (grep_source_is_binary(gs, opt))
>> binary_match_only = 1;
>> break;
>> case GREP_BINARY_NOMATCH:
>> - if (grep_source_is_binary(gs))
>> + if (grep_source_is_binary(gs, opt))
>> return 0; /* Assume unmatch */
>> break;
>
> The is_binary function learned about "opt" so that it could pass it to
> grep_source_load, which might do the textconv for us. But we _know_ that
> we will not here, because we see that we have other GREP_BINARY flags.
>
> And when we do have _TEXTCONV:
>
>> case GREP_BINARY_TEXT:
>> + case GREP_BINARY_TEXTCONV:
>> break;
>
> We do not call is_binary. :)
>
>> -static int grep_source_load_sha1(struct grep_source *gs)
>> +static int grep_source_load_sha1(struct grep_source *gs, struct grep_opt *opt)
>> {
>> enum object_type type;
>> -
>> grep_read_lock();
>> - gs->buf = read_sha1_file(gs->identifier, &type, &gs->size);
>> + if (opt->binary == GREP_BINARY_TEXTCONV) {
>> + struct diff_filespec *df = alloc_filespec(gs->name);
>> + gs->size = fill_textconv(gs->driver, df, &gs->buf);
>> + free_filespec(df);
>> + } else {
>> + gs->buf = read_sha1_file(gs->identifier, &type, &gs->size);
>> + }
>
> So here we do the textconv for the sha1 case. But what about file
> sources?
>
> This is why I think the layer is wrong; you want the fill_textconv
> function to call your abstract _load function (in the diff_filespec
> world, it is diff_filespec_populate, but it is the moral equivalent).
> And you want it to hold off as long as possible in case we can pull the
> value from cache, or feed the working tree version of a file straight to
> the filter.
>
>> -void grep_source_load_driver(struct grep_source *gs)
>> +void grep_source_load_driver(struct grep_source *gs, struct grep_opt *opt)
>> {
>> if (gs->driver)
>> return;
>>
>> - grep_attr_lock();
>> + grep_attr_lock(); //TODO
>> + printf("Looking up userdiff driver for: %s", gs->path);
>> if (gs->path)
>> gs->driver = userdiff_find_by_path(gs->path);
>> if (!gs->driver)
>> gs->driver = userdiff_find_by_name("default");
>> + if (opt->binary == GREP_BINARY_TEXTCONV)
>> + gs->driver = userdiff_get_textconv(gs->driver);
>> grep_attr_unlock();
>> }
>
> This is wrong. The point of userdiff_get_textconv is that it will return
> NULL when we are not doing textconv for this path. So you can use it
> like:
>
> struct userdiff_driver *textconv = userdiff_get_textconv(gs->driver);
>
> if (textconv) {
> /* ok, we are doing textconv. Call our fill_textconv
> * equivalent. */
> }
> else {
> /* nope, plain old file. */
> }
>
> But by assigning it on top of gs->driver, you're going to end up with a
> NULL driver sometimes. And the post-condition of the load_driver
> function is that gs->driver always points to a valid driver (even if it
> is the default one). I wouldn't be surprised if this causes segfaults.
>
>
> So I would do it more like the patch below. Only lightly tested by me.
> There are some refactoring opportunities if you want to bring
> grep_source and diff_filespec closer together.
>
> ---
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 8025964..915c8ef 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -659,6 +659,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> OPT_SET_INT('I', NULL, &opt.binary,
> N_("don't match patterns in binary files"),
> GREP_BINARY_NOMATCH),
> + OPT_BOOL(0, "textconv", &opt.allow_textconv,
> + N_("process binary files with textconv filters")),
> { OPTION_INTEGER, 0, "max-depth", &opt.max_depth, N_("depth"),
> N_("descend at most <depth> levels"), PARSE_OPT_NONEG,
> NULL, 1 },
> diff --git a/grep.c b/grep.c
> index 4bd1b8b..3880d64 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -2,6 +2,8 @@
> #include "grep.h"
> #include "userdiff.h"
> #include "xdiff-interface.h"
> +#include "diff.h"
> +#include "diffcore.h"
>
> static int grep_source_load(struct grep_source *gs);
> static int grep_source_is_binary(struct grep_source *gs);
> @@ -1321,6 +1323,58 @@ static void std_output(struct grep_opt *opt, const void *buf, size_t size)
> fwrite(buf, size, 1, stdout);
> }
>
> +static int fill_textconv_grep(struct userdiff_driver *driver,
> + struct grep_source *gs)
> +{
> + struct diff_filespec *df;
> + char *buf;
> + size_t size;
> +
> + if (!driver || !driver->textconv)
> + return grep_source_load(gs);
> +
> + /*
> + * The textconv interface is intimately tied to diff_filespecs, so we
> + * have to pretend to be one. If we could unify the grep_source
> + * and diff_filespec structs, this mess could just go away.
> + */
> + df = alloc_filespec(gs->path);
> + switch (gs->type) {
> + case GREP_SOURCE_SHA1:
> + fill_filespec(df, gs->identifier, 1, 0100644);
> + break;
> + case GREP_SOURCE_FILE:
> + fill_filespec(df, null_sha1, 0, 0100644);
> + break;
> + default:
> + die("BUG: attempt to textconv something without a path?");
> + }
> +
> + /*
> + * fill_textconv is not remotely thread-safe; it may load objects
> + * behind the scenes, and it modifies the global diff tempfile
> + * structure.
> + */
> + grep_read_lock();
> + size = fill_textconv(driver, df, &buf);
> + grep_read_unlock();
> + free_filespec(df);
> +
> + /*
> + * The normal fill_textconv usage by the diff machinery would just keep
> + * the textconv'd buf separate from the diff_filespec. But much of the
> + * grep code passes around a grep_source and assumes that its "buf"
> + * pointer is the beginning of the thing we are searching. So let's
> + * install our textconv'd version into the grep_source, taking care not
> + * to leak any existing buffer.
> + */
> + grep_source_clear_data(gs);
> + gs->buf = buf;
> + gs->size = size;
> +
> + return 0;
> +}
> +
> static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int collect_hits)
> {
> char *bol;
> @@ -1331,6 +1385,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
> unsigned count = 0;
> int try_lookahead = 0;
> int show_function = 0;
> + struct userdiff_driver *textconv = NULL;
> enum grep_context ctx = GREP_CONTEXT_HEAD;
> xdemitconf_t xecfg;
>
> @@ -1352,19 +1407,36 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
> }
> opt->last_shown = 0;
>
> - switch (opt->binary) {
> - case GREP_BINARY_DEFAULT:
> - if (grep_source_is_binary(gs))
> - binary_match_only = 1;
> - break;
> - case GREP_BINARY_NOMATCH:
> - if (grep_source_is_binary(gs))
> - return 0; /* Assume unmatch */
> - break;
> - case GREP_BINARY_TEXT:
> - break;
> - default:
> - die("bug: unknown binary handling mode");
> + if (opt->allow_textconv) {
> + grep_source_load_driver(gs);
> + /*
> + * We might set up the shared textconv cache data here, which
> + * is not thread-safe.
> + */
> + grep_attr_lock();
> + textconv = userdiff_get_textconv(gs->driver);
> + grep_attr_unlock();
> + }
> +
> + /*
> + * We know the result of a textconv is text, so we only have to care
> + * about binary handling if we are not using it.
> + */
> + if (!textconv) {
> + switch (opt->binary) {
> + case GREP_BINARY_DEFAULT:
> + if (grep_source_is_binary(gs))
> + binary_match_only = 1;
> + break;
> + case GREP_BINARY_NOMATCH:
> + if (grep_source_is_binary(gs))
> + return 0; /* Assume unmatch */
> + break;
> + case GREP_BINARY_TEXT:
> + break;
> + default:
> + die("bug: unknown binary handling mode");
> + }
> }
>
> memset(&xecfg, 0, sizeof(xecfg));
> @@ -1372,7 +1444,7 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
>
> try_lookahead = should_lookahead(opt);
>
> - if (grep_source_load(gs) < 0)
> + if (fill_textconv_grep(textconv, gs) < 0)
> return 0;
>
> bol = gs->buf;
> diff --git a/grep.h b/grep.h
> index 8fc854f..94a7ac2 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -106,6 +106,7 @@ struct grep_opt {
> #define GREP_BINARY_NOMATCH 1
> #define GREP_BINARY_TEXT 2
> int binary;
> + int allow_textconv;
> int extended;
> int use_reflog_filter;
> int pcre;
>
Thanks Jeff, that helps a lot! It covers "grep expr" and "grep expr rev
-- path" just fine. I'll look into "grep expr rev:path" which does not
work yet because of an empty driver.
I also have "show --textconv" covered and a suggestion for "cat-file
--textconv" (to work without a textconv filter).
Expect a mini-series soon :)
Michael
^ permalink raw reply
* Re: [PATCH v3] status: show the branch name if possible in in-progress info
From: Junio C Hamano @ 2013-02-05 16:28 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jonathan Nieder, git, Matthieu Moy
In-Reply-To: <CACsJy8D_kSrJYagxo1LWt=P8XKj9F9_yAwH3vzQWQOi+yvP8kg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Tue, Feb 5, 2013 at 1:38 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Missing description. Stealing from the link you sent:
>>
>> The typical use-case is starting a rebase, do something else, come back
>> the day after, run "git status" or make a new commit and wonder what
>> in the world's going on. Which branch is being rebased is probably the
>> most useful tidbit to help, but the target may help too.
>>
>> Ideally, I would have loved to see "rebasing master on origin/master",
>> but the target ref name is not stored during rebase, so this patch
>> writes "rebasing master on a78c8c98b" as a half-measure to remind
>> future users of that potential improvement.
>>
>> Signed-off-by: <...>
>
> Looking good. Junio, do you need a new patch mail or can you just
> amend the commit message?
I'd like to see you either
- send a reroll, making it clear it is a reroll, or
- tell me to amend.
instead of asking which one I would prefer ;-) One less message I
have to respond to that way.
"commit --amend" done.
^ permalink raw reply
* Re: [PATCH] Add contrib/credentials/netrc with GPG support, try #2
From: Ted Zlatanov @ 2013-02-05 17:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7vvca6u47f.fsf@alter.siamese.dyndns.org>
On Tue, 05 Feb 2013 08:15:48 -0800 Junio C Hamano <gitster@pobox.com> wrote:
JCH> Ted Zlatanov <tzz@lifelogs.com> writes:
>> +# build reverse token map
>> +my %rmap;
>> +foreach my $k (keys %{$options{tmap}}) {
>> + push @{$rmap{$options{tmap}->{$k}}}, $k;
>> +}
JCH> Mental note: "$rmap{foo} -eq 'bar'" means that what Git calls 'bar'
JCH> is found as 'foo' in the netrc/authinfo file. Keys in %rmap are
JCH> what we expect to read from the netrc/authinfo file.
I'll document that better in PATCHv4.
JCH> So you grabbed one line of input, split them into token pairs, and
JCH> built %tokens = ('key Git may want to see' => 'value read from file')
JCH> mapping.
This will be fixed with PATCHv4 to do multiple lines (as defined by the
netrc manpage, etc.).
>> + # for "host X port Y" where Y is an integer (captured by
>> + # $num_port above), set the host to "X:Y"
>> + $tokens{host} = join(':', $tokens{host}, $num_port)
>> + if defined $tokens{host} && defined $num_port;
JCH> What happens when 'host' does not exist? netrc/authinfo should be a
JCH> stream of SP/HT/LF delimited tokens and 'machine' token (or
JCH> 'default') begins a new entry, so it would mean the input file is
JCH> corrupt if we do not have $tokens{host} when we get here, I think.
Yes. I'll make the host/machine token required, which will avoid this
issue.
JCH> Oh, another thing. 'default' is like 'machine' followed by any
JCH> machine name, so the above while loop that reads two tokens
JCH> pair-wise needs to be aware that 'default' is not followed by a
JCH> value. I think the loop will fail to parse this:
JCH> default login anonymous password me@home
JCH> machine k.org login me password mysecret
I'd prefer to ignore "default" because it should not be used for the Git
credential helpers (its only use case is for anonymous services AFAIK).
So I'll add a case to ignore it in PATCHv4, if that's OK.
JCH> Hmph, aren't you checking what you read a bit too early? This is a
JCH> valid input:
JCH> default
JCH> login anonymous
JCH> password me@home
JCH> machine k.org
JCH> login me
JCH> password mysecret
JCH> but does this loop gives mysecret back to me when asked for
JCH> host=k.org and user=me?
To be fixed in PATCHv4, which will require the host/machine, use it as
the primary key, and only examine entries with it.
JCH> I would probably structure this part like this: [...]
I will do it like that, thank you for the suggestion.
I'll also add a simple testing Makefile for my own use, and you can
consider adding tests to the general framework later.
Ted
^ permalink raw reply
* Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Dale R. Worley @ 2013-02-05 17:00 UTC (permalink / raw)
To: git
I have found a situation where "git log" produces (apparently)
endless output. Presumably this is a bug. Following is a (Linux)
script that reliably reproduces the error for me (on Fedora 16):
----------
set -ve
# Print the git version.
git --version
# Create respository.
rm -rf .git
git init
# Initial commit.
( echo 1 ; echo 2 ; echo 3 ) >file
git add file
git commit -m 'Commit P'
git branch B HEAD
# Next commit on master adds line "1a".
( echo 1 ; echo 1a ; echo 2 ; echo 3 ) >file
git add file
git commit -m 'Commit Q'
git checkout B
# Next commit on B adds line "2a".
( echo 1 ; echo 2 ; echo 2a ; echo 3 ) >file
git add file
git commit -m 'Commit R'
# Merge the two commits, but add line "3a" to the commit as well.
git checkout master
git merge --no-commit B
# Show what the merge produces.
cat file
# Add line "3a".
( echo 1 ; echo 1a ; echo 2 ; echo 2a ; echo 3 ; echo 3a ) >file
git commit -m 'Commit S'
# These log commands work.
git log
git log --graph
git log --graph -p
# This log command produces infinite output.
git log --graph -p -m
----------
Dale
^ permalink raw reply
* Re: [PATCH v2 2/2] i18n: mark OPTION_NUMBER (-NUM) for translation
From: Junio C Hamano @ 2013-02-05 17:07 UTC (permalink / raw)
To: Jiang Xin; +Cc: Duy Nguyen, Git List
In-Reply-To: <8d6d4d869ea58e0a26b3bb6377fc102728948997.1360080194.git.worldhello.net@gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> parse-options.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index cd029f..be916 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -497,6 +497,8 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
> const struct option *opts, int full, int err)
> {
> FILE *outfile = err ? stderr : stdout;
> + const char *opt_num_buff = _("-NUM");
> + int opt_num_size = utf8_strwidth(opt_num_buff);
>
> if (!usagestr)
> return PARSE_OPT_HELP;
> @@ -544,8 +546,10 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
> pos += fprintf(outfile, ", ");
> if (opts->long_name)
> pos += fprintf(outfile, "--%s", opts->long_name);
> - if (opts->type == OPTION_NUMBER)
> - pos += fprintf(outfile, "-NUM");
> + if (opts->type == OPTION_NUMBER) {
> + fputs(opt_num_buff, outfile);
> + pos += opt_num_size;
> + }
I somehow suspect that this is going in a direction that makes this
piece of code much less maintainable.
Look at the entire function and see how many places you do fprintf
on strings that are marked with _(). short_name and long_name are
not likely to be translated, but everything else is, especially
multiple places that show _(opts->help) neither of these patches
touch.
I wonder if it makes more sense to add a helper function that
returns the number of column positions (not bytes) with a signature
similar to fprintf() and use that throughout the function instead.
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Junio C Hamano @ 2013-02-05 17:27 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git, Jeff King, Shawn Pearce
In-Reply-To: <5110BD18.3080608@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> I would again like to express my discomfort about this feature, which is
> already listed as "will merge to next".
Do not take "will merge to next" too literally. One major purpose
of marking a topic as such is exactly to solicit comments like this
;-)
> * I didn't see a response to Peff's convincing arguments that this
> should be a client-side feature rather than a server-side feature [1].
Uncluttering is not about a choice client should make. "delayed
advertisement" is an orthogonal issue and requires a larger protocol
update (it needs to make "git fetch" speak first instead of the
current protocol in which "upload-pack" speaks first).
> * I didn't see an answer to Duy's question [2] about what is different
> between the proposed feature and gitnamespaces.
I think Jonathan addressed this already.
> * I didn't see a response to my worries that this feature could be
> abused [3].
You can choose not to advertise allow-tip-sha1-in-want capability; I
do not think it is making things worse than the status quo.
> * Why should a repository have exactly one setting for what refs should
> be hidden? Wouldn't it make more sense to allow multiple "views" to be
> defined?:
You are welcome to extend to have different views, but how would
your clients express which view they would want?
Giving a single view that the serving end decides gives us an
immediate benefit of showing an uncluttered set of refs of server's
choice, without making the problem space larger than necessary.
> * Is it enough to support only reference exclusion (as opposed to
> exclusion and inclusion rules)?
Again, I do not think you cannot extend it to do positive and
negative filtering "exclude these, but include those even though
they match the 'exclude these' patterns I gave you earlier".
> * Why should this feature only be available remotely?
The whole point is to give the server side a choice to show selected
refs, so that it can use hidden portion for its own use. These refs
should not be hidden from local operations like "gc".
I appreciate the comments, but I do not think any point you raised
in this message is very much relevant as objections.
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Junio C Hamano @ 2013-02-05 17:36 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Michael Haggerty, git, Jeff King, Shawn Pearce
In-Reply-To: <20130205083327.GA4931@elie.Belkin>
Jonathan Nieder <jrnieder@gmail.com> writes:
>> * I didn't see a response to Peff's convincing arguments that this
>> should be a client-side feature rather than a server-side feature [1].
>
> The client can't control the size of the ref advertisement. That is
> the main motivation if I understood correctly.
The answer to this question is more nuanced.
With the current protocol, it is upload-pack who speaks first, so
there is no way for the requestor to say "I am from an updated Git
suite and understand how to tell you to give me limited set of
refs", before upload-pack blasts 4MB of ref advertisement to it.
If the side that fetches is potentially interested in finding out
any and all refs, then an alternative solution would be to break
the current protocol, open a separate port and have upload-pack-2
listen to it, sit silently to let the requestor speak first when it
gets connection to that port.
But if the primary thing you are interested in is to hide the
references that:
(1) the server side needs to keep track of for its own use; but
(2) the requestors do not have to learn about from upload-pack,
we can do so without breaking older requestors. That is what the
early part of this series is about. We can view the last patch to
add the allow-tip-sha1-in-want as an icing on the cake.
It has the side effect of reducing the transfer overhead, because by
hiding the internal refs, the server side will stop blasting 4MB of
ref advertisements the requestors are not interested in, and that
would be the primary observable outcome from the end-user's point of
view (i.e. your "git pull --ff-only" will become a lot faster).
^ permalink raw reply
* Re: [PATCH v3 0/8] Hiding refs
From: Junio C Hamano @ 2013-02-05 17:38 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Jonathan Nieder, git, Jeff King, Shawn Pearce
In-Reply-To: <5110DF1D.8010505@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 02/05/2013 09:33 AM, Jonathan Nieder wrote:
>> Michael Haggerty wrote:
>>
>>> I would again like to express my discomfort about this feature, which is
>>> already listed as "will merge to next". Frankly, I have the feeling
>>> that this feature is being steamrolled in before a community consensus
>>> has been reached and indeed before many valid points raised by other
>>> members of the community have even been addressed. For example:
>>
>> In $dayjob I work with Gerrit, so I think I can start to answer some
>> of these questions.
>>
>>> * I didn't see a response to Peff's convincing arguments that this
>>> should be a client-side feature rather than a server-side feature [1].
>>
>> The client can't control the size of the ref advertisement. That is
>> the main motivation if I understood correctly.
>
> Not according to Junio [4]:
>
> Look at this as a mechanism for the repository owner to control the
> clutter in what is shown to the intended audience of what s/he
> publishes in the repository. Network bandwidth reduction of
> advertisement is a side effect of clutter reduction, and not
> necessarily the primary goal.
See my response to Jonathan.
> Hiderefs creates a "dark" corner of a remote git repo that can hold
> arbitrary content that is impossible for anybody to discover but
> nevertheless possible for anybody to download (if they know the name of
> a hidden reference).
That is why allow-tip-sha1-in-want is a separate opt-in feature only
the server side controls.
^ permalink raw reply
* Re: Bug in "git log --graph -p -m" (version 1.7.7.6)
From: Junio C Hamano @ 2013-02-05 17:40 UTC (permalink / raw)
To: Dale R. Worley; +Cc: git
In-Reply-To: <201302051700.r15H0GXx031004@freeze.ariadne.com>
worley@alum.mit.edu (Dale R. Worley) writes:
> I have found a situation where "git log" produces (apparently)
> endless output. Presumably this is a bug. Following is a (Linux)
> script that reliably reproduces the error for me (on Fedora 16):
Wasn't this fixed at v1.8.1.1~13 or is this a different issue?
^ 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