* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Jeff King @ 2012-01-29 3:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael Haggerty, Mark Jason Dominus, git
In-Reply-To: <7vbopoxp5q.fsf@alter.siamese.dyndns.org>
On Fri, Jan 27, 2012 at 11:27:29PM -0800, Junio C Hamano wrote:
> > Given this design, shouldn't "git branch --edit-description" fail if the
> > branch doesn't already exist?
>
> The only use scenario in the original design was to have a way to store
> the description given a branch name, and giving a description to a branch
> that does not yet exist is outside the scope of the design. So it all
> boils down to what is the most convenient and the simplest to explain.
How do we define "branch exists"? That the ref exists? What about a HEAD
that points to a branch-to-be-born?
Specifically, I am wondering whether this:
$ git init
$ git branch --edit-description
should work. Right now it edits the description for "master", even
though you haven't yet committed to it.
-Peff
^ permalink raw reply
* Re: Cloning bare repository always warns that its empty.
From: Jeff King @ 2012-01-29 3:24 UTC (permalink / raw)
To: nmartin; +Cc: git
In-Reply-To: <1327806561158-7233455.post@n2.nabble.com>
On Sat, Jan 28, 2012 at 07:09:21PM -0800, nmartin wrote:
> I created a bare repository (empty) on my server which I making public over
> http. I cloned this repository on another machine(A), committed changes, and
> pushed the changes to the server. On the server when I run $ git log I can
> see the commit that I pushed. However if I clone this repository on a 3rd
> machine(B).. the repository is empty.
Is your http server git-aware (i.e., running the smart-git http-backend
CGI), or it is simply serving the repository files? If the latter, then
you need to arrange to run the "git update-server-info" when you push
into it, which will update the information that "dumb" http clients will
look at.
If the server is running git v1.6.6 or later, you can turn this on in
the server repository by running:
git config receive.updateserverinfo true
-Peff
^ permalink raw reply
* Bug: "git checkout -b" should be allowed in empty repo
From: Michael Haggerty @ 2012-01-29 6:09 UTC (permalink / raw)
To: git
When starting a new repo, git seems to insist that the first commit be
made on a branch named "master":
$ git --version
git version 1.7.9
$ git init git-test
Initialized empty Git repository in /home/mhagger/tmp/git-test/.git/
$ cd git-test
$ git checkout -b foo
fatal: You are on a branch yet to be born
I would call this a bug; the last command should be allowed. The
plumbing allows it:
$ git symbolic-ref HEAD refs/heads/foo
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Michael Haggerty @ 2012-01-29 6:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Jason Dominus, git
In-Reply-To: <7vbopoxp5q.fsf@alter.siamese.dyndns.org>
On 01/28/2012 08:27 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>>>> Where is branch blarf?
>>>
>>> You haven't created one in that sequence yet.
>>>
>>> This is more of a documentation bug.
>>>
>>> As the SYNOPSIS section makes it clear, the '--edit-description' is a
>>> separate mode from showing (the first group), creating or repointing (the
>>> second), or renaming (the third).
>>>
>>> After that you should be able to say "git branch blarf" and view its
>>> description.
>>
>> Given this design, shouldn't "git branch --edit-description" fail if the
>> branch doesn't already exist?
>
> The only use scenario in the original design was to have a way to store
> the description given a branch name, and giving a description to a branch
> that does not yet exist is outside the scope of the design. So it all
> boils down to what is the most convenient and the simplest to explain.
>
> We could error it out (i.e. you cannot name a thing that does not yet
> exist), or we could consider it is a convenience feature that you can
> prepare a description even before you create one, or we could even tweak
> it more like "-t $name" that tries to work both on existing one (without
> changing any base) or non-existing one, creating it while at it. The last
> approach historically is the most error prone (we had numerous bugs in the
> create_branch() helper after it started allowing an existing branch when
> updating the "track" information) and I would rather not go that route if
> we can avoid it.
>
> Honestly speaking, I haven't formed an opinion.
I vote for an error. Otherwise a typo in the branch name would lead to
the description's apparent disappearance into Nirvana. An error would,
for example, have made it clear to the OP what was happening.
A more useful option might be
git branch --with-description <branchname> [<start-point>]
i.e., that a branch's description can be set at the same time as the
branch is created.
>> Shouldn't it also resolve ambiguous reference names?
>
> As this mode, just like other modes of "git branch" _always_ takes a
> branch name, I do not think there is any "ambiguous reference name".
>
> You give $name to it, and it should always mean refs/heads/$name, and
> there shouldn't be any DWIMming to something like refs/heads/mh/$name
>
> Or do you have other kinds of ambiguity in mind?
OK, I wasn't aware that only local branches can have descriptions
attached to them. Given that fact, you are correct that DWIMming
doesn't make sense.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Junio C Hamano @ 2012-01-29 6:42 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Mark Jason Dominus, git
In-Reply-To: <4F24E78A.7060502@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 01/28/2012 08:27 AM, Junio C Hamano wrote:
>>
>> We could error it out (i.e. you cannot name a thing that does not yet
>> exist), or we could consider it is a convenience feature that you can
>> prepare a description even before you create one, or we could even tweak
>> it more like "-t $name" that tries to work both on existing one (without
>> changing any base) or non-existing one, creating it while at it. The last
>> approach historically is the most error prone (we had numerous bugs in the
>> create_branch() helper after it started allowing an existing branch when
>> updating the "track" information) and I would rather not go that route if
>> we can avoid it.
>>
>> Honestly speaking, I haven't formed an opinion.
>
> I vote for an error. Otherwise a typo in the branch name would lead to
> the description's apparent disappearance into Nirvana. An error would,
> for example, have made it clear to the OP what was happening.
>
> A more useful option might be
>
> git branch --with-description <branchname> [<start-point>]
>
> i.e., that a branch's description can be set at the same time as the
> branch is created.
So you are saying either option 1 or 3 is preferrable, while I was saying
I would rather avoid 3 if we could avoid it. Is that the short version?
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-01-29 6:56 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <4F24E287.3040302@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> When starting a new repo, git seems to insist that the first commit be
> made on a branch named "master":
>
> $ git --version
> git version 1.7.9
> $ git init git-test
> Initialized empty Git repository in /home/mhagger/tmp/git-test/.git/
> $ cd git-test
> $ git checkout -b foo
> fatal: You are on a branch yet to be born
>
> I would call this a bug; the last command should be allowed. The
> plumbing allows it:
>
> $ git symbolic-ref HEAD refs/heads/foo
Your last sentence is nonsense. The plumbing equivalent of that command
is *not* what you wrote above, but is more like [*1*]:
git update-ref refs/heads/foo $(git rev-parse --verify HEAD) &&
git symbolic-ref HEAD refs/heads/foo
And the first step will fail the same way. While I share the sense of
annoyance with you, I do not think that it is a bug in "checkout -b".
When you are on an unborn branch, what the "symbolic-ref HEAD" command
reports does *not* appear in the output from the "for-each-ref refs/heads"
command (similarly, that branch name does not appear in the output from
the "git branch" command).
Such a behaviour indeed is *curious* and very *different* from the normal
case of being on an existing branch, but is that a bug?
You need to first admit that the state immediately after "git init" (or
for that matter, "checkout --orphan") where you are on an unborn branch
*is* special. Some things that would normally make sense would not.
[Footnote]
*1* Because you are not switching to a different commit, there won't be a
need for the third step which is "git read-tree -m -u HEAD@{1} HEAD" that
would usually be necessary if you are giving a starting commit that is
different from HEAD.
^ permalink raw reply
* Re: Bug: "git checkout -b" should be allowed in empty repo
From: Junio C Hamano @ 2012-01-29 7:50 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <7vwr8bvvxj.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> ...
> Such a behaviour indeed is *curious* and very *different* from the normal
> case of being on an existing branch, but is that a bug?
>
> You need to first admit that the state immediately after "git init" (or
> for that matter, "checkout --orphan") where you are on an unborn branch
> *is* special. Some things that would normally make sense would not.
[sorry for having sent an incomplete message without conclusion]
The question then becomes this: what do you want to do about it with this
specific case, and more importantly what do you want to do about other
commands and options that would not make sense when HEAD knows what branch
the user wants to put the first commit on but there is no commit yet?
For some commands, we _do_ try to come up with a special case codepath so
that a command issued in the unborn state mimics the behaviour of the
command issued in the normal case to various different degrees. "git pull"
into an unborn branch simply resets to the other branch, for example, and
while technically speaking that is not merging the other branch into the
current commit (which does not exist), we do so because it was deemed to
be the most sensible behaviour to parallel the normal case.
I am not sure "git checkout -b foo" (without explict HEAD [*1*]) should
special case and degenerate to "symbolic-ref HEAD refs/heads/foo" when
HEAD points to a nonexistent branch. The mimicking does not go far enough
to satisfy people who are pedantic enough to expect "git checkout -b foo"
to work when you haven't even instantiated your current branch (when you
are on an already instantiated branch, after "git checkout -b foo", "git
branch" output will show both foo and the branch you were on, but if you
start from an unborn branch, the behaviour will be different and a pedant
will notice the difference).
It may make sense to let
$ git branch -m trunk
or even
$ git branch -m master trunk
move away from an unborn "master'"after "git init", with a special case
codepath. When you start from an instanticated branch, after a successful
such renaming, the original branch will not exist, and the new branch will
exist. This property would also hold true if you start from an unborn one,
so it would be much better mimickery than "git checkout -b foo" case you
brought up in this thread.
[Footnote]
*1* Given HEAD or $(git symbolic-ref HEAD) as the starting commit
explicitly, the command should error out.
^ permalink raw reply
* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Michael Haggerty @ 2012-01-29 10:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Mark Jason Dominus, git
In-Reply-To: <7v39azxb5l.fsf@alter.siamese.dyndns.org>
On 01/29/2012 07:42 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>
>> On 01/28/2012 08:27 AM, Junio C Hamano wrote:
>>>
>>> We could error it out (i.e. you cannot name a thing that does not yet
>>> exist), or we could consider it is a convenience feature that you can
>>> prepare a description even before you create one, or we could even tweak
>>> it more like "-t $name" that tries to work both on existing one (without
>>> changing any base) or non-existing one, creating it while at it. The last
>>> approach historically is the most error prone (we had numerous bugs in the
>>> create_branch() helper after it started allowing an existing branch when
>>> updating the "track" information) and I would rather not go that route if
>>> we can avoid it.
>>>
>>> Honestly speaking, I haven't formed an opinion.
>>
>> I vote for an error. Otherwise a typo in the branch name would lead to
>> the description's apparent disappearance into Nirvana. An error would,
>> for example, have made it clear to the OP what was happening.
>>
>> A more useful option might be
>>
>> git branch --with-description <branchname> [<start-point>]
>>
>> i.e., that a branch's description can be set at the same time as the
>> branch is created.
>
> So you are saying either option 1 or 3 is preferrable, while I was saying
> I would rather avoid 3 if we could avoid it. Is that the short version?
Not quite. I agree that "--add-description" should fail if the branch
already exists. But I was suggesting that there be a new *different*
option that can be used when creating a branch. "--with-description" is
probably not a great name, but I think it is a good idea that it be
spelled differently than "--add-description". Perhaps even "--message",
even though the abbreviation "-m" is precluded by the existing "-m" option.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-29 10:29 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git, Torsten Bögershausen
In-Reply-To: <CACsJy8BKQHLdoXfSKsULkWWbWjWEuZgr=bVNKmgCSArvwbf2UA@mail.gmail.com>
On 22.01.12 10:58, Nguyen Thai Ngoc Duy wrote:
> On Sun, Jan 22, 2012 at 5:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> [Pinging Nguyen who has worked rather extensively on the start-up sequence
>> for ideas.]
>>
[snip]
>
> I just have a quick look, you reencode opendir, readdir, and
> closedir() to precomposed form. But files are still in decomposed
> form, does open(<precomposed file>) work when only <decomposed file>
> exists?
Yes. All function like stat(), lstat(), open(), fopen(), unlink() behave the same
for precomped or decomposed. This is similar to the ignore case feature.
And because the default HFS+ is case preserving, case insenstive and unicode decomposing
all at the same time, a file name "Ä" could be reached under 4 different names.
Please see the output of the test script:
(which is at the end of this email)
tests/Darwin_i386/NFC file name created as nfc is readable as nfd
tests/Darwin_i386/NFC readdir returns nfd but expected is nfc
tests/Darwin_i386/NFD file name created as nfd is readable as nfc
tests/Darwin_i386/NFCNFD 1 file found in directory, but there should be 2
tests/Darwin_i386/NFCNFD nfc is missing, nfd is present
tests/Darwin_i386/NFCNFD nfc File content overwritten by nfd
tests/Darwin_i386/NFDNFC 1 file found in directory, but there should be 2
tests/Darwin_i386/NFDNFC nfc is missing, nfd is present
tests/Darwin_i386/NFDNFC nfd File content overwritten by nfc
>
>>> In order to prevent that ever a file name in decomposed unicode is
>>> entering the index, a "brute force" attempt is taken: all arguments into
>>> git (argv[1]..argv[n]) are converted into precomposed unicode. This is
>>> done in git.c by calling precompose_argv(). This function is actually a
>>> #define, and it is only defined under Mac OS. Nothing is converted on
>>> any other platforms.
>
> This is not entirely safe. Filenames can be taken from a file for
> example (--stdin option or similar). Unless I'm mistaken, all file
> names must enter git through the index, the conversion at read-cache.c
> may be a better option.
Good point, thanks.
I added some code to read-cache.c, and it works for files, but not for directories.
I looked through the code for "case-ignoring" directory names, and couldn't
find something obvious. More work is to be done.
[snip]
> I'd rather encode at index level and read_directory() than at argv[].
>But if reencoding argv is the only feasible way, perhaps put the
>conversion in parse_options()?
I tried that, and found that git-lsfiles.c doesn't use parse_options.
[snip]
On the long run I want to get rid of the argv[] conversion completely,
but I'm not there yet.
Thanks for all comments and inspiration!
(and apologies for my long response times I use to have)
/Torsten
PS:
Here the script.
Mac OS writes decomposd unicode to HFS+, precomposed unicode to VFAT and SAMBA.
In any case readdir() returns decomposed.
=================
#!/bin/sh
errorandout() {
echo Error: The shell can not handle nfd
echo try to run /bin/bash $0
rm -rf $DIR
exit 1
}
checkDirNfcOrNfd() {
DDNFCNFD=$1
readdirexp=$2
if test -r $DDNFCNFD/$aumlnfc; then
x=`cat $DDNFCNFD/$aumlnfc`
if test "$x" = nfd; then
echo $DDNFCNFD file name created as nfd is readable as nfc
fi
fi
if test -r $DDNFCNFD/$aumlnfd; then
x=`cat $DDNFCNFD/$aumlnfd 2>/dev/null` || {
echo $DDNFCNFD nfd is not readable, but readdir says that it exist
}
if test "$x" = nfc; then
echo $DDNFCNFD file name created as nfc is readable as nfd
fi
fi
readdirres=`echo $DDNFCNFD/*`
if test "$readdirres" != "$DDNFCNFD/$readdirexp"; then
if test "$readdirres" = $DDNFCNFD/$aumlnfd; then
echo $DDNFCNFD readdir returns nfd but expected is nfc
fi
if test "$readdirres" = $DDNFCNFD/$aumlnfc; then
echo $DDNFCNFD readdir returns nfc but expected is nfd
fi
fi
}
checkdirnfcnfd() {
DDNFCNFD=$1
if test `ls -1 $DDNFCNFD | wc -l` != 2; then
if test `ls -1 $DDNFCNFD | wc -l` == 1; then
echo $DDNFCNFD 1 file found in directory, but there should be 2
else
echo $DDNFCNFD 2 files should be in directory
fi
fi
x=`echo $DDNFCNFD/*`
a=`echo $DDNFCNFD/$aumlnfd $DDNFCNFD/$aumlnfc`
b=`echo $DDNFCNFD/$aumlnfc $DDNFCNFD/$aumlnfd`
c=`echo $DDNFCNFD/$aumlnfc $DDNFCNFD/$aumlnfc`
d=`echo $DDNFCNFD/$aumlnfd $DDNFCNFD/$aumlnfd`
e=`echo $DDNFCNFD/$aumlnfc`
f=`echo $DDNFCNFD/$aumlnfd`
case "$x" in
$a)
;;
$b)
;;
$c)
echo $DDNFCNFD nfd is hidden, nfc is listed twice
;;
$d)
echo $DDNFCNFD nfc is hidden, nfd is listed twice
;;
$e)
echo $DDNFCNFD nfd is missing, nfc is present
;;
$f)
echo $DDNFCNFD nfc is missing, nfd is present
;;
*)
echo $DDNFCNFD x`echo $x | xxd`
;;
esac
if ! test -r $DDNFCNFD/$aumlnfc; then
echo $DDNFCNFD/nfc File does not exist
else
x=`cat $DDNFCNFD/$aumlnfc`
if test "$x" != nfc; then
echo $DDNFCNFD nfc File content overwritten by $x
fi
fi
if ! test -r $DDNFCNFD/$aumlnfd; then
echo $DDNFCNFD/nfd File does not exist
else
x=`cat $DDNFCNFD/$aumlnfd`
if test "$x" != nfd; then
echo $DDNFCNFD nfd File content overwritten by $x
fi
fi
}
aumlnfc=$(printf '\303\204')
aumlnfd=$(printf '\101\314\210')
DIR=tests/`uname -s`_`uname -m`
echo "DIR=$DIR"
rm -rf $DIR/NFC &&
rm -rf $DIR/NFD &&
rm -rf $DIR/NFCNFD &&
rm -rf $DIR/NFDNFC &&
mkdir -p $DIR/NFC &&
mkdir -p $DIR/NFD &&
mkdir -p $DIR/NFDNFC &&
mkdir -p $DIR/NFCNFD &&
echo nfc > $DIR/NFC/$aumlnfc &&
echo nfd > $DIR/NFD/$aumlnfd &&
echo nfd > $DIR/NFDNFC/$aumlnfd &&
echo nfc > $DIR/NFDNFC/$aumlnfc &&
echo nfc > $DIR/NFCNFD/$aumlnfc &&
echo nfd > $DIR/NFCNFD/$aumlnfd && {
# test 1: basic if the shell handles nfd
if ! test -r $DIR/NFD/$aumlnfd; then
errorandout
fi
for DD in tests/*; do
checkDirNfcOrNfd $DD/NFC $aumlnfc
checkDirNfcOrNfd $DD/NFD $aumlnfd
checkdirnfcnfd $DD/NFCNFD
checkdirnfcnfd $DD/NFDNFC
done
} || errorandout
^ permalink raw reply
* Re: [PATCH v3] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 12:54 UTC (permalink / raw)
To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129012234.GD16079@server.brlink.eu>
On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
>
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
Nice and succinct.
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
>
O.K.
> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
> (As there is no check if the filter_path would have been found in
> the usual search as the project path is checked with forks).
>
Now I understand how project_filter interacts with strict_export.
Though I am not sure if this "paranoid mode" is really necessary. I don't
see how you could get in situation where scanning from $project_list and
filtering with $project_filter prefix, and scanning from
$project_list/$project_filter would give different results.
I think you are overly paranoid here, but perhaps it is better to be
overly strict, and then relax it if it turns out to be not necessary.
> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently being
> used to ensure nothing is exported that should not be viewable.
>
Sidenote: support for actionless PATH_INFO URLs would make it even more
complicated...
> Additionally change html page headers to not only link the project
> root and the currently selected project but also the directories in
> between using project_filter.
>
Excuse me changing my mind, but I think that as far as this patch series
is applied as whole, it would be better for maintability to keep those
two patches split; though put the above as a [part of] commit message
in 2/2 patch.
> Signed-off-by: Bernhard R. Link <brlink@debian.org>
> ---
>
> changes since v2:
> improve description
> remove || 0 for boolean argument
> merge with patch using this feature
> use user-visible configuration names instead of internal ones
>
> * Jakub Narebski <jnareb@gmail.com> [120128 23:45]:
> > "Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
> > > If strict_export is enabled and there is no projects_list, it still
> > > traverses the full tree and only filters afterwards to avoid anything
> > > getting visible by this. Otherwise only the subtree needs to be
> > > traversed, significantly reducing load times.
> > >
> > I still don't understand interaction between project_filter ('pf'),
> > $strict_export and $projects_list being either directory or a file
> > with a list of projects.
> >
> > Does it mean, that when $projects_list is a file with a list of projects,
> > and we use project_filter, then:
> >
> > * if $strict_export is false, then $project_list is ignored, and the
> > filtered list of projects is created by scanning
> > "$projectroot/$project_filter"
>
> No. If project_list is set, i.e. a file, then this is always used.
> If it is a directory (because it is not set thus set to projectroot),
> then with forks it still traverses that directory (as that was checked
> before to be a reachable project with a previous call to
> git_get_projects_list). In the case of project_filter only the directory
> is traversed without strict_export and the whole projectroot is
> traversed with strict_export.
>
O.K., now I understand it.
> Is the new description better.
>
Yes it is.
> > A few nitpicks with respect to patch itself.
> >
> > > -2827,6 +2835,7 @@ sub git_get_project_url_list {
> > >
> > > sub git_get_projects_list {
> > > my $filter = shift || '';
> > > + my $paranoid = shift || 0;
> > > my @list;
> > >
> >
> > First, undefined value is false in Perl, so there is no need for
> > " || 0" in setting $paranoid variable.
>
> I thought it make it clearer that the argument might not be set and
> what the default is. But that is personal taste.
First, optional parameter defaults to false in the 'my $foo = shift;'
or equivalent form is (I think) idiomatic Perl. Second, this way of
writing it is used through gitweb code (CodingGuidelines: imitate existing
coding practices).
> > Second, why not use global variable $strict_export instead of adding
> > another parameter to git_get_projects_list()?
>
> That would change the action=forks behaviour to traverse the whole
> projectroot two times. This way paranoia is only activated if
> strict_mode is set _and_ the argument was not yet checked to be
> reachable.
Thanks for explanation.
> gitweb/gitweb.perl | 52 ++++++++++++++++++++++++++++++++++++++++++++--------
> 1 files changed, 44 insertions(+), 8 deletions(-)
Not that large for a new feature...
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] completion: --edit-description option for git-branch
From: Ralf Thielow @ 2012-01-29 12:55 UTC (permalink / raw)
To: spearce; +Cc: git, Ralf Thielow
Signed-off-by: Ralf Thielow <ralf.thielow@googlemail.com>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 1496c6d..e44eefd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1152,7 +1152,7 @@ _git_branch ()
__gitcomp "
--color --no-color --verbose --abbrev= --no-abbrev
--track --no-track --contains --merged --no-merged
- --set-upstream
+ --set-upstream --edit-description
"
;;
*)
--
1.7.9.dirty
^ permalink raw reply related
* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Torsten Bögershausen @ 2012-01-29 12:57 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, git
In-Reply-To: <4F251FA1.80400@web.de>
> I tried that, and found that git-lsfiles.c doesn't use parse_options.
Oops, I shouldn't have written that: git-lsfiles uses parse_options.
Sorry for the noise.
/torsten
^ permalink raw reply
* [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Bernhard R. Link @ 2012-01-29 16:06 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <201201291354.50241.jnareb@gmail.com>
This commit changes the project listing views (project_list,
project_index and opml) to limit the output to only projects in a
subdirectory if the new optional parameter ?pf=directory name is used.
The change is quite minimal as git_get_projects_list already can limit
itself to a subdirectory (though that was previously only used for
'forks').
If there is a GITWEB_LIST file, the contents are just filtered like
with the forks action.
Without a GITWEB_LIST file only the given subdirectory is searched
for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
In the later case GITWEB_PROJECTROOT is traversed normally (unlike
with forks) and projects not in the directory ignored.
(As there is no check if the filter_path would have been found in
the usual search as the project path is checked with forks).
Reusing $project instead of adding a new parameter would have been
nicer from a UI point-of-view (including PATH_INFO support) but
would complicate the $project validating code that is currently being
used to ensure nothing is exported that should not be viewable.
Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
* Jakub Narebski <jnareb@gmail.com> [120129 13:54]:
> On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> Though I am not sure if this "paranoid mode" is really necessary. I don't
> see how you could get in situation where scanning from $project_list and
> filtering with $project_filter prefix, and scanning from
> $project_list/$project_filter would give different results.
>
> I think you are overly paranoid here, but perhaps it is better to be
> overly strict, and then relax it if it turns out to be not necessary.
As far as I do understand it, this is the only (hopefully unecessary)
effect strict_export without a project_list has in gitweb, so I did not
want to remove that with this change.
> Excuse me changing my mind, but I think that as far as this patch series
> is applied as whole, it would be better for maintability to keep those
> two patches split; though put the above as a [part of] commit message
> in 2/2 patch.
Split again, though this time only the change for existing pages in the
second commit and the code duplication you spoke against removed.
gitweb/gitweb.perl | 43 ++++++++++++++++++++++++++++++++++++-------
1 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..f0e03d8 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -760,6 +760,7 @@ our @cgi_param_mapping = (
search_use_regexp => "sr",
ctag => "by_tag",
diff_style => "ds",
+ project_filter => "pf",
# this must be last entry (for manipulation from JavaScript)
javascript => "js"
);
@@ -976,7 +977,7 @@ sub evaluate_path_info {
our ($action, $project, $file_name, $file_parent, $hash, $hash_parent, $hash_base,
$hash_parent_base, @extra_options, $page, $searchtype, $search_use_regexp,
- $searchtext, $search_regexp);
+ $searchtext, $search_regexp, $project_filter);
sub evaluate_and_validate_params {
our $action = $input_params{'action'};
if (defined $action) {
@@ -994,6 +995,13 @@ sub evaluate_and_validate_params {
}
}
+ our $project_filter = $input_params{'project_filter'};
+ if (defined $project_filter) {
+ if (!validate_pathname($project_filter)) {
+ die_error(404, "Invalid project_filter parameter");
+ }
+ }
+
our $file_name = $input_params{'file_name'};
if (defined $file_name) {
if (!validate_pathname($file_name)) {
@@ -2827,6 +2835,7 @@ sub git_get_project_url_list {
sub git_get_projects_list {
my $filter = shift || '';
+ my $paranoid = shift;
my @list;
$filter =~ s/\.git$//;
@@ -2839,7 +2848,7 @@ sub git_get_projects_list {
my $pfxlen = length("$dir");
my $pfxdepth = ($dir =~ tr!/!!);
# when filtering, search only given subdirectory
- if ($filter) {
+ if ($filter and not $paranoid) {
$dir .= "/$filter";
$dir =~ s!/+$!!;
}
@@ -2864,6 +2873,10 @@ sub git_get_projects_list {
}
my $path = substr($File::Find::name, $pfxlen + 1);
+ # paranoidly only filter here
+ if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
+ next;
+ }
# we check related file in $projectroot
if (check_export_ok("$projectroot/$path")) {
push @list, { path => $path };
@@ -3823,6 +3836,18 @@ sub print_header_links {
}
}
+sub print_nav_breadcrumbs_path {
+ my $dirprefix = undef;
+ while (my $part = shift) {
+ $dirprefix .= "/" if defined $dirprefix;
+ $dirprefix .= $part;
+ print $cgi->a({-href => href(project => undef,
+ project_filter => $dirprefix,
+ action=>"project_list")},
+ esc_html($part)) . " / ";
+ }
+}
+
sub print_nav_breadcrumbs {
my %opts = @_;
@@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
print " / $opts{-action_extra}";
}
print "\n";
+ } elsif (defined $project_filter) {
+ print_nav_breadcrumbs_path(split '/', $project_filter);
}
}
@@ -3963,9 +3990,11 @@ sub git_footer_html {
}
} else {
- print $cgi->a({-href => href(project=>undef, action=>"opml"),
+ print $cgi->a({-href => href(project=>undef, action=>"opml",
+ project_filter => $project_filter),
-class => $feed_class}, "OPML") . " ";
- print $cgi->a({-href => href(project=>undef, action=>"project_index"),
+ print $cgi->a({-href => href(project=>undef, action=>"project_index",
+ project_filter => $project_filter),
-class => $feed_class}, "TXT") . "\n";
}
print "</div>\n"; # class="page_footer"
@@ -5979,7 +6008,7 @@ sub git_project_list {
die_error(400, "Unknown order parameter");
}
- my @list = git_get_projects_list();
+ my @list = git_get_projects_list($project_filter, $strict_export);
if (!@list) {
die_error(404, "No projects found");
}
@@ -6018,7 +6047,7 @@ sub git_forks {
}
sub git_project_index {
- my @projects = git_get_projects_list();
+ my @projects = git_get_projects_list($project_filter, $strict_export);
if (!@projects) {
die_error(404, "No projects found");
}
@@ -7855,7 +7884,7 @@ sub git_atom {
}
sub git_opml {
- my @list = git_get_projects_list();
+ my @list = git_get_projects_list($project_filter, $strict_export);
if (!@list) {
die_error(404, "No projects found");
}
--
1.7.8.3
^ permalink raw reply related
* [PATCH 2/2] gitweb: place links to parent directories in page header
From: Bernhard R. Link @ 2012-01-29 16:07 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
Change html page headers to not only link the project root and the
currently selected project but also the directories in between using
project_filter.
Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
gitweb/gitweb.perl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f0e03d8..e2a9146 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
if (defined $project) {
- print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
+ my @dirname = split '/', $project;
+ my $projectbasename = pop @dirname;
+ print_nav_breadcrumbs_path(@dirname);
+ print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
if (defined $action) {
my $action_print = $action ;
if (defined $opts{-action_extra}) {
--
1.7.8.3
^ permalink raw reply related
* [PATCH v4 2/2] gitweb: place links to parent directories in page header
From: Bernhard R. Link @ 2012-01-29 16:13 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>
Change html page headers to not only link the project root and the
currently selected project but also the directories in between using
project_filter.
Signed-off-by: Bernhard R. Link <brlink@debian.org>
---
gitweb/gitweb.perl | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f0e03d8..e2a9146 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
if (defined $project) {
- print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
+ my @dirname = split '/', $project;
+ my $projectbasename = pop @dirname;
+ print_nav_breadcrumbs_path(@dirname);
+ print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
if (defined $action) {
my $action_print = $action ;
if (defined $opts{-action_extra}) {
--
1.7.8.3
^ permalink raw reply related
* Re: [PATCH V4] git on Mac OS and precomposed unicode
From: Erik Faye-Lund @ 2012-01-29 16:26 UTC (permalink / raw)
To: Carlos Martín Nieto; +Cc: Torsten Bögershausen, git
In-Reply-To: <1327184934.31804.32.camel@centaur.lab.cmartin.tk>
On Sat, Jan 21, 2012 at 11:28 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> On Sat, 2012-01-21 at 20:36 +0100, Torsten Bögershausen wrote:
>> * (Not all Windows versions support UTF-8 yet:
>> Msysgit needs the unicode branch, cygwin supports UTF-8 since 1.7)
>
> This might be overly pedantic, but Windows doesn't really deal with
> UTF-8. To use Unicode you need to use the "wide" variant of the
> functions, and those take UTF-16.
This is exactly what the 'unicode'-branch in msysGit does, so the
comment is not incorrect at all.
^ permalink raw reply
* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 16:41 UTC (permalink / raw)
To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>
On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
>
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
>
> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
> (As there is no check if the filter_path would have been found in
> the usual search as the project path is checked with forks).
I am still unsure if it is really necessary, but nevermind...
> Reusing $project instead of adding a new parameter would have been
> nicer from a UI point-of-view (including PATH_INFO support) but
> would complicate the $project validating code that is currently being
> used to ensure nothing is exported that should not be viewable.
>
> Signed-off-by: Bernhard R. Link <brlink@debian.org>
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
[...]
> @@ -3823,6 +3836,18 @@ sub print_header_links {
> }
> }
>
> +sub print_nav_breadcrumbs_path {
> + my $dirprefix = undef;
> + while (my $part = shift) {
> + $dirprefix .= "/" if defined $dirprefix;
> + $dirprefix .= $part;
> + print $cgi->a({-href => href(project => undef,
> + project_filter => $dirprefix,
> + action=>"project_list")},
> + esc_html($part)) . " / ";
> + }
> +}
> +
> sub print_nav_breadcrumbs {
> my %opts = @_;
>
> @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
> print " / $opts{-action_extra}";
> }
> print "\n";
> + } elsif (defined $project_filter) {
> + print_nav_breadcrumbs_path(split '/', $project_filter);
> }
> }
>
This could have been split into a separate 2/3 commit, but nevermind;
it can be squashed here.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v4 2/2] gitweb: place links to parent directories in page header
From: Jakub Narebski @ 2012-01-29 16:46 UTC (permalink / raw)
To: Bernhard R. Link; +Cc: git
In-Reply-To: <20120129161316.GD13937@server.brlink.eu>
On Sun, 29 Jan 2012, Bernhard R. Link wrote:
> Change html page headers to not only link the project root and the
> currently selected project but also the directories in between using
> project_filter.
Nice interface to the new feature... though it doesn't really address
the problem that gitweb homepage is slow to generate with large number
of projects. Still, it is IMVHO a good improvement.
> Signed-off-by: Bernhard R. Link <brlink@debian.org>
Acked-by: Jakub Narebski <jnareb@gmail.com>
> ---
> gitweb/gitweb.perl | 5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index f0e03d8..e2a9146 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -3853,7 +3853,10 @@ sub print_nav_breadcrumbs {
>
> print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
> if (defined $project) {
> - print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
> + my @dirname = split '/', $project;
> + my $projectbasename = pop @dirname;
> + print_nav_breadcrumbs_path(@dirname);
> + print $cgi->a({-href => href(action=>"summary")}, esc_html($projectbasename));
> if (defined $action) {
> my $action_print = $action ;
> if (defined $opts{-action_extra}) {
> --
Nicely short with refactoring of print_nav_breadcrumbs_path() in 1/2!
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Junio C Hamano @ 2012-01-29 21:06 UTC (permalink / raw)
To: Bernhard R. Link; +Cc: Jakub Narebski, git
In-Reply-To: <20120129160615.GA13937@server.brlink.eu>
"Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
> This commit changes the project listing views (project_list,
> project_index and opml) to limit the output to only projects in a
> subdirectory if the new optional parameter ?pf=directory name is used.
>
> The change is quite minimal as git_get_projects_list already can limit
> itself to a subdirectory (though that was previously only used for
> 'forks').
>
> If there is a GITWEB_LIST file, the contents are just filtered like
> with the forks action.
Meaning, a directory is shown if it is listed on GITWEB_LIST and is a
subdirectory of the directory specified with project_filter? If so,
spelling it out instead of saying "just filtered like with the forks
action" may be clearer without making the description excessively longer.
> Without a GITWEB_LIST file only the given subdirectory is searched
> for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> with forks) and projects not in the directory ignored.
It is unclear to me what "In the later case" refers to, even assuming that
it is a typo of "the latter case".
Do you mean "When there is no GITWEB_LIST but GITWEB_STRICT_EXPORT is set,
project_filter that specifies anything outside GITWEB_PROJECTROOT is
ignored"?
A more fundamental issue I have with this patch is how an end user starts
using this. Once project_filter is set, the breadcrumbs would let the user
click and navigate around, but in my superficial glance at the patch it is
not apparent how the initial setting of project_filter can happen without
the user manually adding pf= to the URL, which is a less than ideal end
user experience.
> @@ -2839,7 +2848,7 @@ sub git_get_projects_list {
> my $pfxlen = length("$dir");
> my $pfxdepth = ($dir =~ tr!/!!);
> # when filtering, search only given subdirectory
> - if ($filter) {
> + if ($filter and not $paranoid) {
> $dir .= "/$filter";
> $dir =~ s!/+$!!;
> }
> @@ -2864,6 +2873,10 @@ sub git_get_projects_list {
> }
>
> my $path = substr($File::Find::name, $pfxlen + 1);
> + # paranoidly only filter here
> + if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
> + next;
> + }
When you find "foo" directory and a project_filter tells you to match
"foo", because $path does not match "^foo/", it will not match (even
though its subdirectory "foo/bar" would)?
> +sub print_nav_breadcrumbs_path {
> + my $dirprefix = undef;
> + while (my $part = shift) {
> + $dirprefix .= "/" if defined $dirprefix;
> + $dirprefix .= $part;
> + print $cgi->a({-href => href(project => undef,
> + project_filter => $dirprefix,
> + action=>"project_list")},
> + esc_html($part)) . " / ";
> + }
> +}
> +
> sub print_nav_breadcrumbs {
> my %opts = @_;
>
> @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
> print " / $opts{-action_extra}";
> }
> print "\n";
> + } elsif (defined $project_filter) {
> + print_nav_breadcrumbs_path(split '/', $project_filter);
> }
> }
Hmm.
While this may not be wrong, I wonder if this is limiting a useful feature
too narrowly. When I visit "/pub/scm /linux/kernel/git/torvals/linux.git"
at git.kernel.org, for example, there currently are two links, "/pub/scm"
to the toplevel and "/linux/kernel/git/torvals/linux.git" to itself. I
often wish to see uplinks to intermediate levels like "/linux/kernel/git"
and "/linux/kernel/git/torvalds".
Perhaps that is the topic of your second patch. I dunno.
^ permalink raw reply
* Re: BUG 1.7.9: git branch fails to create new branch when --edit-description is used
From: Junio C Hamano @ 2012-01-29 21:49 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Mark Jason Dominus, git
In-Reply-To: <4F24E78A.7060502@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 01/28/2012 08:27 AM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>
> I vote for an error. Otherwise a typo in the branch name would lead to
> the description's apparent disappearance into Nirvana. An error would,
> for example, have made it clear to the OP what was happening.
I agree with this statement.
Unless or until we update --edit-description to create a new branch when
given a name of a branch that does not exist, the existing behaviour is
simply an accident waiting to happen.
^ permalink raw reply
* [PATCH/RFC v2] grep: Add the option '--exclude'
From: Albert Yale @ 2012-01-29 22:42 UTC (permalink / raw)
To: git; +Cc: gitster, Albert Yale
Signed-off-by: Albert Yale <surfingalbert@gmail.com>
---
This is a revision to my previous patch.
The exclusion information is now held
inside the pathspec.
Feedback would again be appreciated,
Albert Yale
Documentation/git-grep.txt | 7 +++++
builtin/grep.c | 43 ++++++++++++++++++++++++++++++++-
cache.h | 3 ++
dir.c | 55 ++++++++++++++++++++++++++++++++++++++++----
4 files changed, 101 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6a8b1e3..db143e3 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -22,6 +22,7 @@ SYNOPSIS
[--color[=<when>] | --no-color]
[-A <post-context>] [-B <pre-context>] [-C <context>]
[-f <file>] [-e] <pattern>
+ [-x<pattern>|--exclude<pattern>]
[--and|--or|--not|(|)|-e <pattern>...]
[ [--exclude-standard] [--cached | --no-index | --untracked] | <tree>...]
[--] [<pathspec>...]
@@ -124,6 +125,12 @@ OPTIONS
Use fixed strings for patterns (don't interpret pattern
as a regex).
+-x<pattern>::
+--exclude<pattern>::
+ In addition to those found in .gitignore (per directory) and
+ $GIT_DIR/info/exclude, also consider these patterns to be in the
+ set of the ignore rules in effect.
+
-n::
--line-number::
Prefix the line number to matching lines.
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..9772fa4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -530,6 +530,10 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
continue;
if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
continue;
+ if (pathspec->exclude &&
+ pathspec->exclude->nr &&
+ match_pathspec_depth(pathspec->exclude, ce->name, ce_namelen(ce), 0, NULL))
+ continue;
/*
* If CE_VALID is on, we assume worktree file and its cache entry
* are identical, even if worktree file has been modified, so use
@@ -566,6 +570,11 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
while (tree_entry(tree, &entry)) {
int te_len = tree_entry_len(&entry);
+ if (pathspec->exclude &&
+ pathspec->exclude->nr &&
+ match_pathspec_depth(pathspec->exclude, entry.path, strlen(entry.path), 0, NULL))
+ continue;
+
if (match != all_entries_interesting) {
match = tree_entry_interesting(&entry, base, tn_len, pathspec);
if (match == all_entries_not_interesting)
@@ -606,8 +615,16 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
struct object *obj, const char *name)
{
- if (obj->type == OBJ_BLOB)
+ if (obj->type == OBJ_BLOB) {
+ const char *name_without_sha1 = strchr(name, ':') + 1;
+
+ if (pathspec->exclude &&
+ pathspec->exclude->nr &&
+ match_pathspec_depth(pathspec->exclude, name_without_sha1, strlen(name_without_sha1), 0, NULL))
+ return 0;
+
return grep_sha1(opt, obj->sha1, name, 0);
+ }
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -673,6 +690,10 @@ static int grep_directory(struct grep_opt *opt, const struct pathspec *pathspec,
int namelen = strlen(name);
if (!match_pathspec_depth(pathspec, name, namelen, 0, NULL))
continue;
+ if (pathspec->exclude &&
+ pathspec->exclude->nr &&
+ match_pathspec_depth(pathspec->exclude, name, namelen, 0, NULL))
+ continue;
hit |= grep_file(opt, dir.entries[i]->name);
if (hit && opt->status_only)
break;
@@ -764,6 +785,14 @@ static int pattern_callback(const struct option *opt, const char *arg,
return 0;
}
+static int exclude_cb(const struct option *opt, const char *arg,
+ int unset)
+{
+ struct string_list *exclude_list = opt->value;
+ string_list_append(exclude_list, arg);
+ return 0;
+}
+
static int help_callback(const struct option *opt, const char *arg, int unset)
{
return -1;
@@ -792,7 +821,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pattern_type_pcre,
};
int pattern_type = pattern_type_unspecified;
-
+ struct string_list exclude_list = STRING_LIST_INIT_NODUP;
struct option options[] = {
OPT_BOOLEAN(0, "cached", &cached,
"search in index instead of in the work tree"),
@@ -872,6 +901,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
"read patterns from file", file_callback),
{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+ { OPTION_CALLBACK, 'x', "exclude", &exclude_list, "pattern",
+ "add <pattern> to ignore rules", PARSE_OPT_NONEG, exclude_cb },
{ OPTION_CALLBACK, 0, "and", &opt, NULL,
"combine patterns specified with -e",
PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
@@ -1053,6 +1084,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
pathspec.max_depth = opt.max_depth;
pathspec.recursive = 1;
+ if( exclude_list.nr ) {
+ create_pathspec_from_string_list(&pathspec.exclude, &exclude_list);
+ pathspec.exclude->max_depth = opt.max_depth;
+ pathspec.exclude->recursive = 1;
+ }
+
if (show_in_pager && (cached || list.nr))
die(_("--open-files-in-pager only works on the worktree"));
@@ -1102,5 +1139,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
if (hit && show_in_pager)
run_pager(&opt, prefix);
free_grep_patterns(&opt);
+ free_pathspec(&pathspec);
+ string_list_clear(&exclude_list, 0);
return !hit;
}
diff --git a/cache.h b/cache.h
index 10afd71..882a390 100644
--- a/cache.h
+++ b/cache.h
@@ -533,9 +533,12 @@ struct pathspec {
int len;
unsigned int use_wildcard:1;
} *items;
+
+ struct pathspec *exclude;
};
extern int init_pathspec(struct pathspec *, const char **);
+extern int create_pathspec_from_string_list(struct pathspec **, const struct string_list *);
extern void free_pathspec(struct pathspec *);
extern int ce_path_match(const struct cache_entry *ce, const struct pathspec *pathspec);
diff --git a/dir.c b/dir.c
index 0a78d00..fd04afa 100644
--- a/dir.c
+++ b/dir.c
@@ -9,6 +9,8 @@
#include "dir.h"
#include "refs.h"
+#include "string-list.h"
+
struct path_simplify {
int len;
const char *path;
@@ -1259,6 +1261,49 @@ static int pathspec_item_cmp(const void *a_, const void *b_)
return strcmp(a->match, b->match);
}
+static int init_pathspec_item( struct pathspec_item *item, const char *path )
+{
+ item->match = path;
+ item->len = strlen(path);
+ item->use_wildcard = !no_wildcard(path);
+
+ return item->use_wildcard;
+}
+
+int create_pathspec_from_string_list(struct pathspec **pathspec, const struct string_list *path_list)
+{
+ int i;
+
+ *pathspec = xcalloc( 1, sizeof( struct pathspec ) );
+
+ if (!path_list->nr)
+ return 0;
+
+ (*pathspec)->nr = path_list->nr;
+ (*pathspec)->items = xcalloc(path_list->nr, sizeof(struct pathspec_item));
+
+ for (i = 0; i < path_list->nr; i++) {
+ struct pathspec_item *item = (*pathspec)->items+i;
+ const char *path = path_list->items[i].string;
+
+ (*pathspec)->has_wildcard |= init_pathspec_item( item, path );
+ }
+
+ qsort((*pathspec)->items, (*pathspec)->nr,
+ sizeof(struct pathspec_item), pathspec_item_cmp);
+
+ /*
+ * Please comment/review:
+ *
+ * Here, pathspec->raw is NULL despite pathspec->nr being non-zero.
+ * A possible problem might arrise if other areas of the code make
+ * the assumption that a NULL pathspec->raw means that pathspec->nr
+ * is zero.
+ */
+
+ return 0;
+}
+
int init_pathspec(struct pathspec *pathspec, const char **paths)
{
const char **p = paths;
@@ -1279,11 +1324,7 @@ int init_pathspec(struct pathspec *pathspec, const char **paths)
struct pathspec_item *item = pathspec->items+i;
const char *path = paths[i];
- item->match = path;
- item->len = strlen(path);
- item->use_wildcard = !no_wildcard(path);
- if (item->use_wildcard)
- pathspec->has_wildcard = 1;
+ pathspec->has_wildcard |= init_pathspec_item( item, path );
}
qsort(pathspec->items, pathspec->nr,
@@ -1296,4 +1337,8 @@ void free_pathspec(struct pathspec *pathspec)
{
free(pathspec->items);
pathspec->items = NULL;
+ if (pathspec->exclude) {
+ free_pathspec(pathspec->exclude);
+ pathspec->exclude = NULL;
+ }
}
--
1.7.8.3
^ permalink raw reply related
* Re: [PATCH/RFC v2] grep: Add the option '--exclude'
From: Junio C Hamano @ 2012-01-29 23:02 UTC (permalink / raw)
To: Albert Yale; +Cc: git
In-Reply-To: <1327876934-61526-1-git-send-email-surfingalbert@gmail.com>
Albert Yale <surfingalbert@gmail.com> writes:
> Feedback would again be appreciated,
Hmm.
> [-f <file>] [-e] <pattern>
> + [-x<pattern>|--exclude<pattern>]
Compare the above two lines and notice some funny in the added one?
> @@ -124,6 +125,12 @@ OPTIONS
> Use fixed strings for patterns (don't interpret pattern
> as a regex).
>
> +-x<pattern>::
> +--exclude<pattern>::
> + In addition to those found in .gitignore (per directory) and
> + $GIT_DIR/info/exclude, also consider these patterns to be in the
> + set of the ignore rules in effect.
> +
Likewise.
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 9ce064a..9772fa4 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -530,6 +530,10 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec, int
> continue;
> if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
> continue;
> + if (pathspec->exclude &&
> + pathspec->exclude->nr &&
> + match_pathspec_depth(pathspec->exclude, ce->name, ce_namelen(ce), 0, NULL))
> + continue;
Why isn't this just
if (!match_pathspec_depth(pathspec, ce->name, ce_namelen(ce), 0, NULL))
that is, *no change whatsoever* on the existing codepaths that call
match_pathspec_depth()? Can't the "even if one of the positive pathspec
matched, if one of the excluded one matches, declare that the path does
*not* match" logic live in match_pathspec_depth() itself?
Exactly the same comment applies to all the other additions that calls
match_pathspec_depth() with pathspec->exclude as its first parameter in
this patch.
> @@ -1053,6 +1084,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
> pathspec.max_depth = opt.max_depth;
> pathspec.recursive = 1;
>
> + if( exclude_list.nr ) {
> + create_pathspec_from_string_list(&pathspec.exclude, &exclude_list);
> + pathspec.exclude->max_depth = opt.max_depth;
> + pathspec.exclude->recursive = 1;
> + }
> +
Style. Notice where SPs should be near parentheses on "if" statement in
our codebase.
^ permalink raw reply
* Re: [PATCH v4 1/2] gitweb: add project_filter to limit project list to a subdirectory
From: Jakub Narebski @ 2012-01-29 23:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Bernhard R. Link, git
In-Reply-To: <7v7h0afcc2.fsf@alter.siamese.dyndns.org>
On Sun, 29 Jan 2012, Junio C Hamano wrote:
> "Bernhard R. Link" <brl+git@mail.brlink.eu> writes:
>
> > This commit changes the project listing views (project_list,
> > project_index and opml) to limit the output to only projects in a
> > subdirectory if the new optional parameter ?pf=directory name is used.
> >
> > The change is quite minimal as git_get_projects_list already can limit
> > itself to a subdirectory (though that was previously only used for
> > 'forks').
> >
> > If there is a GITWEB_LIST file, the contents are just filtered like
> > with the forks action.
>
> Meaning, a directory is shown if it is listed on GITWEB_LIST and is a
> subdirectory of the directory specified with project_filter? If so,
> spelling it out instead of saying "just filtered like with the forks
> action" may be clearer without making the description excessively longer.
This means the following:
If $projects_list point to file with a list of projects, gitweb will
show only those project on the list which name matches $project_filter
prefix.
> > Without a GITWEB_LIST file only the given subdirectory is searched
> > for projects (like with forks) unless GITWEB_STRICT_EXPORT is enabled.
> > In the later case GITWEB_PROJECTROOT is traversed normally (unlike
> > with forks) and projects not in the directory ignored.
>
> It is unclear to me what "In the later case" refers to, even assuming that
> it is a typo of "the latter case".
>
> Do you mean "When there is no GITWEB_LIST but GITWEB_STRICT_EXPORT is set,
> project_filter that specifies anything outside GITWEB_PROJECTROOT is
> ignored"?
If $projects_list points to a directory, but $strict_export is true, then
$project_list is scanned recursively for git repositories, as without
this feature, but only those projects that begin with $project_filter
are shown.
Otherwise ($projects_list points to directory and $strict_export not true)
then $project_filter subdirectory of $projects_list is scanned recursively
for git repositories (i.e. starting from "projects_list/$project_filter").
I am not sure if this paranoia mode is really needed for $strict_export,
though.
> A more fundamental issue I have with this patch is how an end user starts
> using this. Once project_filter is set, the breadcrumbs would let the user
> click and navigate around, but in my superficial glance at the patch it is
> not apparent how the initial setting of project_filter can happen without
> the user manually adding pf= to the URL, which is a less than ideal end
> user experience.
The second patch in series adds breadcrumbs allowing to filter projects
in any per-project view.
This feature was originally intended for giving handcrafter URL with
'pf=....' to people...
> > @@ -2839,7 +2848,7 @@ sub git_get_projects_list {
> > my $pfxlen = length("$dir");
> > my $pfxdepth = ($dir =~ tr!/!!);
> > # when filtering, search only given subdirectory
> > - if ($filter) {
> > + if ($filter and not $paranoid) {
> > $dir .= "/$filter";
> > $dir =~ s!/+$!!;
> > }
> > @@ -2864,6 +2873,10 @@ sub git_get_projects_list {
> > }
> >
> > my $path = substr($File::Find::name, $pfxlen + 1);
> > + # paranoidly only filter here
> > + if ($paranoid && $filter && $path !~ m!^\Q$filter\E/!) {
> > + next;
> > + }
>
> When you find "foo" directory and a project_filter tells you to match
> "foo", because $path does not match "^foo/", it will not match (even
> though its subdirectory "foo/bar" would)?
Strictly speaking the match is on dirname of a project path; the basename
of a project does not matter. It is intended, but perhaps should be made
more clear in the commit message.
> > +sub print_nav_breadcrumbs_path {
> > + my $dirprefix = undef;
> > + while (my $part = shift) {
> > + $dirprefix .= "/" if defined $dirprefix;
> > + $dirprefix .= $part;
> > + print $cgi->a({-href => href(project => undef,
> > + project_filter => $dirprefix,
> > + action=>"project_list")},
> > + esc_html($part)) . " / ";
> > + }
> > +}
> > +
> > sub print_nav_breadcrumbs {
> > my %opts = @_;
> >
> > @@ -3841,6 +3866,8 @@ sub print_nav_breadcrumbs {
> > print " / $opts{-action_extra}";
> > }
> > print "\n";
> > + } elsif (defined $project_filter) {
> > + print_nav_breadcrumbs_path(split '/', $project_filter);
> > }
> > }
>
> Hmm.
>
> While this may not be wrong, I wonder if this is limiting a useful feature
> too narrowly. When I visit "/pub/scm /linux/kernel/git/torvals/linux.git"
> at git.kernel.org, for example, there currently are two links, "/pub/scm"
> to the toplevel and "/linux/kernel/git/torvals/linux.git" to itself. I
> often wish to see uplinks to intermediate levels like "/linux/kernel/git"
> and "/linux/kernel/git/torvalds".
>
> Perhaps that is the topic of your second patch. I dunno.
Yes, it is.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: git-gui Ctrl-U (unstage) broken
From: Pat Thoyts @ 2012-01-29 23:19 UTC (permalink / raw)
To: Victor Engmark; +Cc: git
In-Reply-To: <CAA5Ydx-mi7i7mWDYO=Cbw4g1b7LR0hw4Tcqe9gMtBoCkDRuvYA@mail.gmail.com>
Victor Engmark <victor.engmark@gmail.com> writes:
>Using the git-gui available with the default Ubuntu 10.10 repos, I'm
>not able to unstage files with the default keyboard shortcut. To
>reproduce:
>1. Change a file in the repository
>2. Run `git gui`
>3. Stage the changed file
>4. Select the changed file in the "Staged Changes (Will Commit)" list
>5. Click Ctrl-U
>
>Expected outcome: The selected file should be unstaged.
>
>Actual outcome: Nothing at all changes in the GUI.
>
>Verified that other keyboard shortcuts work: Ctrl-T, Ctrl-I, Ctrl--,
>Ctrl-+, F5. These (except Ctrl-T, obviously) were tested in* both the
>"Unstaged Changes" and "Staged Changes (Will Commit)" listsp
>
>* That is, after focusing a single element in that list.
>
>Version info:
>
>git-gui version 0.12.0.64.g89d6
>git version 1.7.1
>
>Tcl/Tk version 8.5.8
>Aspell 0.60.6, en_US
I checked this with the current version (gitgui-0.16.0) and it works ok
for me (on windows) - ie: ctrl-u unstaged a selected file.
--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD
^ permalink raw reply
* [PATCH 0/3] completion: trivial cleanups
From: Felipe Contreras @ 2012-01-29 23:41 UTC (permalink / raw)
To: git; +Cc: Felipe Contreras
And an improvement for zsh.
Felipe Contreras (3):
completion: be nicer with zsh
completion: remove old code
completion: remove unused code
contrib/completion/git-completion.bash | 47 +++++---------------------------
1 files changed, 7 insertions(+), 40 deletions(-)
--
1.7.8.3
^ 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