* [PATCH 5/6] Git.pm: teach "ident" to query explicitness
From: Jeff King @ 2012-11-13 16:53 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Thomas Rast, Junio C Hamano, Jonathan Nieder
In-Reply-To: <20121113164845.GD20361@sigill.intra.peff.net>
"git var" recently learned to report on whether an ident we
fetch from it was configured explicitly or implicitly. Let's
make that information available to callers of the ident
function.
Because evaluating "ident" in an array versus scalar context
already has a meaning, we cannot return our extra value in a
backwards compatible way. Instead, we require the caller to
add an extra "explicit" flag to request the information.
The ident_person function, on the other hand, always returns
a scalar, so we are free to overload it in an array context.
Signed-off-by: Jeff King <peff@peff.net>
---
perl/Git.pm | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/perl/Git.pm b/perl/Git.pm
index 497f420..1994ec1 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -737,7 +737,7 @@ sub remote_refs {
}
-=item ident ( TYPE | IDENTSTR )
+=item ident ( TYPE | IDENTSTR [, options] )
=item ident_person ( TYPE | IDENTSTR | IDENTARRAY )
@@ -750,6 +750,10 @@ and either returns it as a scalar string or as an array with the fields parsed.
Alternatively, it can take a prepared ident string (e.g. from the commit
object) and just parse it.
+If the C<explicit> option is set to 1, the returned array will contain an
+additional boolean specifying whether the ident was configure explicitly by the
+user.
+
C<ident_person> returns the person part of the ident - name and email;
it can take the same arguments as C<ident> or the array returned by C<ident>.
@@ -763,17 +767,22 @@ The synopsis is like:
=cut
sub ident {
- my ($self, $type) = _maybe_self(@_);
- my $identstr;
+ my ($self, $type, %options) = _maybe_self(@_);
+ my ($identstr, $explicit);
if (lc $type eq lc 'committer' or lc $type eq lc 'author') {
- my @cmd = ('var', 'GIT_'.uc($type).'_IDENT');
+ my $uc = uc($type);
+ my @cmd = ('var', "GIT_${uc}_IDENT", "GIT_${uc}_EXPLICIT");
unshift @cmd, $self if $self;
- $identstr = command_oneline(@cmd);
+ ($identstr, $explicit) = command(@cmd);
} else {
$identstr = $type;
}
if (wantarray) {
- return $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+ my @ret = $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
+ if ($options{explicit} && defined $explicit) {
+ push @ret, $explicit if defined $explicit;
+ }
+ return @ret;
} else {
return $identstr;
}
@@ -781,8 +790,11 @@ sub ident {
sub ident_person {
my ($self, @ident) = _maybe_self(@_);
- $#ident == 0 and @ident = $self ? $self->ident($ident[0]) : ident($ident[0]);
- return "$ident[0] <$ident[1]>";
+ $#ident == 0 and @ident = $self ?
+ $self->ident($ident[0], explicit => 1) :
+ ident($ident[0], explicit => 1);
+ my $ret = "$ident[0] <$ident[1]>";
+ return wantarray ? ($ret, @ident[3]) : $ret;
}
--
1.8.0.207.gdf2154c
^ permalink raw reply related
* Re: Notes in format-patch
From: Junio C Hamano @ 2012-11-13 16:56 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <7vhaotfou6.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> Michael J Gruber venit, vidit, dixit 12.11.2012 15:18:
>>> 'git replace' parses the revision arguments when it creates replacements
>>> (so that a sha1 can be abbreviated, e.g.) but not when deleting
>>> replacements.
>>>
>>> Make it parse the argument to 'replace -d' in the same way.
>>>
>>> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>>> ---
>>>
>>> Notes:
>>> v3 safeguards the hex buffer against reuse
>>> builtin/replace.c | 16 ++++++++++------
>>> t/t6050-replace.sh | 11 +++++++++++
>>> 2 files changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/builtin/replace.c b/builtin/replace.c
>>
>> By the way - Junio, is that the intented outcome of "format-patch
>> --notes"? I would rather put the newline between the note and the
>> diffstat...
>
> I do not mind (actually I personally would prefer to see) a blank
> line between the three-dash and "Notes:", but I agree that we should
> have a blank line before the diffstat block.
As the topic seems to be already in Peff's next, here is a trivial
fix for this in incremental form.
-- >8 --
Subject: format-patch: add a blank line between notes and diffstat
The last line of the note text comes immediately before the diffstat
block, making the latter unnecessarily harder to view.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
log-tree.c | 1 +
1 file changed, 1 insertion(+)
diff --git i/log-tree.c w/log-tree.c
index 712a22b..9303fd8 100644
--- i/log-tree.c
+++ w/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
opt->shown_dashes = 1;
}
strbuf_addstr(&msgbuf, ctx.notes_message);
+ strbuf_addch(&msgbuf, '\n');
}
if (opt->show_log_size) {
^ permalink raw reply related
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jeff King @ 2012-11-13 17:04 UTC (permalink / raw)
To: Drew Northup
Cc: glpk xypron, git, jnareb, Junio C Hamano, Jason J Pyeron CTR (US),
Andreas Schwab
In-Reply-To: <CAM9Z-nkuHj8MWLfWsvY=EqHXCUS+Pk5Ezv6m5J+cnh7cQHNc_g@mail.gmail.com>
On Tue, Nov 13, 2012 at 09:44:06AM -0500, Drew Northup wrote:
> I don't buy the argument that we don't need to clean up the input as
> well. There are scant few of us that are going to name a file
> "<script>alert("Something Awful")</script>" in this world (I am
> probably one of them). Input validation is key to keeping problems
> like this from coming up repeatedly as those writing the guts of
> programs are typically more interested in getting the "assigned task"
> done and reporting the output to the user in a safe manner.
Oh, you absolutely do need to clean up the input side. And we do. Notice
how validate_pathname cleans out dots that could allow an attacker to do
a "../../etc/passwd" attack. But the input validation is _different_
than the output escaping. We are turning arbitrary junk from the user
into something we know is safe to treat as a filename. Our goal is
protecting the filesystem and the server, and we do that already.
Protecting the browser on output is a different problem, and happens
only when we are sending to the browser.
As far as "people will not use <script>" in their filenames, the
end-game to any quoting or blacklist fix is that we need to escape or
black _all_ HTML. Because whether it is "<b>" or "<script>", it is
still wrong. Are you as comfortable saying that nobody will ever have a
"<" or "&" in their filename?
> > 3. Your filter is too simplistic. At the very least, it would not
> > filter out "<SCRIPT>". I am not up to date on all of the
> > sneaking-around-HTML-filters attacks that are available these days,
> > but I wonder if one could also get around it using XML entities or
> > similar.
>
> You will note that I said "a more definitive fix is in order" in my
> original. In other words, I claimed it to be utterly incomplete to
> start with.
Sorry if I came off as too harsh. My intent was to guide you in the
right direction for the definitive fix. The fact that I ended up rolling
the patch myself was just because my "probably something like this"
ended with everybody saying "yeah, that", and it seemed simpler to just
roll a test and be done.
> > I think the right answer is going to be a well-placed call to esc_html.
> > This already happens automatically when we go through the CGI
> > element-building functions, but obviously we failed to make the call
> > when building the output manually. This is a great reason why template
> > languages which default to safe expansion should always be used.
> > Unfortunately, gitweb is living in 1995 in terms of web frameworks.
>
> Escaping the output protects the user, but it DOES NOT protect the
> server. We MUST handle both possibilities.
Right. But I think we already do, via validate_pathname. If that is not
the case, please point it out.
> Besides, inserting one call to esc_html only fixes one attack path. I
> didn't look to see if all others were already covered.
Properly quoting output is something that the web framework should do
for you. gitweb uses CGI.pm, which does help with that, but we do not
use it consistently. If there are other problematic areas, I think the
best path forward is to use our framework more.
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Jeff King @ 2012-11-13 17:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <7vpq3hfpm7.fsf@alter.siamese.dyndns.org>
On Tue, Nov 13, 2012 at 08:13:04AM -0800, Junio C Hamano wrote:
> >> That's right, AUTHOR_IDENT would fall back to the default email and full name.
> >
> > Yeah, I find that somewhat questionable in the current behavior, and I'd
> > consider it a bug. Typically we prefer the committer ident when given a
> > choice (e.g., for writing reflog entries).
>
> Just to make sure I follow the discussion correctly, do you mean
> that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
> specified more concretely and we usually use COMMIITTER for this
> kind of thing in the first place but send-email does not in this
> case (I do not see "git var GIT_AUTHOR_IDENT" returning value from
> the implicit logic as a bug in this case---just making sure).
Having discussed more, I think there are two questionable things:
1. Preferring author over committer
2. Failing to fall back to committer when author is implicit or bogus
(because "git var" dies).
I think (1) may fall into the "this is not how I would do it today, but
it is not worth a possible regression" category.
I think (2) might be worth fixing, though. Certainly when the author is
bogus (by IDENT_STRICT rules), which I think was the original intent of
the "$repoauthor || $repocommitter" code. Probably when the author ident
is implicit, though that is more hazy to me.
> For a change with low benefit/cost ratio like this, the best course
> of action often is to stop paying attention to the thread that has
> become unproductive and venomous, and resurrect the topic after
> people forgot about it and doing it right yourself ;-).
I came to the same conclusion, but decided to simply do it right now
while it was in my head. Hopefully we can progress by reviewing the
series I just posted.
-Peff
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Erik Faye-Lund @ 2012-11-13 17:20 UTC (permalink / raw)
To: Jeff King
Cc: Felipe Contreras, git, Thomas Rast, Junio C Hamano,
Jonathan Nieder
In-Reply-To: <20121112233546.GG10531@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 12:35 AM, Jeff King <peff@peff.net> wrote:
> On Sun, Nov 11, 2012 at 06:06:50PM +0100, Felipe Contreras wrote:
>
>> There's no point in asking this over and over if the user already
>> properly configured his/her name and email.
>>
>> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
>> ---
>>
>> I got really tired of 'git send-email' always asking me from which address to send mails... that's already configured.
>
> It should be defaulting to your regular git ident, and you just have to
> hit enter, right?
>
> I think it's probably reasonable to skip that "enter" in most cases. But
> I'm not sure why we ever asked in the first place. What do people input
> there if they are not taking the default?
>
I usually input "Linus Torvalds <torvalds@linux-foundation.org>", to
give my patch high priority ;-)
^ permalink raw reply
* Re: [BUG] gitweb: XSS vulnerability of RSS feed
From: Jakub Narębski @ 2012-11-13 17:22 UTC (permalink / raw)
To: Jeff King
Cc: Drew Northup, glpk xypron, git, Junio C Hamano,
Jason J Pyeron CTR (US), Andreas Schwab
In-Reply-To: <20121113170452.GE20361@sigill.intra.peff.net>
On Tue, Nov 13, 2012 at 6:04 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 13, 2012 at 09:44:06AM -0500, Drew Northup wrote:
>> Besides, inserting one call to esc_html only fixes one attack path. I
>> didn't look to see if all others were already covered.
>
> Properly quoting output is something that the web framework should do
> for you. gitweb uses CGI.pm, which does help with that, but we do not
> use it consistently. If there are other problematic areas, I think the
> best path forward is to use our framework more.
Well, calling CGI.pm a _framework_ is overly generous, but it does
include some HTML generation subroutines / methods, and gitweb
makes use of them, especially $cgi->a() for links.
But it cannot help in this case, because here we are generating XML:
RSS or Atom feed. There was proposal some time ago to switch
to using XML::FeedPP or XML::Atom::Feed + XML::RSS::Feed for
feed generation.
Perhaps it is high time to switch to some Perl web (micro)framework,
like Dancer, Mojolicious or Catalyst... but not requiring extra modules
has its advantages (and there always exist Gitalist).
--
Jakub Narebski
^ permalink raw reply
* Re: [PATCH] send-email: add proper default sender
From: Junio C Hamano @ 2012-11-13 17:23 UTC (permalink / raw)
To: Jeff King; +Cc: Felipe Contreras, git, Thomas Rast, Jonathan Nieder
In-Reply-To: <20121113171445.GG20361@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Nov 13, 2012 at 08:13:04AM -0800, Junio C Hamano wrote:
>
>> >> That's right, AUTHOR_IDENT would fall back to the default email and full name.
>> >
>> > Yeah, I find that somewhat questionable in the current behavior, and I'd
>> > consider it a bug. Typically we prefer the committer ident when given a
>> > choice (e.g., for writing reflog entries).
>>
>> Just to make sure I follow the discussion correctly, do you mean
>> that the bug is that we pick a fuzzy AUTHOR when COMMITTER is
>> specified more concretely and we usually use COMMIITTER for this
>> kind of thing in the first place but send-email does not in this
>> case (I do not see "git var GIT_AUTHOR_IDENT" returning value from
>> the implicit logic as a bug in this case---just making sure).
>
> Having discussed more, I think there are two questionable things:
>
> 1. Preferring author over committer
>
> 2. Failing to fall back to committer when author is implicit or bogus
> (because "git var" dies).
>
> I think (1) may fall into the "this is not how I would do it today, but
> it is not worth a possible regression" category.
>
> I think (2) might be worth fixing, though. Certainly when the author is
> bogus (by IDENT_STRICT rules), which I think was the original intent of
> the "$repoauthor || $repocommitter" code. Probably when the author ident
> is implicit, though that is more hazy to me.
I agree with both points. Thanks.
^ permalink raw reply
* Re: [PATCH 5/6] Git.pm: teach "ident" to query explicitness
From: Jeff King @ 2012-11-13 17:25 UTC (permalink / raw)
To: Matt Kraai; +Cc: git
In-Reply-To: <20121113172300.GA16241@ftbfs.org>
On Tue, Nov 13, 2012 at 09:23:00AM -0800, Matt Kraai wrote:
> Minor nits:
>
> On Tue, Nov 13, 2012 at 11:53:20AM -0500, Jeff King wrote:
> > @@ -750,6 +750,10 @@ and either returns it as a scalar string or as an array with the fields parsed.
> > Alternatively, it can take a prepared ident string (e.g. from the commit
> > object) and just parse it.
> >
> > +If the C<explicit> option is set to 1, the returned array will contain an
> > +additional boolean specifying whether the ident was configure explicitly by the
>
> s/configure/configured/
Thanks.
> > if (wantarray) {
> > - return $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
> > + my @ret = $identstr =~ /^(.*) <(.*)> (\d+ [+-]\d{4})$/;
> > + if ($options{explicit} && defined $explicit) {
> > + push @ret, $explicit if defined $explicit;
>
> Isn't the test on this line redundant given that defined $explicit is
> already guaranteed by the condition on the previous line?
Yes, thanks (left over from an earlier attempt that tried to avoid
having $options{explicit}).
-Peff
^ permalink raw reply
* Re: Notes in format-patch
From: Junio C Hamano @ 2012-11-13 17:26 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <7vzk2le918.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> As the topic seems to be already in Peff's next, here is a trivial
> fix for this in incremental form.
>
> -- >8 --
> Subject: format-patch: add a blank line between notes and diffstat
>
> The last line of the note text comes immediately before the diffstat
> block, making the latter unnecessarily harder to view.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> log-tree.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git i/log-tree.c w/log-tree.c
> index 712a22b..9303fd8 100644
> --- i/log-tree.c
> +++ w/log-tree.c
> @@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
> opt->shown_dashes = 1;
> }
> strbuf_addstr(&msgbuf, ctx.notes_message);
> + strbuf_addch(&msgbuf, '\n');
> }
>
> if (opt->show_log_size) {
... and it is broken X-<.
The blank line should be added before the diffstat, not after the
notes message (t3307 shows a case where we give notes without
diffstat, and we shouldn't be adding an extra blank line in that
case.
^ permalink raw reply
* What's cooking in git.git (Nov 2012, #03; Tue, 13)
From: Jeff King @ 2012-11-13 17:52 UTC (permalink / raw)
To: git
What's cooking in git.git (Nov 2012, #03; Tue, 13)
--------------------------------------------------
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.
This is my final "what's cooking" as interim maintainer. I didn't
graduate anything to master, but I updated my plans for each topic to
give Junio an idea of where I was.
You can find the changes described here in the integration branches of
my repository at:
git://github.com/peff/git.git
Until Junio returns, kernel.org and the other "usual" places will not be
updated.
--------------------------------------------------
[New Topics]
* jk/maint-gitweb-xss (2012-11-12) 1 commit
- gitweb: escape html in rss title
Fixes an XSS vulnerability in gitweb.
Will merge to 'next'.
* jk/send-email-sender-prompt (2012-11-13) 6 commits
- send-email: do not prompt for explicit repo ident
- Git.pm: teach "ident" to query explicitness
- var: provide explicit/implicit ident information
- var: accept multiple variables on the command line
- ident: keep separate "explicit" flags for author and committer
- ident: make user_ident_explicitly_given private
Avoid annoying sender prompt in git-send-email, but only when it is
safe to do so.
Needs review.
* mg/replace-resolve-delete (2012-11-13) 1 commit
- replace: parse revision argument for -d
Be more user friendly to people using "git replace -d".
Will merge to 'next'.
* ml/cygwin-mingw-headers (2012-11-12) 1 commit
- Update cygwin.c for new mingw-64 win32 api headers
Make git work on newer cygwin.
Will merge to 'next'.
--------------------------------------------------
[Stalled]
* rc/maint-complete-git-p4 (2012-09-24) 1 commit
(merged to 'next' on 2012-10-29 at af52cef)
+ Teach git-completion about git p4
Comment from Pete will need to be addressed in a follow-up patch.
* as/test-tweaks (2012-09-20) 7 commits
- tests: paint unexpectedly fixed known breakages in bold red
- tests: test the test framework more thoroughly
- [SQUASH] t/t0000-basic.sh: quoting of TEST_DIRECTORY is screwed up
- tests: refactor mechanics of testing in a sub test-lib
- tests: paint skipped tests in bold blue
- tests: test number comes first in 'not ok $count - $message'
- tests: paint known breakages in bold yellow
Various minor tweaks to the test framework to paint its output
lines in colors that match what they mean better.
Has the "is this really blue?" issue Peff raised resolved???
* jc/maint-name-rev (2012-09-17) 7 commits
- describe --contains: use "name-rev --algorithm=weight"
- name-rev --algorithm=weight: tests and documentation
- name-rev --algorithm=weight: cache the computed weight in notes
- name-rev --algorithm=weight: trivial optimization
- name-rev: --algorithm option
- name_rev: clarify the logic to assign a new tip-name to a commit
- name-rev: lose unnecessary typedef
"git name-rev" names the given revision based on a ref that can be
reached in the smallest number of steps from the rev, but that is
not useful when the caller wants to know which tag is the oldest one
that contains the rev. This teaches a new mode to the command that
uses the oldest ref among those which contain the rev.
I am not sure if this is worth it; for one thing, even with the help
from notes-cache, it seems to make the "describe --contains" even
slower. Also the command will be unusably slow for a user who does
not have a write access (hence unable to create or update the
notes-cache).
Stalled mostly due to lack of responses.
* jc/xprm-generation (2012-09-14) 1 commit
- test-generation: compute generation numbers and clock skews
A toy to analyze how bad the clock skews are in histories of real
world projects.
Stalled mostly due to lack of responses.
* jc/blame-no-follow (2012-09-21) 2 commits
- blame: pay attention to --no-follow
- diff: accept --no-follow option
Teaches "--no-follow" option to "git blame" to disable its
whole-file rename detection.
Stalled mostly due to lack of responses.
* jc/doc-default-format (2012-10-07) 2 commits
- [SQAUSH] allow "cd Doc* && make DEFAULT_DOC_TARGET=..."
- Allow generating a non-default set of documentation
Need to address the installation half if this is to be any useful.
* mk/maint-graph-infinity-loop (2012-09-25) 1 commit
- graph.c: infinite loop in git whatchanged --graph -m
The --graph code fell into infinite loop when asked to do what the
code did not expect ;-)
Anybody who worked on "--graph" wants to comment?
Stalled mostly due to lack of responses.
* jc/add-delete-default (2012-08-13) 1 commit
- git add: notice removal of tracked paths by default
"git add dir/" updated modified files and added new files, but does
not notice removed files, which may be "Huh?" to some users. They
can of course use "git add -A dir/", but why should they?
Resurrected from graveyard, as I thought it was a worthwhile thing
to do in the longer term.
Waiting for comments.
* mb/remote-default-nn-origin (2012-07-11) 6 commits
- Teach get_default_remote to respect remote.default.
- Test that plain "git fetch" uses remote.default when on a detached HEAD.
- Teach clone to set remote.default.
- Teach "git remote" about remote.default.
- Teach remote.c about the remote.default configuration setting.
- Rename remote.c's default_remote_name static variables.
When the user does not specify what remote to interact with, we
often attempt to use 'origin'. This can now be customized via a
configuration variable.
Expecting a re-roll.
"The first remote becomes the default" bit is better done as a
separate step.
* mh/ceiling (2012-10-29) 8 commits
- string_list_longest_prefix(): remove function
- setup_git_directory_gently_1(): resolve symlinks in ceiling paths
- longest_ancestor_length(): require prefix list entries to be normalized
- longest_ancestor_length(): take a string_list argument for prefixes
- longest_ancestor_length(): use string_list_split()
- Introduce new function real_path_if_valid()
- real_path_internal(): add comment explaining use of cwd
- Introduce new static function real_path_internal()
Elements of GIT_CEILING_DIRECTORIES list may not match the real
pathname we obtain from getcwd(), leading the GIT_DIR discovery
logic to escape the ceilings the user thought to have specified.
--------------------------------------------------
[Cooking]
* mo/cvs-server-updates (2012-10-16) 10 commits
- cvsserver Documentation: new cvs ... -r support
- cvsserver: add t9402 to test branch and tag refs
- cvsserver: support -r and sticky tags for most operations
- cvsserver: Add version awareness to argsfromdir
- cvsserver: generalize getmeta() to recognize commit refs
- cvsserver: implement req_Sticky and related utilities
- cvsserver: add misc commit lookup, file meta data, and file listing functions
- cvsserver: define a tag name character escape mechanism
- cvsserver: cleanup extra slashes in filename arguments
- cvsserver: factor out git-log parsing logic
Needs review by folks interested in cvsserver.
* ta/doc-cleanup (2012-10-25) 6 commits
(merged to 'next' on 2012-11-13 at e11fafd)
+ Documentation: build html for all files in technical and howto
+ Documentation/howto: convert plain text files to asciidoc
+ Documentation/technical: convert plain text files to asciidoc
+ Change headline of technical/send-pack-pipeline.txt to not confuse its content with content from git-send-pack.txt
+ Shorten two over-long lines in git-bisect-lk2009.txt by abbreviating some sha1
+ Split over-long synopsis in git-fetch-pack.txt into several lines
Will merge to 'master' in the sixth batch.
* lt/diff-stat-show-0-lines (2012-10-17) 1 commit
- Fix "git diff --stat" for interesting - but empty - file changes
We failed to mention a file without any content change but whose
permission bit was modified, or (worse yet) a new file without any
content in the "git diff --stat" output.
Needs some test updates.
* jc/prettier-pretty-note (2012-10-26) 11 commits
(merged to 'next' on 2012-11-04 at 40e3e48)
+ Doc User-Manual: Patch cover letter, three dashes, and --notes
+ Doc format-patch: clarify --notes use case
+ Doc notes: Include the format-patch --notes option
+ Doc SubmittingPatches: Mention --notes option after "cover letter"
+ Documentation: decribe format-patch --notes
+ format-patch --notes: show notes after three-dashes
+ format-patch: append --signature after notes
+ pretty_print_commit(): do not append notes message
+ pretty: prepare notes message at a centralized place
+ format_note(): simplify API
+ pretty: remove reencode_commit_message()
Now that Philip has submitted some documentation updates, this is
looking more ready.
Will merge to 'master' in the fifth batch.
* jc/same-encoding (2012-11-04) 1 commit
(merged to 'next' on 2012-11-04 at 54991f2)
+ reencode_string(): introduce and use same_encoding()
Various codepaths checked if two encoding names are the same using
ad-hoc code and some of them ended up asking iconv() to convert
between "utf8" and "UTF-8". The former is not a valid way to spell
the encoding name, but often people use it by mistake, and we
equated them in some but not all codepaths. Introduce a new helper
function to make these codepaths consistent.
Will merge to 'master' in the fifth batch.
* cr/cvsimport-local-zone (2012-11-04) 2 commits
(merged to 'next' on 2012-11-04 at 292f0b4)
+ cvsimport: work around perl tzset issue
+ git-cvsimport: allow author-specific timezones
Allows "cvsimport" to read per-author timezone from the author info
file.
Will merge to 'master' in the fifth batch.
* fc/zsh-completion (2012-10-29) 3 commits
- completion: add new zsh completion
- completion: add new __gitcompadd helper
- completion: get rid of empty COMPREPLY assignments
There were some comments on this, but I wasn't clear on the outcome.
Need to take a closer look.
* jc/apply-trailing-blank-removal (2012-10-12) 1 commit
- apply.c:update_pre_post_images(): the preimage can be truncated
Fix to update_pre_post_images() that did not take into account the
possibility that whitespace fix could shrink the preimage and
change the number of lines in it.
Extra set of eyeballs appreciated.
* jn/warn-on-inaccessible-loosen (2012-10-14) 4 commits
- config: exit on error accessing any config file
- doc: advertise GIT_CONFIG_NOSYSTEM
- config: treat user and xdg config permission problems as errors
- config, gitignore: failure to access with ENOTDIR is ok
An RFC to deal with a situation where .config/git is a file and we
notice .config/git/config is not readable due to ENOTDIR, not
ENOENT; I think a bit more refactored approach to consistently
address permission errors across config, exclude and attrs is
desirable. Don't we also need a check for an opposite situation
where we open .config/git/config or .gitattributes for reading but
they turn out to be directories?
* as/check-ignore (2012-11-08) 14 commits
- t0007: fix tests on Windows
- Documentation/check-ignore: we show the deciding match, not the first
- Add git-check-ignore sub-command
- dir.c: provide free_directory() for reclaiming dir_struct memory
- pathspec.c: move reusable code from builtin/add.c
- dir.c: refactor treat_gitlinks()
- dir.c: keep track of where patterns came from
- dir.c: refactor is_path_excluded()
- dir.c: refactor is_excluded()
- dir.c: refactor is_excluded_from_list()
- dir.c: rename excluded() to is_excluded()
- dir.c: rename excluded_from_list() to is_excluded_from_list()
- dir.c: rename path_excluded() to is_path_excluded()
- dir.c: rename cryptic 'which' variable to more consistent name
Duy helped to reroll this.
Expecting a re-roll.
* so/prompt-command (2012-10-17) 4 commits
(merged to 'next' on 2012-10-25 at 79565a1)
+ coloured git-prompt: paint detached HEAD marker in red
+ Fix up colored git-prompt
+ show color hints based on state of the git tree
+ Allow __git_ps1 to be used in PROMPT_COMMAND
Updates __git_ps1 so that it can be used as $PROMPT_COMMAND,
instead of being used for command substitution in $PS1, to embed
color escape sequences in its output.
Will cook in 'next'.
* aw/rebase-am-failure-detection (2012-10-11) 1 commit
- rebase: Handle cases where format-patch fails
I am unhappy a bit about the possible performance implications of
having to store the output in a temporary file only for a rare case
of format-patch aborting.
* nd/wildmatch (2012-10-15) 13 commits
(merged to 'next' on 2012-10-25 at 510e8df)
+ Support "**" wildcard in .gitignore and .gitattributes
+ wildmatch: make /**/ match zero or more directories
+ wildmatch: adjust "**" behavior
+ wildmatch: fix case-insensitive matching
+ wildmatch: remove static variable force_lower_case
+ wildmatch: make wildmatch's return value compatible with fnmatch
+ t3070: disable unreliable fnmatch tests
+ Integrate wildmatch to git
+ wildmatch: follow Git's coding convention
+ wildmatch: remove unnecessary functions
+ Import wildmatch from rsync
+ ctype: support iscntrl, ispunct, isxdigit and isprint
+ ctype: make sane_ctype[] const array
Allows pathname patterns in .gitignore and .gitattributes files
with double-asterisks "foo/**/bar" to match any number of directory
hierarchies.
I suspect that this needs to be plugged to pathspec matching code;
otherwise "git log -- 'Docum*/**/*.txt'" would not show the log for
commits that touch Documentation/git.txt, which would be confusing
to the users.
Will cook in 'next'.
* jk/lua-hackery (2012-10-07) 6 commits
- pretty: fix up one-off format_commit_message calls
- Minimum compilation fixup
- Makefile: make "lua" a bit more configurable
- add a "lua" pretty format
- add basic lua infrastructure
- pretty: make some commit-parsing helpers more public
Interesting exercise. When we do this for real, we probably would want
to wrap a commit to make it more like an "object" with methods like
"parents", etc.
* nd/pretty-placeholder-with-color-option (2012-09-30) 9 commits
. pretty: support %>> that steal trailing spaces
. pretty: support truncating in %>, %< and %><
. pretty: support padding placeholders, %< %> and %><
. pretty: two phase conversion for non utf-8 commits
. utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences
. utf8.c: move display_mode_esc_sequence_len() for use by other functions
. pretty: support %C(auto[,N]) to turn on coloring on next placeholder(s)
. pretty: split parsing %C into a separate function
. pretty: share code between format_decoration and show_decorations
This causes warnings with -Wuninitialized, so I've ejected it from pu
for the time being.
* jc/maint-fetch-tighten-refname-check (2012-10-19) 1 commit
(merged to 'next' on 2012-11-04 at eda85ef)
+ get_fetch_map(): tighten checks on dest refs
This was split out from discarded jc/maint-push-refs-all topic.
Will merge to 'master' in the fifth batch.
* jh/symbolic-ref-d (2012-10-21) 1 commit
(merged to 'next' on 2012-11-04 at b0762f5)
+ git symbolic-ref --delete $symref
Add "symbolic-ref -d SYM" to delete a symbolic ref SYM.
It is already possible to remove a symbolic ref with "update-ref -d
--no-deref", but it may be a good addition for completeness.
Will merge to 'master' in the fifth batch.
* jh/update-ref-d-through-symref (2012-10-21) 2 commits
- Fix failure to delete a packed ref through a symref
- t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
"update-ref -d --deref SYM" to delete a ref through a symbolic ref
that points to it did not remove it correctly.
Need to check reviews, but is probably ready for 'next'.
* jk/config-ignore-duplicates (2012-10-29) 9 commits
(merged to 'next' on 2012-10-29 at 67fa0a2)
+ builtin/config.c: Fix a sparse warning
(merged to 'next' on 2012-10-25 at 233df08)
+ git-config: use git_config_with_options
+ git-config: do not complain about duplicate entries
+ git-config: collect values instead of immediately printing
+ git-config: fix regexp memory leaks on error conditions
+ git-config: remove memory leak of key regexp
+ t1300: test "git config --get-all" more thoroughly
+ t1300: remove redundant test
+ t1300: style updates
Drop duplicate detection from git-config; this lets it
better match the internal config callbacks, which clears up
some corner cases with includes.
Will merge to 'master' in the sixth batch.
* ph/submodule-sync-recursive (2012-10-29) 2 commits
(merged to 'next' on 2012-11-04 at a000f78)
+ Add tests for submodule sync --recursive
+ Teach --recursive to submodule sync
Adds "--recursive" option to submodule sync.
Will merge to 'master' in the fifth batch.
* fc/completion-test-simplification (2012-10-29) 2 commits
- completion: simplify __gitcomp test helper
- completion: refactor __gitcomp related tests
Clean up completion tests.
There were some comments on the list.
Expecting a re-roll.
* fc/remote-testgit-feature-done (2012-10-29) 1 commit
- remote-testgit: properly check for errors
Needs review.
* jk/maint-diff-grep-textconv (2012-10-28) 1 commit
(merged to 'next' on 2012-11-04 at 790337b)
+ diff_grep: use textconv buffers for add/deleted files
(this branch is used by jk/pickaxe-textconv.)
Fixes inconsistent use of textconv with "git log -G".
Will merge to 'master' in the fifth batch.
* jk/pickaxe-textconv (2012-10-28) 2 commits
- pickaxe: use textconv for -S counting
- pickaxe: hoist empty needle check
(this branch uses jk/maint-diff-grep-textconv.)
Use textconv filters when searching with "log -S".
Waiting for a sanity check and review from Junio.
* as/maint-doc-fix-no-post-rewrite (2012-11-02) 1 commit
(merged to 'next' on 2012-11-09 at 117a91e)
+ commit: fixup misplacement of --no-post-rewrite description
Will merge to 'master' in the fifth batch.
* fc/remote-bzr (2012-11-08) 5 commits
- remote-bzr: update working tree
- remote-bzr: add support for remote repositories
- remote-bzr: add support for pushing
- remote-bzr: add simple tests
- Add new remote-bzr transport helper
New remote helper for bzr.
Will merge to 'next'.
* fc/remote-hg (2012-11-12) 20 commits
- remote-hg: avoid bad refs
- remote-hg: try the 'tip' if no checkout present
- remote-hg: fix compatibility with older versions of hg
- remote-hg: add missing config for basic tests
- remote-hg: the author email can be null
- remote-hg: add option to not track branches
- remote-hg: add extra author test
- remote-hg: add tests to compare with hg-git
- remote-hg: add bidirectional tests
- test-lib: avoid full path to store test results
- remote-hg: add basic tests
- remote-hg: fake bookmark when there's none
- remote-hg: add compat for hg-git author fixes
- remote-hg: add support for hg-git compat mode
- remote-hg: match hg merge behavior
- remote-hg: make sure the encoding is correct
- remote-hg: add support to push URLs
- remote-hg: add support for remote pushing
- remote-hg: add support for pushing
- Add new remote-hg transport helper
New remote helper for hg.
Will merge to 'next'.
* jk/maint-http-half-auth-fetch (2012-10-31) 2 commits
(merged to 'next' on 2012-11-09 at af69926)
+ remote-curl: retry failed requests for auth even with gzip
+ remote-curl: hoist gzip buffer size to top of post_rpc
Fixes fetch from servers that ask for auth only during the actual
packing phase. This is not really a recommended configuration, but it
cleans up the code at the same time.
Will merge to 'master' in the sixth batch.
* js/hp-nonstop (2012-10-30) 1 commit
(merged to 'next' on 2012-11-09 at fe58205)
+ fix 'make test' for HP NonStop
Will merge to 'master' in the fifth batch.
* kb/preload-index-more (2012-11-02) 1 commit
(merged to 'next' on 2012-11-09 at a750ebd)
+ update-index/diff-index: use core.preloadindex to improve performance
Use preloadindex in more places, which has a nice speedup on systems
with slow stat calls (and even on Linux).
Will merge to 'master' in the sixth batch.
* mh/notes-string-list (2012-11-08) 5 commits
(merged to 'next' on 2012-11-09 at 7a4c58c)
+ string_list_add_refs_from_colon_sep(): use string_list_split()
+ notes: fix handling of colon-separated values
+ combine_notes_cat_sort_uniq(): sort and dedup lines all at once
+ Initialize sort_uniq_list using named constant
+ string_list: add a function string_list_remove_empty_items()
Improve the asymptotic performance of the cat_sort_uniq notes merge
strategy.
Will merge to 'master' in the fifth batch.
* mh/strbuf-split (2012-11-04) 4 commits
(merged to 'next' on 2012-11-09 at fa984b1)
+ strbuf_split*(): document functions
+ strbuf_split*(): rename "delim" parameter to "terminator"
+ strbuf_split_buf(): simplify iteration
+ strbuf_split_buf(): use ALLOC_GROW()
Cleanups and documentation for strbuf_split.
Will merge to 'master' in the fifth batch.
* mm/maint-doc-commit-edit (2012-11-02) 1 commit
(merged to 'next' on 2012-11-09 at 8dab7f5)
+ Document 'git commit --no-edit' explicitly
Will merge to 'master' in the fifth batch.
* cr/push-force-tag-update (2012-11-12) 5 commits
- push: update remote tags only with force
- push: flag updates that require force
- push: flag updates
- push: add advice for rejected tag reference
- push: return reject reasons via a mask
Require "-f" for push to update a tag, even if it is a fast-forward.
Needs review.
I'm undecided yet on whether the goal is the right thing to do, but it
does prevent some potential mistakes. I haven't looked closely at the
implementation itself; review from interested parties would be helpful.
* fc/fast-export-fixes (2012-11-08) 14 commits
- fast-export: don't handle uninteresting refs
- fast-export: make sure updated refs get updated
- fast-export: fix comparison in tests
- fast-export: trivial cleanup
- remote-testgit: make clear the 'done' feature
- remote-testgit: report success after an import
- remote-testgit: exercise more features
- remote-testgit: cleanup tests
- remote-testgit: remove irrelevant test
- remote-testgit: get rid of non-local functionality
- Add new simplified git-remote-testgit
- Rename git-remote-testgit to git-remote-testpy
- remote-testgit: fix direction of marks
- fast-export: avoid importing blob marks
Improvements to fix fast-export bugs, including how refs pointing to
already-seen commits are handled. An earlier 4-commit version of this
series looked good to me, but this much-expanded version has not seen
any comments.
Looks like it has been re-rolled, but I haven't checked it out yet.
Needs review.
* mg/maint-pull-suggest-upstream-to (2012-11-08) 1 commit
(merged to 'next' on 2012-11-13 at bd74252)
+ push/pull: adjust missing upstream help text to changed interface
Follow-on to the new "--set-upstream-to" topic from v1.8.0 to avoid
suggesting the deprecated "--set-upstream".
Will merge to 'master' in the fifth batch.
* mh/alt-odb-string-list-cleanup (2012-11-08) 2 commits
(merged to 'next' on 2012-11-13 at 2bf41d9)
+ link_alt_odb_entries(): take (char *, len) rather than two pointers
+ link_alt_odb_entries(): use string_list_split_in_place()
Cleanups in the alternates code. Fixes a potential bug and makes the
code much cleaner.
Will merge to 'master' in the sixth batch.
* pf/editor-ignore-sigint (2012-11-11) 5 commits
- launch_editor: propagate SIGINT from editor to git
- run-command: do not warn about child death by SIGINT
- run-command: drop silent_exec_failure arg from wait_or_whine
- launch_editor: ignore SIGINT while the editor has control
- launch_editor: refactor to use start/finish_command
Avoid confusing cases where the user hits Ctrl-C while in the editor
session, not realizing git will receive the signal. Since most editors
will take over the terminal and will block SIGINT, this is not likely
to confuse anyone.
Some people raised issues with emacsclient, which are addressed by this
re-roll. It should probably also handle SIGQUIT, and there were a
handful of other review comments.
Expecting a re-roll.
* pp/gitweb-config-underscore (2012-11-08) 1 commit
- gitweb: make remote_heads config setting work
The key "gitweb.remote_heads" is not legal git config; this maps it to
"gitweb.remoteheads".
Junio raised a good point about the implementation for three-level
variables.
Expecting a re-roll.
* pw/maint-p4-rcs-expansion-newline (2012-11-08) 1 commit
(merged to 'next' on 2012-11-13 at e90cc7c)
+ git p4: RCS expansion should not span newlines
I do not have p4 to play with, but looks obviously correct to me.
Will merge to 'master' in the sixth batch.
* rh/maint-gitweb-highlight-ext (2012-11-08) 1 commit
(merged to 'next' on 2012-11-13 at c57d856)
+ gitweb.perl: fix %highlight_ext mappings
Fixes a clever misuse of perl's list interpretation.
Will merge to 'master' in the sixth batch.
* rr/submodule-diff-config (2012-11-08) 3 commits
- submodule: display summary header in bold
- diff: introduce diff.submodule configuration variable
- Documentation: move diff.wordRegex from config.txt to diff-config.txt
Lets "git diff --submodule=log" become the default via configuration.
Almost there. Looks like a new version has been posted, but I haven't
picked it up yet.
Needs review.
^ permalink raw reply
* Re: [PATCH 14/13] test-wildmatch: avoid Windows path mangling
From: Johannes Sixt @ 2012-11-13 18:06 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1352801169-10501-1-git-send-email-pclouds@gmail.com>
Am 13.11.2012 11:06, schrieb Nguyễn Thái Ngọc Duy:
> On Windows, arguments starting with a forward slash is mangled as if
> it were full pathname. This causes the patterns beginning with a slash
> not to be passed to test-wildmatch correctly. Avoid mangling by never
> accepting patterns starting with a slash. Those arguments must be
> rewritten with a leading "XXX" (e.g. "/abc" becomes "XXX/abc"), which
> will be removed by test-wildmatch itself before feeding the patterns
> to wildmatch() or fnmatch().
>
> Reported-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> On Sun, Nov 11, 2012 at 5:47 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > The title taken together with the above explanation makes it sound
> > as if wildmatch code does not work with the pattern /foo on Windows
> > at all and to avoid the issue (instead of fixing the breakage) this
> > patch removes such tests....
>
> OK how about this?
Thanks, the patch lets the tests pass on Windows.
-- Hannes
^ permalink raw reply
* Re: [PATCH 15/13] compat/fnmatch: fix off-by-one character class's length check
From: Johannes Sixt @ 2012-11-13 18:07 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, Jeff King
In-Reply-To: <1352628837-5784-2-git-send-email-pclouds@gmail.com>
Am 11.11.2012 11:13, schrieb Nguyễn Thái Ngọc Duy:
> - if (c1 == CHAR_CLASS_MAX_LENGTH)
> + if (c1 > CHAR_CLASS_MAX_LENGTH)
Nice catch! With this one and 14/13, all tests in t3070 pass on Windows.
-- Hannes
^ permalink raw reply
* Re: Notes in format-patch
From: Junio C Hamano @ 2012-11-13 18:09 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git, Jeff King
In-Reply-To: <7vr4nxe7nf.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ... and it is broken X-<.
>
> The blank line should be added before the diffstat, not after the
> notes message (t3307 shows a case where we give notes without
> diffstat, and we shouldn't be adding an extra blank line in that
> case.
Second try.
-- >8 --
Subject: format-patch: add a blank line between notes and diffstat
The last line of the note text comes immediately before the diffstat
block, making the latter unnecessarily harder to view.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
log-tree.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git c/log-tree.c w/log-tree.c
index 712a22b..4f86def 100644
--- c/log-tree.c
+++ w/log-tree.c
@@ -727,15 +727,16 @@ int log_tree_diff_flush(struct rev_info *opt)
}
if (opt->loginfo && !opt->no_commit_id) {
- /* When showing a verbose header (i.e. log message),
- * and not in --pretty=oneline format, we would want
- * an extra newline between the end of log and the
- * output for readability.
- */
show_log(opt);
if ((opt->diffopt.output_format & ~DIFF_FORMAT_NO_OUTPUT) &&
opt->verbose_header &&
opt->commit_format != CMIT_FMT_ONELINE) {
+ /*
+ * When showing a verbose header (i.e. log message),
+ * and not in --pretty=oneline format, we would want
+ * an extra newline between the end of log and the
+ * diff/diffstat output for readability.
+ */
int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
if (opt->diffopt.output_prefix) {
struct strbuf *msg = NULL;
@@ -743,11 +744,21 @@ int log_tree_diff_flush(struct rev_info *opt)
opt->diffopt.output_prefix_data);
fwrite(msg->buf, msg->len, 1, stdout);
}
- if (!opt->shown_dashes) {
- if ((pch & opt->diffopt.output_format) == pch)
- printf("---");
- putchar('\n');
- }
+
+ /*
+ * We may have shown three-dashes line early
+ * between notes and the log message, in which
+ * case we only want a blank line after the
+ * notes without (an extra) three-dashes line.
+ * Otherwise, we show the three-dashes line if
+ * we are showing the patch with diffstat, but
+ * in that case, there is no extra blank line
+ * after the three-dashes line.
+ */
+ if (!opt->shown_dashes &&
+ (pch & opt->diffopt.output_format) == pch)
+ printf("---");
+ putchar('\n');
}
}
diff_flush(&opt->diffopt);
^ permalink raw reply related
* Re: RFD: fast-import is picky with author names (and maybe it should - but how much so?)
From: Felipe Contreras @ 2012-11-13 18:15 UTC (permalink / raw)
To: Michael J Gruber
Cc: Jeff King, A Large Angry SCM, Git Mailing List, Junio C Hamano
In-Reply-To: <50A21DB9.7070700@drmicha.warpmail.net>
On Tue, Nov 13, 2012 at 11:15 AM, Michael J Gruber
<git@drmicha.warpmail.net> wrote:
> Felipe Contreras venit, vidit, dixit 12.11.2012 23:47:
>> On Mon, Nov 12, 2012 at 10:41 PM, Jeff King <peff@peff.net> wrote:
>>> On Sun, Nov 11, 2012 at 07:48:14PM +0100, Felipe Contreras wrote:
>>>
>>>>> 3. Exporters should not use it if they have any broken-down
>>>>> representation at all. Even knowing that the first half is a human
>>>>> name and the second half is something else would give it a better
>>>>> shot at cleaning than fast-import would get.
>>>>
>>>> I'm not sure what you mean by this. If they have name and email, then
>>>> sure, it's easy.
>>>
>>> But not as easy as just printing it. What if you have this:
>>>
>>> name="Peff <angle brackets> King"
>>> email="<peff@peff.net>"
>>>
>>> Concatenating them does not produce a valid git author name. Sending the
>>> concatenation through fast-import's cleanup function would lose
>>> information (namely, the location of the boundary between name and
>>> email).
>>
>> Right. Unfortunately I'm not aware of any DSCM that does that.
>>
>>> Similarly, one might have other structured data (e.g., CVS username)
>>> where the structure is a useful hint, but some conversion to name+email
>>> is still necessary.
>>
>> CVS might be the only one that has such structured data. I think in
>> subversion the username has no meaning. A 'felipec' subversion
>> username is as bad as a mercurial 'felipec' username.
>
> In subversion, the username has the clearly defined meaning of being a
> username on the subversion host. If the host is, e.g., a sourceforge
> site then I can easily look up the user profile and convert the username
> into a valid e-mail address (<username>@users.sf.net). That is the
> advantage that the exporter (together with user knowledge) has over the
> importer.
>
> If the initial clone process aborts after every single "unknown" user
> it's no fun, of course. On the other hand, if an incremental clone
> (fetch) let's commits with unknown author sneak in it's no fun either
> (because I may want to fetch in crontab and publish that converted beast
> automatically). That is why I proposed neither approach.
>
> Most conveniently, the export side of a remote helper would
>
> - do "obvious" automatic lossless transformations
> - use an author map for other names
This should be done by fast-import. It doesn't make any sense that
every remote helper and fast-exporter out there have their own way of
mapping authors (or none).
> - For names not covered by the above (or having an empty map entry):
> Stop exporting commits but continue parsing commits and amend the author
> map with any unknown usernames (empty entry), and warn the user.
> (crontab script can notify me based on the return code.)
Stop exporting commits but continue parsing commits? I don't know what
that means.
fast-import should try it's best to clean it up, warn the user, sure,
but also store the missing entry on a file, so that it can be filed
later (if the user so wishes).
> If the cloning involves a "foreign clone" (like the hg clone behind the
> scene) then the runtime of the second pass should be much smaller. In
> principle, one could even store all blobs and trees on the first run and
> skip that step on the second, but that would rely on immutability on the
> foreign side, so I dunno. (And to check the sha1, we have to get the
> blob anyways.)
No. There's no concept of partial clones... Either you clone, or you don't.
Wait if the remote helper didn't notice that the author was bad?
fast-import could just just leave everything up to that point, warn
abut what happened, and exit, but the exporter side would die in the
middle of exporting, and it might end up in a bad state, not saving
marks, or who knows what.
It wouldn't work.
The cloning should be full, and the bad authors stored in a scaffold author map.
> As for the format for incomplete entries (foo <some@where>), a technical
> guideline should suffice for those that follow guidelines.
fast-import should do that.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: [PATCH] Add tcsh-completion support to contrib by using git-completion.bash
From: Felipe Contreras @ 2012-11-13 18:31 UTC (permalink / raw)
To: Marc Khouzam; +Cc: git
In-Reply-To: <CAFj1UpFd9X8Jq5o7B4m35i=merBDvOo4NOtwth=UnG2S5X_rGw@mail.gmail.com>
On Mon, Nov 12, 2012 at 9:07 PM, Marc Khouzam <marc.khouzam@gmail.com> wrote:
> this patch allows tcsh-users to get the benefits of the awesome
> git-completion.bash script. It could also help other shells do the same.
Maybe you can try to take a look at the same for zsh:
http://article.gmane.org/gmane.comp.version-control.git/208173
> ---
> contrib/completion/git-completion.bash | 53 +++++++++++++++++++++++++++++++-
> contrib/completion/git-completion.tcsh | 34 ++++++++++++++++++++
> 2 files changed, 86 insertions(+), 1 deletions(-)
> create mode 100755 contrib/completion/git-completion.tcsh
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index be800e0..6d4b57a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1,4 +1,6 @@
> -#!bash
> +#!/bin/bash
> +# The above line is important as this script can be executed when used
> +# with another shell such as tcsh
> #
> # bash/zsh completion support for core Git.
> #
> @@ -2481,3 +2483,52 @@ __git_complete gitk __gitk_main
> if [ Cygwin = "$(uname -o 2>/dev/null)" ]; then
> __git_complete git.exe __git_main
> fi
> +
> +# Method that will output the result of the completion done by
> +# the bash completion script, so that it can be re-used in another
> +# context than the bash complete command.
> +# It accepts 1 to 2 arguments:
> +# 1: The command-line to complete
> +# 2: The index of the word within argument #1 in which the cursor is
> +# located (optional). If parameter 2 is not provided, it will be
> +# determined as best possible using parameter 1.
> +_git_complete_with_output ()
> +{
> + # Set COMP_WORDS to the command-line as bash would.
> + COMP_WORDS=($1)
> +
> + # Set COMP_CWORD to the cursor location as bash would.
> + if [ -n "$2" ]; then
> + COMP_CWORD=$2
> + else
> + # Assume the cursor is at the end of parameter #1.
> + # We must check for a space as the last character which will
> + # tell us that the previous word is complete and the cursor
> + # is on the next word.
> + if [ "${1: -1}" == " " ]; then
> + # The last character is a space, so our
> location is at the end
> + # of the command-line array
> + COMP_CWORD=${#COMP_WORDS[@]}
> + else
> + # The last character is not a space, so our
> location is on the
> + # last word of the command-line array, so we
> must decrement the
> + # count by 1
> + COMP_CWORD=$((${#COMP_WORDS[@]}-1))
> + fi
> + fi
> +
> + # Call _git() or _gitk() of the bash script, based on the first
> + # element of the command-line
> + _${COMP_WORDS[0]}
You might want to use __${COMP_WORDS[0]}_main instead.
> +
> + # Print the result that is stored in the bash variable ${COMPREPLY}
> + for i in ${COMPREPLY[@]}; do
> + echo "$i"
> + done
> +}
> +
> +if [ -n "$1" ] ; then
> + # If there is an argument, we know the script is being executed
> + # so go ahead and run the _git_complete_with_output function
> + _git_complete_with_output "$1" "$2"
> +fi
Why do you need this function in this file? You can very easily add
this function to git-completion.tcsh.
> diff --git a/contrib/completion/git-completion.tcsh
> b/contrib/completion/git-completion.tcsh
> new file mode 100755
> index 0000000..7b7baea
> --- /dev/null
> +++ b/contrib/completion/git-completion.tcsh
> @@ -0,0 +1,34 @@
> +#!tcsh
> +#
> +# tcsh completion support for core Git.
> +#
> +# Copyright (C) 2012 Marc Khouzam <marc.khouzam@gmail.com>
> +# Distributed under the GNU General Public License, version 2.0.
> +#
> +# This script makes use of the git-completion.bash script to
> +# determine the proper completion for git commands under tcsh.
> +#
> +# To use this completion script:
> +#
> +# 1) Copy both this file and the bash completion script to your
> ${HOME} directory
> +# using the names ${HOME}/.git-completion.tcsh and
> ${HOME}/.git-completion.bash.
> +# 2) Add the following line to your .tcshrc/.cshrc:
> +# source ${HOME}/.git-completion.tcsh
> +
> +# One can change the below line to use a different location
> +set __git_tcsh_completion_script = ${HOME}/.git-completion.bash
> +
> +# Check that the user put the script in the right place
> +if ( ! -e ${__git_tcsh_completion_script} ) then
> + echo "ERROR in git-completion.tcsh script. Cannot find:
> ${__git_tcsh_completion_script}. Git completion will not work."
> + exit
> +endif
> +
> +# Make the script executable if it is not
> +if ( ! -x ${__git_tcsh_completion_script} ) then
> + chmod u+x ${__git_tcsh_completion_script}
> +endif
Why not just source it?
> +complete git 'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
> | sort | uniq`/'
> +complete gitk 'p/*/`${__git_tcsh_completion_script} "${COMMAND_LINE}"
> | sort | uniq`/'
This seems to be very different from bash's 'complete'. I wonder if
the 'complete' commands in the original script cause any problems.
Maybe only if you source it, but then again, I would expect a warning
or something when you run it.
But you can use the trick I did with zsh so you can source it:
complete ()
{
# do nothing
return 0
}
. "$script"
It looks like in your case you would need to save the old complete()
function though, or somehow restore it. If you go for this method, you
use the __*_main functions though.
Cheers.
--
Felipe Contreras
^ permalink raw reply
* Re: checkout from neighbour branch undeletes a path?
From: Peter Vereshagin @ 2012-11-13 18:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Peter Vereshagin, git
In-Reply-To: <7v8va5fo7g.fsf@alter.siamese.dyndns.org>
Hello.
2012/11/13 08:43:31 -0800 Junio C Hamano <gitster@pobox.com> => To Peter Vereshagin :
JCH> Peter Vereshagin <peter@vereshagin.org> writes:
JCH>
JCH> > Am wondering if 'checkout branch path' undeletes the files?
JCH>
JCH> "git checkout branch path" (by the way, "branch" does not have to be
JCH> a branch name; any commit object name would do, like "git checkout
JCH> HEAD^^ hello.c") is a way to check out named path(s) out of the
JCH> named commit.
JCH>
JCH> If the commit "branch" has "path" in it, its contents are checked
JCH> out to "path" in your current working tree (and the entry in the
JCH> index updated to match it).
JCH>
JCH> If you happen to have removed "path" in your current working tree
JCH> before running that command, it might look as if there is some
JCH> undelete going on, but that is a wrong way to look at things. The
JCH> version of "path" in the "branch" may or may not be similar to what
JCH> you have removed earlier.
JCH>
Hello.
I solved my problem by mean of 'git rm' instead of 'rm'.
I knew what you said here. Shortly, the difference for my case was:
- I check out the pathdir from the commit in which the pathdir/file00 was
already removed.
- The current branch 'branch01' has no idea the file00 was removed. But I
removed file00 and this is what 'branch01' assumes when I commit n ext time.
But git assumes I need 'file00' although it doesn't exist in the commit I
checkout path from. It doesn't exist by itself before I checkout that path
neither.
How can I know which one of the 'file00' versions is being checked out: the one
that did exist in the 'branch00' (the where I checkout path from) before I
removed it or the one existing in HEAD but not in the work-tree? And why this
and not that?
If it is the one that exists in a current branch but was deleted from trhe
work-tree [d4f7c70] than why it is being checked out not from the commit
supplied as an argument to git?
If it is the one that existed [c3e78ff] before the commit I checkout path from
than why it is being checked out while it doesn't exist in that commit already?
Thank you.
--
Peter Vereshagin <peter@vereshagin.org> (http://vereshagin.org) pgp: A0E26627
^ permalink raw reply
* Re: What's cooking in git.git (Oct 2012, #09; Mon, 29)
From: Ramsay Jones @ 2012-11-13 18:47 UTC (permalink / raw)
To: Jeff King; +Cc: Chris Rorvick, git
In-Reply-To: <20121111174136.GC13188@sigill.intra.peff.net>
Jeff King wrote:
> On Sat, Nov 10, 2012 at 06:33:38PM +0000, Ramsay Jones wrote:
>
>>> We should probably wrap it. I'm planning to queue this on top of Chris's
>>> patch:
>>
>> Unfortunately, I haven't had time yet to test this patch. (Early this week, I
>> went into hospital for a "minor" surgical procedure - I have not yet fully
>> recovered). The patch looks good and I don't anticipate any problems. I will
>> hopefully test it soon (I see it's in pu now) and let you know.
>
> Thanks. I merged it to "next" on Friday, so we will see if anybody
> screams. But please take your time and recover. Git will wait. :)
I have tested this patch and, as expected, it fixes things up for me.
(To be more precise: the current next branch works, but if I revert
commit c2b3af05, then t9604-cvsimport-timestamps.sh fails in the
same way as before!)
Thanks!
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [regression] Newer gits cannot clone any remote repos
From: Ramsay Jones @ 2012-11-13 18:55 UTC (permalink / raw)
To: Douglas Mencken; +Cc: git
In-Reply-To: <CACYvZ7jPd0_XD6YVdfJ2AnKRnKewmzX4uu7w3zt+_gK+qU49gQ@mail.gmail.com>
Douglas Mencken wrote:
> *Any* git clone fails with:
>
> fatal: premature end of pack file, 106 bytes missing
> fatal: index-pack failed
>
> At first, I tried 1.8.0, and it failed. Then I tried to build 1.7.10.5
> then, and it worked. Then I tried 1.7.12.2, but it fails the same way
> as 1.8.0.
> So I decided to git bisect.
>
> b8a2486f1524947f232f657e9f2ebf44e3e7a243 is the first bad commit
> ``index-pack: support multithreaded delta resolving''
This looks like the same problem I had on cygwin, which lead to
commit c0f86547c ("index-pack: Disable threading on cygwin", 26-06-2012).
I didn't notice which platform you are on, but maybe you also have a
thread-unsafe pread()? Could you try re-building git with the
NO_THREAD_SAFE_PREAD build variable set?
HTH.
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: "Jan H. Schönherr" @ 2012-11-13 18:58 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, rene.scharfe
In-Reply-To: <1352803572-14547-1-git-send-email-pclouds@gmail.com>
Hi.
Am 13.11.2012 11:46, schrieb Nguyễn Thái Ngọc Duy:
> Git's ispace does not include 11 and 12. Git's isprint includes
> control space characters (10-13). According to glibc-2.14.1 on C
> locale on Linux, this is wrong. This patch fixes it.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> I wrote a small C program to compare the result of all is* functions
> that Git replaces against the libc version. These are the only ones that
> differ. Which matches what Jan Schönherr commented.
>
> ctype.c | 6 +++---
> git-compat-util.h | 11 ++++++-----
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/ctype.c b/ctype.c
> index 0bfebb4..71311a3 100644
> --- a/ctype.c
> +++ b/ctype.c
> @@ -14,11 +14,11 @@ enum {
> P = GIT_PATHSPEC_MAGIC, /* other non-alnum, except for ] and } */
> X = GIT_CNTRL,
> U = GIT_PUNCT,
> - Z = GIT_CNTRL | GIT_SPACE
> + Z = GIT_CNTRL_SPACE
> };
>
> -const unsigned char sane_ctype[256] = {
> - X, X, X, X, X, X, X, X, X, Z, Z, X, X, Z, X, X, /* 0.. 15 */
> +const unsigned int sane_ctype[256] = {
> + X, X, X, X, X, X, X, X, X, Z, Z, Z, Z, Z, X, X, /* 0.. 15 */
> X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, /* 16.. 31 */
> S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P, /* 32.. 47 */
> D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G, /* 48.. 63 */
An alternative to switching from 1-byte to 4-byte values (don't we have
a 2-byte datatype?), would be to free up GIT_CNTRL and simply do:
#define iscntrl(x) ((x) < 0x20)
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 02f48f6..4ed3f94 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
[...]
> @@ -483,9 +483,10 @@ extern const unsigned char sane_ctype[256];
> #define GIT_PATHSPEC_MAGIC 0x20
> #define GIT_CNTRL 0x40
> #define GIT_PUNCT 0x80
> -#define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
> +#define GIT_SPACE 0x100
> +#define sane_istest(x,mask) ((sane_ctype[(unsigned int)(x)] & (mask)) != 0)
That should better be left "(unsigned char)"? We might access values after the
array otherwise.
(That said, it wasn't really correct before either, when there really is a
possibility that x >= 0x100.)
Regards
Jan
PS: It looks like my isprint() version was given precedence over your
isprint() version during the merge into next. That should also be sorted out,
but I've no idea which one is actually better: two comparisons versus one
cache lookup and a bitop... (though my guess is that comparisons are cheaper,
but then we should also convert isdigit()...)
^ permalink raw reply
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: René Scharfe @ 2012-11-13 19:14 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, Junio C Hamano, schnhrr
In-Reply-To: <1352803572-14547-1-git-send-email-pclouds@gmail.com>
Am 13.11.2012 11:46, schrieb Nguyễn Thái Ngọc Duy:
> Git's isprint includes
> control space characters (10-13). According to glibc-2.14.1 on C
> locale on Linux, this is wrong. This patch fixes it.
isprint() is not in master, yet. Can we perhaps still introduce it in
such a way that we never have an incorrect version in master's history?
And could you please update test-ctype.c to match the change to
isspace()? The tests there just documented the status quo before I made
changes to ctype.c long ago, so it's definitions are just as correct (or
wrong) as the original implementation.
Thanks,
René
^ permalink raw reply
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: René Scharfe @ 2012-11-13 19:15 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy, Linus Torvalds
Cc: git, Junio C Hamano, schnhrr
In-Reply-To: <1352803572-14547-1-git-send-email-pclouds@gmail.com>
Am 13.11.2012 11:46, schrieb Nguyễn Thái Ngọc Duy:
> Git's ispace does not include 11 and 12. [...]
> According to glibc-2.14.1 on C locale on Linux, this is wrong.
11 and 12 being vertical tab (\v) and form-feed (\f). This lack goes
back to the introduction of git's own character classifier macros seven
years ago in 4546738b (Unlocalized isspace and friends).
Linus, do you remember if you left them out on purpose?
Thanks,
René
^ permalink raw reply
* [PATCH] send-email: stop asking when we have an ident
From: Felipe Contreras @ 2012-11-13 19:19 UTC (permalink / raw)
To: git
Cc: Jeff King, Junio C Hamano, Thomas Rast, Jonathan Nieder,
Stephen Boyd, Felipe Contreras 2nd, Felipe Contreras
From: Felipe Contreras 2nd <felipe.contreras+2@gmail.com>
Currently we keep getting questions even when the user has properly
configured his full name and password:
Who should the emails appear to be from?
[Felipe Contreras <felipe.contreras@gmail.com>]
And once a question pops up, other questions are turned on. This is
annoying.
The reason this is safe is because currently the script fails completely
when the autohor (or committer) is not correct, so we won't even be
reaching this point in the code.
The scenarios, and the current situation:
1) No information at all, no fully qualified domain name
fatal: empty ident name (for <felipec@nysa.(none)>) not allowed
2) Only full name
fatal: unable to auto-detect email address (got 'felipec@nysa.(none)')
3) Full name + fqdm
Who should the emails appear to be from?
[Felipe Contreras <felipec@nysa.felipec.org>]
4) Full name + EMAIL
Who should the emails appear to be from?
[Felipe Contreras <felipe.contreras@gmail.com>]
5) User configured
6) GIT_COMMITTER
7) GIT_AUTHOR
All these are the same as 4)
After this patch:
1) 2) won't change: git send-email would still die
4) 5) 6) 7) will change: git send-email won't ask the user
This is good, that's what we would expect, because the identity is
explicit.
3) will change: git send-email won't ask the user
This is bad, because we will try with an address such as
'felipec@nysa.felipec.org', which is most likely not what the user
wants, but the user will get warned by default, and if not, most likley
the sending won't work, which the user can easily fix.
The worst possible scenario is that such a mail address does work, and
the user sends an email from that addres unintentionally, when in fact
the user expected to correct that address in the propmpt.
This is a very, very, very unlikely scenario, with many dependencies:
1) No configured user.name/user.email
2) No specified $EMAIL
3) No configured sendemail.from
4) No specified --from argument
5) A fully qualified domain name
6) A full name in the geckos field
7) A sendmail configuration that allows sending from this domain name
8) confirm=never, or
8.1) confirm configuration not hitting, or
8.2) Getting the error, not being aware of it
9) The user expecting to correct this address in the prompt
In a more likely scenario where 7) is not the case (can't send from
nysa.felipec.org), the user will simply see the mail was not sent
properly, and fix the problem.
The much more likely scenario though, is where 5) is not the case
(nysa.(none)), and git send-email will fail right away like it does now.
So the likelyhood of this affecting anybody seriously is very very slim,
and the chances of this affecting somebody slightly are still very
small. The vast majority, if not all, of git users won't be affected
negatively, and a lot will benefit from this.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
git-send-email.perl | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/git-send-email.perl b/git-send-email.perl
index aea66a0..503e551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -748,16 +748,11 @@ if (!$force) {
}
}
-my $prompting = 0;
if (!defined $sender) {
$sender = $repoauthor || $repocommitter || '';
- $sender = ask("Who should the emails appear to be from? [$sender] ",
- default => $sender,
- valid_re => qr/\@.*\./, confirm_only => 1);
- print "Emails will be sent from: ", $sender, "\n";
- $prompting++;
}
+my $prompting = 0;
if (!@initial_to && !defined $to_cmd) {
my $to = ask("Who should the emails be sent to (if any)? ",
default => "",
--
1.8.0
^ permalink raw reply related
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: Johannes Sixt @ 2012-11-13 19:41 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Junio C Hamano, schnhrr, rene.scharfe
In-Reply-To: <1352803572-14547-1-git-send-email-pclouds@gmail.com>
Am 13.11.2012 11:46, schrieb Nguyễn Thái Ngọc Duy:
> @@ -14,11 +14,11 @@ enum {
> P = GIT_PATHSPEC_MAGIC, /* other non-alnum, except for ] and } */
> X = GIT_CNTRL,
> U = GIT_PUNCT,
> - Z = GIT_CNTRL | GIT_SPACE
> + Z = GIT_CNTRL_SPACE
> };
>
> -const unsigned char sane_ctype[256] = {
> - X, X, X, X, X, X, X, X, X, Z, Z, X, X, Z, X, X, /* 0.. 15 */
> +const unsigned int sane_ctype[256] = {
> + X, X, X, X, X, X, X, X, X, Z, Z, Z, Z, Z, X, X, /* 0.. 15 */
> X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, X, /* 16.. 31 */
> S, P, P, P, R, P, P, P, R, R, G, R, P, P, R, P, /* 32.. 47 */
> D, D, D, D, D, D, D, D, D, D, P, P, P, P, P, G, /* 48.. 63 */
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 02f48f6..4ed3f94 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -474,8 +474,8 @@ extern const char tolower_trans_tbl[256];
> #undef ispunct
> #undef isxdigit
> #undef isprint
> -extern const unsigned char sane_ctype[256];
> -#define GIT_SPACE 0x01
> +extern const unsigned int sane_ctype[256];
> +#define GIT_CNTRL_SPACE 0x01
> #define GIT_DIGIT 0x02
> #define GIT_ALPHA 0x04
> #define GIT_GLOB_SPECIAL 0x08
> @@ -483,9 +483,10 @@ extern const unsigned char sane_ctype[256];
> #define GIT_PATHSPEC_MAGIC 0x20
> #define GIT_CNTRL 0x40
> #define GIT_PUNCT 0x80
> -#define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
> +#define GIT_SPACE 0x100
> +#define sane_istest(x,mask) ((sane_ctype[(unsigned int)(x)] & (mask)) != 0)
> #define isascii(x) (((x) & ~0x7f) == 0)
> -#define isspace(x) sane_istest(x,GIT_SPACE)
> +#define isspace(x) sane_istest(x,GIT_SPACE | GIT_CNTRL_SPACE)
> #define isdigit(x) sane_istest(x,GIT_DIGIT)
> #define isalpha(x) sane_istest(x,GIT_ALPHA)
> #define isalnum(x) sane_istest(x,GIT_ALPHA | GIT_DIGIT)
> @@ -493,7 +494,7 @@ extern const unsigned char sane_ctype[256];
> #define isupper(x) sane_iscase(x, 0)
> #define is_glob_special(x) sane_istest(x,GIT_GLOB_SPECIAL)
> #define is_regex_special(x) sane_istest(x,GIT_GLOB_SPECIAL | GIT_REGEX_SPECIAL)
> -#define iscntrl(x) (sane_istest(x,GIT_CNTRL))
> +#define iscntrl(x) (sane_istest(x,GIT_CNTRL | GIT_CNTRL_SPACE))
> #define ispunct(x) sane_istest(x, GIT_PUNCT | GIT_REGEX_SPECIAL | \
> GIT_GLOB_SPECIAL | GIT_PATHSPEC_MAGIC)
> #define isxdigit(x) (hexval_table[x] != -1)
So we have two properties that overlap:
SSSSSSSSSS
CCCCCCCC
You seem to generate partions:
XXXYYYYYZZZZZ
then assign individual bits to each partition. Now each entry in the
lookup table has only one bit set. Then you define isxxx() to check for
one of the two possible bits:
iscntrl is X or Y
isspace is Y or Z
But shouldn't you just assign one bit for S and another one for C, have
entries in the lookup table with more than one bit set, and check for
only one bit in the isxxx macro?
That way you don't run out of bits as easily as you do with this patch.
-- Hannes
^ permalink raw reply
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: Linus Torvalds @ 2012-11-13 19:40 UTC (permalink / raw)
To: René Scharfe
Cc: Nguyễn Thái Ngọc Duy, Git Mailing List,
Junio C Hamano, schnhrr
In-Reply-To: <50A29C3A.1070000@lsrfire.ath.cx>
On Tue, Nov 13, 2012 at 11:15 AM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
>
> Linus, do you remember if you left them out on purpose?
Umm, no.
I have to wonder why you care? As far as I'm concerned, the only valid
space is space, TAB and CR/LF.
Anything else is *noise*, not space. What's the reason for even caring?
Linus
^ permalink raw reply
* Re: [PATCH nd/wildmatch] Correct Git's version of isprint and isspace
From: Linus Torvalds @ 2012-11-13 19:50 UTC (permalink / raw)
To: René Scharfe
Cc: Nguyễn Thái Ngọc Duy, Git Mailing List,
Junio C Hamano, schnhrr
In-Reply-To: <CA+55aFwsjpOop=4mVkx4e=zw5LH41sD9x-b_WMo4Hvo7ygjEtQ@mail.gmail.com>
On Tue, Nov 13, 2012 at 11:40 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I have to wonder why you care? As far as I'm concerned, the only valid
> space is space, TAB and CR/LF.
>
> Anything else is *noise*, not space. What's the reason for even caring?
Btw, expanding the whitespace selection may actually be very
counter-productive. It is used primarily for things like removing
extraneous space at the end of lines etc, and for that, the current
selection of SPACE, TAB and LF/CR is the right thing to do.
Adding things like FF etc - that are *technically* whitespace, but
aren't the normal kind of silent whitespace - is potentially going to
change things too much. People might *want* a form-feed in their
messages, for all we know.
So I really object to changing things "just because". There's a reason
we do our own ctype.c: it avoids the crazy crap. It avoids the idiotic
localization issues, and it avoids the ambiguous cases.
So just let it be, unless you have some major real reason to actually
care about a real-world case. And if you do, please explain it. Don't
change things just because.
Linus
^ 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