Re: bash completion with colour hints

2012-09-26 Thread Junio C Hamano
Simon Oosthoek  writes:

> I read the guide and now I have some questions:
>
> - It suggests to use the oldest commit that contains the "bug" and can
> support the fix. This would be the very first mention of __git_ps1
> function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585

You could claim that the lack of coloring is a bug, but nobody
complained about it so far (and I personally hate coloring in
prompts as they are distracting noise, and would reject a patch if
it weren't made conditional), so I would think this is more about
"adding a feature the users can choose to use but they do not have
to".

We do not usually add new features to maintenance tracks, so the
result of applying the patch does not have to be merge-able to maint
or amything older.  I would base the patch on v1.7.12 (the latest
stable release) if I were you.

> - I read that git-prompt.sh is meant to support bash and zsh, I have
> only tested it on bash. Should I attempt to test it on zsh or is there a
> kind person with zsh as his/her shell to test it for me?

That is something you should ask on list, like you did here, but the
most effective way to do so is do so when you send a patch you
worked on and tested with bash.  Say "I've tested it only with bash;
can you please take a look?" and Cc the folks you find in the output
of "git log contrib/completion/" who worked on making it workable
with zsh.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] completion: improve shell expansion of items

2012-09-26 Thread Jeff King
On Thu, Sep 27, 2012 at 02:28:55AM -0400, Jeff King wrote:

> Thanks for reminding me to time. I noticed your a31e626 while digging in
> the history, but forgot that I wanted to do a timing test. Sadly, the
> results are very discouraging. Doing a similar test to your 10,000-refs,
> I get:
> 
>   $ refs=$(seq 1 1)
> 
>   $ . git-completion.bash.old
>   $ time __gitcomp_nl "$refs"
>   real0m0.065s
>   user0m0.064s
>   sys 0m0.004s
> 
>   $ . git-completion.bash.new
>   $ time __gitcomp_nl "$refs"
>   real0m1.799s
>   user0m1.828s
>   sys 0m0.036s
> 
> So, a 2700% slowdown. Yuck.
> 
> I also tried running it through sed instead of iterating in bash. I know
> that some people will not like the fork, but curiously enough, it was
> not that much faster. Which makes me wonder if part of the slowdown is
> actually bash unquoting the result in compgen.

Ah. The problem is that most of the load comes from my patch 4/3, which
does a separate iteration. Here are the numbers after just patch 3:

  $ time __gitcomp_nl "$refs"
  real0m0.344s
  user0m0.392s
  sys 0m0.040s

Slower, but not nearly as painful. Here are the numbers using sed[1]
instead:

  $ time __gitcomp_nl "$refs"
  real0m0.100s
  user0m0.084s
  sys 0m0.016s

So a little slower, but probably acceptable. We could maybe do the same
trick on the output side (although it is a little trickier there,
because we need it in a bash array). Of course, this is Linux; the fork
for sed is way more expensive on some systems.

Still, I'd be curious to see it with the callers (e.g., __git_refs)
doing their own quoting. I'd worry that it would become a maintenance
headache, but perhaps we don't have that many lists we feed (there are a
lot of calls to gitcomp_nl, but they are mostly feeding __git_refs).

-Peff

[1] For reference, that patch is:

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 1fc43f7..5ff3742 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,16 +225,15 @@ __git_quote()
 fi
 fi
 
-# Quotes each element of an IFS-delimited list for shell reuse
+# Quotes each element of a newline-delimited list for shell reuse
 __git_quote()
 {
-   local i
-   local delim
-   for i in $1; do
-   local quoted=${i//\'/\'\\\'\'}
-   printf "${delim:+$IFS}'%s'" "$quoted"
-   delim=t
-   done
+   echo "$1" |
+   sed "
+ s/'/'''/g
+ s/^/'/
+ s/$/'/
+   "
 }
 
 # Generates completion reply with compgen, appending a space to possible
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring the location of ~/.gitconfig

2012-09-26 Thread Anurag Priyam
On Thu, Sep 27, 2012 at 11:39 AM, Junio C Hamano  wrote:
> Ramkumar Ramachandra  writes:
>
>> I'd like to configure the location of ~/.gitconfig through an
>> environment variable.  My usecase is a simple enough: I have a
>> repository with all my dotfiles, and I don't want to symlink
>> ~/dotfiles/.gitconfig from $HOME after cloning it.  Does anyone else
>> think the feature will be useful?
>
> Not me. For that particular use case, my approach (long before I
> switched the vcs that controls my dotfiles to git) have always been
> to have ~/src that is version controlled, with a Makefile that
> builds/adjusts dotfiles appropriately for each box and installs them
> in the proper place.

Makefile is what I wanted to avoid when I suggested Ram that maybe Git
could _optionally_ read the location of global gitconfig from an
environment variable that can be exported in zshenv or bash_profile.

I don't think either way is the best way of managing dotfiles.  Just a
matter of preference.  The environment variable approach doesn't
require you to run `make` everytime you sync your dotfiles across
different machines, and that is what I like.

Also, Git allows configuring the location of template directory via an
environment variable (GIT_TEMPLATE_DIR).  Since Git has already fixed
the location of global gitconfig, it might as well read the location
of template directory from there (init.templatedir).  Why the added
flexibility?  Well, I have been exploiting the feature to manage git
templates through my zsh configuration.

-- 
Anurag Priyam
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

2012-09-26 Thread Junio C Hamano
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: A generalization of git blame

2012-09-26 Thread Junio C Hamano
xm...@cs.wisc.edu writes:

>> It largely depends on how the user would interact with your program,
>> which is totally unclear as we haven't seen any part of it.  I do
>> not think we have enough information to answer the question at this
>> point.
>
> Do you mean it largely depends on the diversity of options on input and
> output formats? ...
> ... I know this is not enough for a tool. So this is case, does "how the user
> would interact with your program" mean that I should add ...

I am not saying anything about what you should or should not do. It
is your program, and we haven't seen anything about it, other than
handwaving, what good it will do to its users, so I am not qualified
to make such a comment on it yet.  What I meant by "how the users
would interact with..." are things like this:

 - Why users would want to use it in the first place?  What are
   missing from existing tool set?

 - What kind of questions do users ask to the program and how do
   they ask them?

 - How are the answers to these questions presented by the program
   to the users?

 - How do users interpret and use these answers in what way?

Notice that I didn't include "How do you compute the answers?"  When
we are initially evaluating a feature at "how do they interact" level,
we are not interested in the implementation at all.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] completion: improve shell expansion of items

2012-09-26 Thread Jeff King
On Thu, Sep 27, 2012 at 02:37:51AM +0200, SZEDER Gábor wrote:

> > +# Quotes each element of an IFS-delimited list for shell reuse
> > +__git_quote()
> > +{
> > +   local i
> > +   local delim
> > +   for i in $1; do
> > +   local quoted=${i//\'/\'\\\'\'}
> > +   printf "${delim:+$IFS}'%s'" "$quoted"
> > +   delim=t
> > +   done
> > +}
> [...]
>
> Iterating through all possible completion words undermines the main
> reason for __gitcomp_nl()'s existence: to handle potentially large
> number of possible completion words faster than the old __gitcomp().
> If we really have to iterate in a subshell, then it would perhaps be
> better to drop __gitcomp_nl(), go back to using __gitcomp(), and
> modify that instead.

Thanks for reminding me to time. I noticed your a31e626 while digging in
the history, but forgot that I wanted to do a timing test. Sadly, the
results are very discouraging. Doing a similar test to your 10,000-refs,
I get:

  $ refs=$(seq 1 1)

  $ . git-completion.bash.old
  $ time __gitcomp_nl "$refs"
  real0m0.065s
  user0m0.064s
  sys 0m0.004s

  $ . git-completion.bash.new
  $ time __gitcomp_nl "$refs"
  real0m1.799s
  user0m1.828s
  sys 0m0.036s

So, a 2700% slowdown. Yuck.

I also tried running it through sed instead of iterating in bash. I know
that some people will not like the fork, but curiously enough, it was
not that much faster. Which makes me wonder if part of the slowdown is
actually bash unquoting the result in compgen.

> After all, anyone could drop a file called git-cmd-${meta} on his
> $PATH, and then get cmd- offered, because completion of git commands
> still goes through __gitcomp().

Yeah. I wasn't sure if __gitcomp() actually got used on any arbitrary
data, but that's a good example.

I'm not sure where to go next. I guess we could try pre-quoting via git
when generating the list of refs (or files, or whatever) and hope that
is faster.

With this patch as it is, I'm not sure the slowdown is worth it. Yes,
it's more correct in the face of metacharacters, but those are the
uncommon case by a large margin. I'd hate to have a performance
regression in the common case just for that.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CRLF, LF ... CR ?

2012-09-26 Thread Junio C Hamano
David Aguilar  writes:

> That said, perhaps the "autocrlf" code is simple enough that it
> could be easily tweaked to also handle this special case,...

I wouldn't be surprised if it is quite simple.

We (actually Linus, IIRC) simply declared from the get-go that it is
not worth spending any line of code only to worry about pre OSX
Macintosh when we did the end-of-line stuff, and nobody so far
showed any need.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring the location of ~/.gitconfig

2012-09-26 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> I'd like to configure the location of ~/.gitconfig through an
> environment variable.  My usecase is a simple enough: I have a
> repository with all my dotfiles, and I don't want to symlink
> ~/dotfiles/.gitconfig from $HOME after cloning it.  Does anyone else
> think the feature will be useful?

Not me. For that particular use case, my approach (long before I
switched the vcs that controls my dotfiles to git) have always been
to have ~/src that is version controlled, with a Makefile that
builds/adjusts dotfiles appropriately for each box and installs them
in the proper place.

Of course, if .gitconfig _is_ the same across all boxes I use, that
Makefile is likely to "install" by making a symlink ~/.gitconfig
that points at src/dot/gitconfig without "building".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: A generalization of git blame

2012-09-26 Thread xmeng
> It largely depends on how the user would interact with your program,
> which is totally unclear as we haven't seen any part of it.  I do
> not think we have enough information to answer the question at this
> point.

Do you mean it largely depends on the diversity of options on input and
output formats? Currently I just take a path and output lists like:

53: (1da177e4c3f41524e886b7f1b8a0c1fc7321cac2,Linus Torvalds)
(201b6264ff3865090747f58f48e087c3a35e0dbc,Christoph Lameter)
(e2bdb933ab8b7db71c318a4ddcf78a9fffd61ecb,Hugh Dickins)
54: (7cf9c2c76c1a17b32f2da85b50cd4fe468ed44b5,Nick Piggin)
(e2bdb933ab8b7db71c318a4ddcf78a9fffd61ecb,Hugh Dickins)
55: (e2bdb933ab8b7db71c318a4ddcf78a9fffd61ecb,Hugh Dickins)
56: (1da177e4c3f41524e886b7f1b8a0c1fc7321cac2,Linus Torvalds)

This shows for all lines in the specified file, all commits that have
changed a line.

I know this is not enough for a tool. So this is case, does "how the user
would interact with your program" mean that I should add more options like
which revision to start, some parameters in the algorithm, choice on
different diff algorithms, and options for customizing the contents of
output?

> If it is a trivial script that largely depends on what we already
> ship, I would not mind carrying it in contrib/.  If it is anything
> substantial and substantially useful, however, I would suspect that
> you are better off not be in in my tree, but rather want to be
> independent.  Finishing it to be useful for your purpose, publishing
> it somewhere people can take a look at and adding a pointer to
> https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools is
> probably where you would want to start.

I think it would be helpful for our discussion if I first briefly
introduce my approach. It starts at the current head and tracks all the
lines in the specified file. It then goes along the commit graph
topologically. For any two commits A and B connected by an edge from A to
B (commit B is the parent of commit A) and if A is not a merge commit, it
first calls the tree-diff interface to get the added lines  in A and
deleted lines in B, and detect renaming. Then it applies ldiff algorithm
(http://sourceforge.net/projects/ldiff/ . It is in perl. I adapt it into c
in my program) to match the added lines and deleted lines. The result is
that now we have some added lines, some deleted lines and some changed
lines. For added lines, I know the this commit is the last one to change
it. For changed lines, I add this commit to the final result and keep
tracking them. Currently I ignore those deleted lines. But if A is a merge
commit, it doesn't change authorship except for merge conflicts.

I did start with implementing my approach with python. The script keeps
calling git-log to get the diff and then apply ldiff. The problem is I
cannot store whole git-log result in the memory. Even the diff contents of
a specified file can explode my memory. So then I only query for a small
amount of commits each time. This makes my program extremely slow. It
takes about half hour to analyze a file in a project with 15000 commits.

Then I refactor my code based on git internal api. It turns out to work
very well. Now the program can finish in 10sec instead of half hour. And
now I can also apply my tool to larger project like linux kernel.

So if I want other people to use my tool, I firstly add an entry on the
webpage, and then other interested people can get my tool by cloning from
my git branch?


>  (1) We won't know how useful and interesting your contribution be
>  for our audience, until we see it; and
>
>  (2) If you truly believe in your work (find it useful, find writing
>  it fun, etc.), that would be incentive enough for you to work
>  on it, whether or not the result will land in my tree.  You
>  should instead aim for something so brilliant that we would
>  come to you begging for your permission to include it in our
>  project.
>
> I think it applies to your inquiry as well.

You are definitely right. Actually I have spent a lot of time on my tool.
I just want to know if there are some issues that I should know, otherwise
it will prevent me from publishing it or prevent it from becoming a
standard git built-in tool.

Thanks


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote:

> Ideally, yes.  Is there an easy way to access that from Perl? (and
> for the older versions of SVN folks people are running).

Subversion's swig bindings only wrap a few apr functions and do not
depend on fuller apr bindings.

Something like svn_dirent_is_under_root() could be useful.  It uses
whatever base path you choose.  I haven't tried using base_path=""
yet.  New in Subversion 1.7, but that would be ok --- an imperfect
fallback for older Subversion versions would be fine.

Unfortunately, from swig/core.i: "SWIG can't digest these functions
yet, so ignore them for now. TODO: make them work."

svn_client_args_to_target_array2() is exposed and would probably be
perfect.  I can't seem to find how to create an apr_array_header_t
to pass as its first argument, alas.

> Perhaps we can expose equivalent functionality in git via
> git-rev-parse instead?

It might be possible to add a git-svn--helper command that links to
libsvn or apr, but ick.  The trouble is it's not clear at all how
Subversion's perl API was *designed* to be used.

Hm.
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Using bitmaps to accelerate fetch and clone

2012-09-26 Thread Shawn Pearce
Google has published a series of patches (see links below) to JGit to
improve fetch and clone performance by adding compressed bitmaps to
the pack-*.idx structure.

Operation   Index V2   Index VE003
Clone   37530ms (524.06 MiB) 82ms (524.06 MiB)
Fetch (1 commit back)  75ms 107ms
Fetch (10 commits back)   456ms (269.51 KiB)341ms (265.19 KiB)
Fetch (100 commits back)  449ms (269.91 KiB)337ms (267.28 KiB)
Fetch (1000 commits back)2229ms ( 14.75 MiB)189ms ( 14.42 MiB)
Fetch (1 commits back)   2177ms ( 16.30 MiB)254ms ( 15.88 MiB)
Fetch (10 commits back) 14340ms (185.83 MiB)   1655ms (189.39 MiB)

In the table the repository tested was Android's
platform/frameworks/base. The time shown is the time spent in the
"Counting objects" phase of creating a pack for a client using the
git:// protocol. The byte size shown is the size of the pack
transferred to the client, and "commits back" describes how far behind
the client was from the server when it started the fetch. In all test
runs the client was git-core and the server was JGit on the same
machine.

The amount of disk space used by the compressed bitmaps is tunable,
but averages 10-15% of the pack-*.idx file size. So about 8 MiB of
additional space for this repository. A repository owner can reduce
the worst case time used in the 10 commit back case by using
slightly more disk and positioning more bitmaps more frequently
throughout history. The code doesn't do this by default because the
expectation is that a client is probably not 100k commits behind.
Instead it populates bitmaps at all branch and tag tips, and densely
(every few hundred commits) near the tips, and spaces them out more
the further back in history it goes. We assume the older history is
accessed less often, and doesn't need to waste additional disk space
or precious buffer cache.

The basic gist of the implementation is a bitmap has a 1 bit set for
each object that is reachable from the commit the bitmap is associated
with. An index file may have a unique bitmap for hundreds of commits
in the corresponding pack file. The set of objects to send is
performed by doing a simple computation:

  OR (all want lines) AND NOT OR (all have lines)

There are two key patches in the series that implement the file format
change and logic involved:

* https://git.eclipse.org/r/7939

  Defines the new E003 index format and the bit set
  implementation logic.

* https://git.eclipse.org/r/7940

  Uses E003 indexes when available to make packs, and
  the logic required to make E003 format indexes during GC.

:-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] completion: improve shell expansion of items

2012-09-26 Thread SZEDER Gábor
Hi,

On Wed, Sep 26, 2012 at 05:51:19PM -0400, Jeff King wrote:
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index be800e0..b0416ea 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -225,6 +225,18 @@ fi
>  fi
>  fi
>  
> +# Quotes each element of an IFS-delimited list for shell reuse
> +__git_quote()
> +{
> + local i
> + local delim
> + for i in $1; do
> + local quoted=${i//\'/\'\\\'\'}
> + printf "${delim:+$IFS}'%s'" "$quoted"
> + delim=t
> + done
> +}
> +
>  # Generates completion reply with compgen, appending a space to possible
>  # completion words, if necessary.
>  # It accepts 1 to 4 arguments:
> @@ -261,7 +273,7 @@ __gitcomp_nl ()
>  __gitcomp_nl ()
>  {
>   local IFS=$'\n'
> - COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
> + COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" -- 
> "${3-$cur}"))

Iterating through all possible completion words undermines the main
reason for __gitcomp_nl()'s existence: to handle potentially large
number of possible completion words faster than the old __gitcomp().
If we really have to iterate in a subshell, then it would perhaps be
better to drop __gitcomp_nl(), go back to using __gitcomp(), and
modify that instead.

After all, anyone could drop a file called git-cmd-${meta} on his
$PATH, and then get cmd- offered, because completion of git commands
still goes through __gitcomp().

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder  wrote:
> Maybe we can use apr_filepath_merge() to avoid reinventing the wheel?

Ideally, yes.  Is there an easy way to access that from Perl? (and
for the older versions of SVN folks people are running).

Perhaps we can expose equivalent functionality in git via
git-rev-parse instead?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote:

> It should probably just return the root path ("c:/" and
> "http://www.example.com/"; respectively).

That means recognizing drive letters and URLs.  Hm.

Subversion commands seem to use svn_client_args_to_target_array2
to canonicalize arguments.  It does something like the following:

 1. split at @PEG revision
 2. check if it looks like a URL (svn_path_is_url).  If so:

i.   urlencode characters with high bit set (svn_path_uri_from_iri)
ii.  urlencode some other special characters (svn_path_uri_autoescape)
 (that is: [ "<>\\^`{|}])
iii. (on Windows) convert backslashes to forward slashes
iv.  complain if there are any '..' (svn_path_is_backpath_present)
v.   make url scheme and hostname lowercase, strip default portnumber,
 strip trailing '/', collapse '/'-es including urlencoded %2F,
 strip '.' and '%2E' components, make drive letter in file:///
 URLs uppercase, urlencode uri-special characters
 (svn_uri_canonicalize)

   Otherwise:

i.   canonicalize case, handle '..' and '.' components, and make
 path separators into '/'
 (apr_filepath_merge(APR_FILEPATH_TRUENAME))
ii.  strip trailing '/', collapse '/'-es except in UNC paths,
 strip '.' components, make drive letter uppercase
 (svn_dirent_canonicalize)
iii. deal with "truepath collisions" (unwanted case
 canonicalizations)
iv.  reject .svn and _svn directories

Maybe we can use apr_filepath_merge() to avoid reinventing the wheel?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/3] completion: quote completions we find

2012-09-26 Thread Jeff King
On Wed, Sep 26, 2012 at 05:51:19PM -0400, Jeff King wrote:

> This is not a complete fix, as the completion result does
> will still contain metacharacters, so it would need extra
> quoting to actually be used on a command line. But it's
> still a step in the right direction, because:
> [...]
>   2. We don't know yet what the final fix will look like,
>  but this is probably the first step towards it. It
>  handles quoting on the input side of compgen; the next
>  step will likely be handling the quoting on the output
>  side of compgen to yield a usable string for the
>  command line.

Here is on attempt at that. It seems to work for me, but I know it is
not what bash does internally with:

  $ ls
  name with spaces
  $ cat name

Because in bash's internal case, it knows that "name with spaces" is the
real entry (because if you have many entries and double-tab, it shows it
without the quotes), and only later adds the quoting.

So while this works, I'm not sure if it is optimal or if it has any
weird side effects.

-- >8 --
Subject: [PATCH] completion: quote completions we find

If you try to complete a filename with spaces, you might end
up with this:

  $ git show HEAD:name
  $ git show HEAD:name with spaces

which is technically correct, but does not help you, since
the shell will split the words as soon as you hit enter.
Instead, let's quote completions coming out of __gitcomp_nl
to yield:

  $ git show HEAD:'name with spaces'

Signed-off-by: Jeff King 
---
 contrib/completion/git-completion.bash | 23 +++
 t/t9902-completion.sh  |  6 +++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index b0416ea..1fc43f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -274,6 +274,29 @@ __gitcomp_nl ()
 {
local IFS=$'\n'
COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" -- 
"${3-$cur}"))
+
+   local i
+   for (( i=0; i < ${#COMPREPLY[@]}; i++ )); do
+   # Hack to handle the extra space we add for some
+   # entries, since we use "-o nospace".
+   local stripped
+   case "${COMPREPLY[$i]}" in
+   *\ )
+   stripped=' '
+   COMPREPLY[$i]=${COMPREPLY[$i]% }
+   ;;
+   esac
+
+   # Now we can check if we need actual quoting.
+   case "${COMPREPLY[$i]}" in
+   *[\ \$\'\"\(\)]*)
+   COMPREPLY[$i]="'${COMPREPLY[$i]//\'/\'\\\'\'}'"
+   ;;
+   esac
+
+   # And then restore any stripped space.
+   COMPREPLY[$i]="${COMPREPLY[$i]}$stripped"
+   done
 }
 
 __git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index da67705..d2c5104 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -274,7 +274,7 @@ test_expect_success 'complete tree filename with spaces' '
git add . &&
git commit -m spaces &&
test_completion_long "git show HEAD:nam" <<-\EOF
-   name with spaces_
+   '\''name with spaces'\''_
EOF
 '
 
@@ -283,8 +283,8 @@ test_expect_success 'complete tree filename with 
metacharacters' '
git add . &&
git commit -m meta &&
test_completion_long "git show HEAD:nam" <<-\EOF
-   name with ${meta}_
-   name with spaces_
+   '\''name with ${meta}'\''_
+   '\''name with spaces'\''_
EOF
 '
 
-- 
1.7.12.10.g31da6dd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder  wrote:
> Eric Wong wrote:
> > That said, I'd favor an implementation that split on m{/+} and
> > collapsed as Michael mentioned.
> 
> Sounds sensible.  Is canonicalize_path responsible for collapsing
> runs of slashes?  What should _collapse_dotdot do to
> "c:/.." or "http://www.example.com/..";?

It should probably just return the root path ("c:/" and
"http://www.example.com/"; respectively).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] completion: improve shell expansion of items

2012-09-26 Thread Jeff King
The current completion code doesn't deal properly with items
(tags, branches, etc.) that have ${} in them because they
get expanded by bash while using compgen.

This patch is a rewrite of Felipe Contreras's 25ae7cf, which
attempted to fix the problem by quoting the values we pass
to __gitcomp_nl. However, that patch ended up quoting the
whole list as a single item, which broke other completions.
Instead, we need to split the newline-delimited list into
elements and quote each one individually.

This is not a complete fix, as the completion result does
will still contain metacharacters, so it would need extra
quoting to actually be used on a command line. But it's
still a step in the right direction, because:

  1. The current code's expansion means that things that are
 broken expansions (e.g., "${foo:bar}") will actually
 cause the completion function to barf, breaking all
 completion (even if you weren't going to complete that
 item). This patch fixes that so you can at least
 complete "foo" when "${foo:bar}" exists.

  2. We don't know yet what the final fix will look like,
 but this is probably the first step towards it. It
 handles quoting on the input side of compgen; the next
 step will likely be handling the quoting on the output
 side of compgen to yield a usable string for the
 command line.

Signed-off-by: Jeff King 
---
 contrib/completion/git-completion.bash | 14 +-
 t/t9902-completion.sh  |  2 +-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index be800e0..b0416ea 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -225,6 +225,18 @@ fi
 fi
 fi
 
+# Quotes each element of an IFS-delimited list for shell reuse
+__git_quote()
+{
+   local i
+   local delim
+   for i in $1; do
+   local quoted=${i//\'/\'\\\'\'}
+   printf "${delim:+$IFS}'%s'" "$quoted"
+   delim=t
+   done
+}
+
 # Generates completion reply with compgen, appending a space to possible
 # completion words, if necessary.
 # It accepts 1 to 4 arguments:
@@ -261,7 +273,7 @@ __gitcomp_nl ()
 __gitcomp_nl ()
 {
local IFS=$'\n'
-   COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
+   COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$(__git_quote "$1")" -- 
"${3-$cur}"))
 }
 
 __git_heads ()
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index cbd0fb6..da67705 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -278,7 +278,7 @@ test_expect_success 'complete tree filename with spaces' '
EOF
 '
 
-test_expect_failure 'complete tree filename with metacharacters' '
+test_expect_success 'complete tree filename with metacharacters' '
echo content >"name with \${meta}" &&
git add . &&
git commit -m meta &&
-- 
1.7.12.10.g31da6dd
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/3] t9902: add completion tests for "odd" filenames

2012-09-26 Thread Jeff King
We correctly handle completion items with spaces just fine,
since we pass the lists around with newline delimiters.
However, we do not handle filenames with shell
metacharacters, as "compgen -W" performs expansion on the
list we give it.

Signed-off-by: Jeff King 
---
Actually, these vectors are not strictly correct, as I think ultimately
we would like to return

  'name with spaces'

with quotes.  But this lets us test that at least the basics work, and
we can update the test vectors later.

It would be nice to test completion on a file with an embedded newline
(which will also not work), but I'm not even sure what the result is
supposed to look like, so I couldn't write a sane test case. Since I
don't plan on fixing that anytime soon anyway, I think it's sane to just
leave it until later if somebody actually cares.

 t/t9902-completion.sh | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 2fc833a..cbd0fb6 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -269,4 +269,23 @@ test_expect_success ': completes paths' '
EOF
 '
 
+test_expect_success 'complete tree filename with spaces' '
+   echo content >"name with spaces" &&
+   git add . &&
+   git commit -m spaces &&
+   test_completion_long "git show HEAD:nam" <<-\EOF
+   name with spaces_
+   EOF
+'
+
+test_expect_failure 'complete tree filename with metacharacters' '
+   echo content >"name with \${meta}" &&
+   git add . &&
+   git commit -m meta &&
+   test_completion_long "git show HEAD:nam" <<-\EOF
+   name with ${meta}_
+   name with spaces_
+   EOF
+'
+
 test_done
-- 
1.7.12.10.g31da6dd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/3] t9902: add a few basic completion tests

2012-09-26 Thread Jeff King
We were not testing ref or tree completion at all. Let's
give them even basic sanity checks to avoid regressions.

Signed-off-by: Jeff King 
---
This would have caught the recent breakage, and also paves the way for
testing the new fix.

 t/t9902-completion.sh | 41 +
 1 file changed, 41 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 92d7eb4..2fc833a 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -61,6 +61,15 @@ test_completion ()
test_cmp expected out
 }
 
+# Like test_completion, but reads expectation from stdin,
+# which is convenient when it is multiline. We also process "_" into
+# spaces to make test vectors more readable.
+test_completion_long ()
+{
+   tr _ " " >expected &&
+   test_completion "$1"
+}
+
 newline=$'\n'
 
 test_expect_success '__gitcomp - trailing space - options' '
@@ -228,4 +237,36 @@ test_expect_success 'general options plus command' '
test_completion "git --no-replace-objects check" "checkout "
 '
 
+test_expect_success 'setup for ref completion' '
+   echo content >file1 &&
+   echo more >file2 &&
+   git add . &&
+   git commit -m one &&
+   git branch mybranch &&
+   git tag mytag
+'
+
+test_expect_success 'checkout completes ref names' '
+   test_completion_long "git checkout m" <<-\EOF
+   master_
+   mybranch_
+   mytag_
+   EOF
+'
+
+test_expect_success 'show completes all refs' '
+   test_completion_long "git show m" <<-\EOF
+   master_
+   mybranch_
+   mytag_
+   EOF
+'
+
+test_expect_success ': completes paths' '
+   test_completion_long "git show mytag:f" <<-\EOF
+   file1_
+   file2_
+   EOF
+'
+
 test_done
-- 
1.7.12.10.g31da6dd

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Revert "completion: fix shell expansion of items"

2012-09-26 Thread Jeff King
On Wed, Sep 26, 2012 at 12:37:25AM +0200, SZEDER Gábor wrote:

> I looked into this issue, but quickly got lost in quoting-escaping
> hell.  Ideally we could do some quoting in __gitcomp_nl(), so it would
> work for all kinds of input, but I couldn't come up with anything
> working.  Alternatively, we could modify __gitcomp_nl()'s callers, or
> rather the helper functions supplying input to __gitcomp_nl() to do
> the quoting or escaping themselves.  Actually, that's quite easy for
> local refs, at least, because for-each-ref's builtin quoting support
> does the trick:

I feel like insanity lies that way, because every caller is going to
have to do its own quoting. On the other hand, I think it would be the
only way to handle completion of entries with embedded newlines (as it
is now, we pass in a newline-delimited list with no opportunity for
quoting).

Here's a simple patch series that fixes the problem and adds a few basic
sanity checks.

  [1/3]: t9902: add a few basic completion tests
  [2/3]: t9902: add completion tests for "odd" filenames
  [3/3]: completion: improve shell expansion of items

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Eric Wong wrote:

> That said, I'd favor an implementation that split on m{/+} and
> collapsed as Michael mentioned.

Sounds sensible.  Is canonicalize_path responsible for collapsing
runs of slashes?  What should _collapse_dotdot do to
"c:/.." or "http://www.example.com/..";?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Eric Wong
Jonathan Nieder  wrote:
> Hi,
> 
> Michael G Schwern wrote:
> > On 2012.7.30 12:51 PM, Eric Wong wrote:
> >> Michael G Schwern wrote:
> 
> >>> _collapse_dotdot() works better than the existing regex did.
> >>
> >> I don't dispute it's better, but it's worth explaining in the commit
> >> message to reviewers why something is "better".
> >
> > Yeah.  I figured the tests covered that.
> 
> Now I'm tripping up on the same thing.  Eric, did you ever find out
> what the motivation for this patch was?  Is SVN 1.7 more persnickety
> about runs of multiple slashes in a row or something, or is it more
> of an aesthetic thing?

I'm not sure about this case specifically, but SVN has (and will likely
become) more persnickety over time.  I haven't had a chance to check SVN
itself, but I think being defensive and giving it prettier paths will
be safer in the future.

That said, I'd favor an implementation that split on m{/+} and
collapsed as Michael mentioned.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug in Submodule add

2012-09-26 Thread Jens Lehmann
Am 26.09.2012 06:18, schrieb Jonathan Johnson:
> I believe I have found an issue with the way `submodule add` detects a 
> submodule that already exists in the repository. 

Yes, this is an issue and thanks for the detailed report.

> To reproduce
> 
> 1) add a git submodule in a specific location (we'll say it's at 
> `./submodule/location`)
> 2) go through the normal steps of removing a submodule, as listed here - 
> https://git.wiki.kernel.org/index.php/GitSubmoduleTutorial
> 3) Now the submodule is completely removed and there is no reference to it in 
> .gitmodules or .git/config
> 4) Re-add a different repository at the same location (`./submodule/location`)
> 
> Expected - The new submodule repository will be set up at 
> ./submodule/location and have the new repository as its origin
> 
> What Actually Happens - The new submodule uses the existing `$gitdir` (old 
> repository) as the actual backing repository to the submodule, but the new 
> repository is reflected in .gitmodules and .git/config.
> 
> So to recap, the result is that `git remote show origin`  in the submodule 
> shows a different origin than is in .gitmodules and .git/config
> 
> One simple step to remedy this would be to add the deletion of the backing 
> repository from the .git/modules directory, but again, I think an actual 
> command to take care of all of these steps is in order anyways.  Not sure you 
> want to encourage people poking around in the .git directory.

Unfortunately just throwing away the old repository under .git/modules,
whether manually or by a git command, is no real solution here: it would
make it impossible to go back to a commit which records the old submodule
and check that out again.

The reason for this issue is that the submodule path is used as its name
by "git submodule add". While we could check this type of conflict locally,
we can't really avoid it due to the distributed nature of git (somebody
else could add a different repo under the same path - and thus the same
name - in another clone of the repo).

The only long term solution I can think of is to use some kind of UUID for
the name, so that the names of newly added submodules won't have a chance
to clash anymore. For the short term aborting "git submodule add" when a
submodule of that name already exists in .git/modules of the superproject
together with the ability to provide a custom name might at least solve
the local clashes.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/7] Add join_paths() to safely concatenate paths.

2012-09-26 Thread Jonathan Nieder
Hi,

Michael G. Schwern wrote:

> Otherwise you might wind up with things like...
>
> my $path1 = undef;
> my $path2 = 'foo';
> my $path = $path1 . '/' . $path2;
>
> creating '/foo'.  Or this...
>
> my $path1 = 'foo/';
> my $path2 = 'bar';
> my $path = $path1 . '/' . $path2;
>
> creating 'foo//bar'.

I'm still puzzled by this one, too.  I don't understand the
motivation.  Is this to make joining paths less fragile, by preserving
the property that join_paths($a, $b) names the directory you would get
to by first chdir-ing into $a and then into $b?

It would be easier to understand as two patches: first, one that
extracts join_paths without any functional change, and then one that
changes its implementation with an explanation for what positive
functional effect that would have.

> --- a/git-svn.perl
> +++ b/git-svn.perl
[...]
> @@ -1275,7 +1276,7 @@ sub get_svnprops {
>   $path = $cmd_dir_prefix . $path;
>   fatal("No such file or directory: $path") unless -e $path;
>   my $is_dir = -d $path ? 1 : 0;
> - $path = $gs->{path} . '/' . $path;
> + $path = join_paths($gs->{path}, $path);
>  
>   # canonicalize the path (otherwise libsvn will abort or fail to
>   # find the file)

This can't be for the //-collapsing effect since the path is about
to be canonicalized.  It can't be for the initial-/ effect since
that is stripped away by canonicalization, too.

So no functional effect here, good or bad.

[...]
> --- a/perl/Git/SVN.pm
> +++ b/perl/Git/SVN.pm
[...]
> @@ -316,9 +320,7 @@ sub init_remote_config {
>   }
>   my $old_path = $self->path;
>   $url =~ s!^\Q$min_url\E(/|$)!!;
> - if (length $old_path) {
> - $url .= "/$old_path";
> - }
> + $url = join_paths($url, $old_path);
>   $self->path($url);

This is probably not for the normal //-collapsing effect because
$url already has its trailing / stripped off.  Maybe it is for
cases where $old_path has leading slashes or $min_url has multiple
trailing ones?

In the end it shouldn't make a difference, once a later patch teaches
Git::SVN->path to canonicalize.

Is the functional change in this patch for aesthetic reasons, or is
there some other component (perhaps in a later patch) that relies on
it?

Thanks again for your help,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Can git pull from a mercurial repository?

2012-09-26 Thread Cosmin Stejerean
I definitely wrote git-hg for the purpose of checking out a mercurial
repo so I can develop locally against it with git and then submit
patches. Getting push support was never really a priority for me.
Someone did eventually contribute some mechanism for pushing things
back in a pull request, so I merged it in. I occasionally get pull
requests for minor things around push, so it seems like at least a few
people are using it.

The way to get proper push support would be to use the hg-git
extension. The git-hg-again project uses that, and if I had some time
I'd love to move git-hg over to that approach as well. Patches
welcome. Alternatively, you can always https://www.gittip.com/cosmin/
:)


- Cosmin

On Wed, Sep 26, 2012 at 4:46 AM, Georgi Chorbadzhiyski  wrote:
> Around 09/26/2012 11:46 AM, Max Horn scribbled:
>> On 26.09.2012, at 09:38, Georgi Chorbadzhiyski wrote:
>>> Around 09/25/2012 05:15 PM, Max Horn scribbled:
 I think there is a lot of demand for a "git-hg" bridge, a way to 
 seemlessly access a Mercurial repository as if it was a git repository. A 
 converse to hg-git 
>>>
>>> I've already mentioned this, but such a tool already exists and it
>>> is working very well (IMHO): http://offbytwo.com/git-hg/
>>
>> I guess this is a matter of perspective. It doesn't work at all for me 
>> because it does not really support pushing. (It does have a "push" command, 
>> but at least last time I looked, it was utterly broken; see also 
>>  for a 
>> discussion (not written by me!). I'd be happy to learn that has changed, 
>> though I just looked, and it still uses "hg convert", so I don't see how it 
>> possibly could work...
>
> I have not tested push (I'm using git-hg to sync hg repo and develop
> using git, no pushing back to hg, just sending patches).
>
> According to git-hg README "Push supported added as well although it
> is still experimental". You should report the "push" bugs to the
> author(s) they may be able to fix them.
>
> --
> Georgi Chorbadzhiyski
> http://georgi.unixsol.org/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add MALLOC_CHECK_ and MALLOC_PERTURB_ libc env to the test suite for detecting heap corruption

2012-09-26 Thread René Scharfe
Sorry for being late.  Just wanted to try out this new feature and
ended up reading this old thread.

Am 15.09.2012 01:18, schrieb Junio C Hamano:
>   t/perf/perf-lib.sh |  1 +
>   t/test-lib.sh  | 26 --
>   2 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git i/t/perf/perf-lib.sh w/t/perf/perf-lib.sh
> index a1361e5..1d0bb9d 100644
> --- i/t/perf/perf-lib.sh
> +++ w/t/perf/perf-lib.sh
> @@ -42,6 +42,7 @@ else
>  fi
>   
>  TEST_NO_CREATE_REPO=t
> +TEST_NO_MALLOC_=t

Why the trailing underscore?  Perhaps add "CHECK" before the equal sign?

>   . ../test-lib.sh
>   
> diff --git i/t/test-lib.sh w/t/test-lib.sh
> index b0c0c84..aad4606 100644
> --- i/t/test-lib.sh
> +++ w/t/test-lib.sh
> @@ -95,12 +95,24 @@ export EDITOR
>  
>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind
> -expr "$GIT_TEST_OPTS" : ".*\(--valgrind\)" >/dev/null || {
> - MALLOC_CHECK_=3
> - export MALLOC_CHECK_
> - MALLOC_PERTURB_="$( expr \( $$ % 255 \) + 1)"
> - export MALLOC_PERTURB_
> -}
> +if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
> +   test -n "TEST_NO_MALLOC_"

Without a $, you'll get nothing. ;-)

('test -n "string"' is always true, unlike 'test -n "$variable"'.)

> +then
> + setup_malloc_check () {
> + : nothing
> + }
> + teardown_malloc_check () {
> + : nothing
> + }
> +else
> + setup_malloc_check () {
> + MALLOC_CHECK_=3 MALLOC_PERTURB_=165
> + export MALLOC_CHECK_ MALLOC_PERTURB_
> + }
> + teardown_malloc_check () {
> + unset MALLOC_CHECK_ MALLOC_PERTURB_
> + }

Would it make sense to restore the previous values?  Hrm, can't think of a use 
case.

> +fi
>   
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -311,7 +323,9 @@ test_run_ () {
>  
>   if test -z "$immediate" || test $eval_ret = 0 || test -n 
> "$expecting_failure"
>   then
> + setup_malloc_check
>   test_eval_ "$test_cleanup"
> + teardown_malloc_check
>   fi
>   if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"
>   then

-- >8 --
Subject: [PATCH] MALLOC_CHECK: enable it, unless disabled explicitly

The malloc checks in tests are currently disabled.  Actually evaluate
the variable for turning them off and enable them if it's unset.

Also use this opportunity to give it the more descriptive and
consistent name TEST_NO_MALLOC_CHECK.

Signed-off-by: Rene Scharfe 
---
 t/perf/perf-lib.sh | 2 +-
 t/test-lib.sh  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 1d0bb9d..a816fbc 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -42,7 +42,7 @@ else
 fi
 
 TEST_NO_CREATE_REPO=t
-TEST_NO_MALLOC_=t
+TEST_NO_MALLOC_CHECK=t
 
 . ../test-lib.sh
 
diff --git a/t/test-lib.sh b/t/test-lib.sh
index bff3d75..617d831 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -105,7 +105,7 @@ export EDITOR
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-   test -n "TEST_NO_MALLOC_"
+   test -n "$TEST_NO_MALLOC_CHECK"
 then
setup_malloc_check () {
: nothing
-- 
1.7.12



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths.

2012-09-26 Thread Jonathan Nieder
Hi,

Michael G Schwern wrote:
> On 2012.7.30 12:51 PM, Eric Wong wrote:
>> Michael G Schwern wrote:

>>> _collapse_dotdot() works better than the existing regex did.
>>
>> I don't dispute it's better, but it's worth explaining in the commit
>> message to reviewers why something is "better".
>
> Yeah.  I figured the tests covered that.

Now I'm tripping up on the same thing.  Eric, did you ever find out
what the motivation for this patch was?  Is SVN 1.7 more persnickety
about runs of multiple slashes in a row or something, or is it more
of an aesthetic thing?

Puzzled,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 8/8] t1504: stop resolving symlinks in GIT_CEILING_DIRECTORIES

2012-09-26 Thread Michael Haggerty
This test used to explicitly resolve symlinks in the paths derived
from TRASH_DIRECTORY that were written to GIT_CEILING_DIRECTORIES,
because the code handling GIT_CEILING_DIRECTORIES was confused by
symlinks.  This is no longer necessary.

Signed-off-by: Michael Haggerty 
---
 t/t1504-ceiling-dirs.sh | 67 -
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index cce87a5..a4a5202 100755
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -14,8 +14,7 @@ test_fail() {
'
 }
 
-TRASH_ROOT="$PWD"
-ROOT_PARENT=$(dirname "$TRASH_ROOT")
+ROOT_PARENT=$(dirname "$TRASH_DIRECTORY")
 
 
 unset GIT_CEILING_DIRECTORIES
@@ -32,16 +31,16 @@ test_prefix ceil_at_parent ""
 GIT_CEILING_DIRECTORIES="$ROOT_PARENT/"
 test_prefix ceil_at_parent_slash ""
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY"
 test_prefix ceil_at_trash ""
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/"
 test_prefix ceil_at_trash_slash ""
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub"
 test_prefix ceil_at_sub ""
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/"
 test_prefix ceil_at_sub_slash ""
 
 
@@ -56,55 +55,55 @@ export GIT_CEILING_DIRECTORIES
 GIT_CEILING_DIRECTORIES=""
 test_prefix subdir_ceil_empty "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY"
 test_fail subdir_ceil_at_trash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/"
 test_fail subdir_ceil_at_trash_slash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub"
 test_fail subdir_ceil_at_sub
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/"
 test_fail subdir_ceil_at_sub_slash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/dir"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/dir"
 test_prefix subdir_ceil_at_subdir "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/dir/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/dir/"
 test_prefix subdir_ceil_at_subdir_slash "sub/dir/"
 
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/su"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/su"
 test_prefix subdir_ceil_at_su "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/su/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/su/"
 test_prefix subdir_ceil_at_su_slash "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/di"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/di"
 test_prefix subdir_ceil_at_sub_di "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/di"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub/di"
 test_prefix subdir_ceil_at_sub_di_slash "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/subdi"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/subdi"
 test_prefix subdir_ceil_at_subdi "sub/dir/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/subdi"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/subdi"
 test_prefix subdir_ceil_at_subdi_slash "sub/dir/"
 
 
-GIT_CEILING_DIRECTORIES="/foo:$TRASH_ROOT/sub"
+GIT_CEILING_DIRECTORIES="/foo:$TRASH_DIRECTORY/sub"
 test_fail second_of_two
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub:/bar"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub:/bar"
 test_fail first_of_two
 
-GIT_CEILING_DIRECTORIES="/foo:$TRASH_ROOT/sub:/bar"
+GIT_CEILING_DIRECTORIES="/foo:$TRASH_DIRECTORY/sub:/bar"
 test_fail second_of_three
 
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/sub"
 GIT_DIR=../../.git
 export GIT_DIR
 test_prefix git_dir_specified ""
@@ -123,41 +122,41 @@ export GIT_CEILING_DIRECTORIES
 GIT_CEILING_DIRECTORIES=""
 test_prefix sd_ceil_empty "s/d/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY"
 test_fail sd_ceil_at_trash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/"
 test_fail sd_ceil_at_trash_slash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/s"
 test_fail sd_ceil_at_s
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/s/"
 test_fail sd_ceil_at_s_slash
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s/d"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/s/d"
 test_prefix sd_ceil_at_sd "s/d/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s/d/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/s/d/"
 test_prefix sd_ceil_at_sd_slash "s/d/"
 
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/su"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/su"
 test_prefix sd_ceil_at_su "s/d/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/su/"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/su/"
 test_prefix sd_ceil_at_su_slash "s/d/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s/di"
+GIT_CEILING_DIRECTORIES="$TRASH_DIRECTORY/s/di"
 test_prefix sd_ceil_at_s_di "s/d/"
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/s/di"
+GIT_CEIL

[PATCH 7/8] longest_ancestor_length(): resolve symlinks before comparing paths

2012-09-26 Thread Michael Haggerty
longest_ancestor_length() relies on a textual comparison of directory
parts to find the part of path that overlaps with one of the paths in
prefix_list.  But this doesn't work if any of the prefixes involves a
symbolic link, because the directories will look different even though
they might logically refer to the same directory.  So canonicalize the
paths listed in prefix_list using real_path_if_valid() before trying
to find matches.

path is already in canonical form, so doesn't need to be canonicalized
again.

This fixes some problems with using GIT_CEILING_DIRECTORIES that
contains paths involving symlinks, including t4035 if run with --root
set to a path involving symlinks.

Remove a number of tests of longest_ancestor_length().  It is awkward
to test longest_ancestor_length() now, because its new path
normalization behavior depends on the contents of the whole
filesystem.  But we can live without the tests, because
longest_ancestor_length() is now built of reusable components that are
themselves tested separately: string_list_split(),
string_list_longest_prefix(), and real_path_if_valid().

Signed-off-by: Michael Haggerty 
---
 path.c| 17 --
 t/t0060-path-utils.sh | 64 ---
 2 files changed, 10 insertions(+), 71 deletions(-)

diff --git a/path.c b/path.c
index 5cace83..981bb06 100644
--- a/path.c
+++ b/path.c
@@ -570,22 +570,25 @@ int normalize_path_copy(char *dst, const char *src)
 
 static int normalize_path_callback(struct string_list_item *item, void 
*cb_data)
 {
-   char buf[PATH_MAX+2];
+   char *buf;
const char *ceil = item->string;
-   int len = strlen(ceil);
+   const char *realpath;
+   int len;
 
-   if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+   if (!*ceil || !is_absolute_path(ceil))
return 0;
-   memcpy(buf, ceil, len+1);
-   if (normalize_path_copy(buf, buf) < 0)
+   realpath = real_path_if_valid(ceil);
+   if (!realpath)
return 0;
-   len = strlen(buf);
+   len = strlen(realpath);
+   buf = xmalloc(len + 2); /* Leave space for possible trailing slash */
+   strcpy(buf, realpath);
if (len == 0 || buf[len-1] != '/') {
buf[len++] = '/';
buf[len++] = '\0';
}
free(item->string);
-   item->string = xstrdup(buf);
+   item->string = buf;
return 1;
 }
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ef2345..c97bbf2 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -12,28 +12,6 @@ norm_path() {
"test \"\$(test-path-utils normalize_path_copy '$1')\" = '$2'"
 }
 
-# On Windows, we are using MSYS's bash, which mangles the paths.
-# Absolute paths are anchored at the MSYS installation directory,
-# which means that the path / accounts for this many characters:
-rootoff=$(test-path-utils normalize_path_copy / | wc -c)
-# Account for the trailing LF:
-if test $rootoff = 2; then
-   rootoff=# we are on Unix
-else
-   rootoff=$(($rootoff-1))
-fi
-
-ancestor() {
-   # We do some math with the expected ancestor length.
-   expected=$3
-   if test -n "$rootoff" && test "x$expected" != x-1; then
-   expected=$(($expected+$rootoff))
-   fi
-   test_expect_success "longest ancestor: $1 $2 => $expected" \
-   "actual=\$(test-path-utils longest_ancestor_length '$1' '$2') &&
-test \"\$actual\" = '$expected'"
-}
-
 # Absolute path tests must be skipped on Windows because due to path mangling
 # the test program never sees a POSIX-style absolute path
 case $(uname -s) in
@@ -93,48 +71,6 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX
 norm_path /d1/.../d2 /d1/.../d2 POSIX
 norm_path /d1/..././../d2 /d1/d2 POSIX
 
-ancestor / "" -1
-ancestor / / -1
-ancestor /foo "" -1
-ancestor /foo : -1
-ancestor /foo ::. -1
-ancestor /foo ::..:: -1
-ancestor /foo / 0
-ancestor /foo /fo -1
-ancestor /foo /foo -1
-ancestor /foo /foo/ -1
-ancestor /foo /bar -1
-ancestor /foo /bar/ -1
-ancestor /foo /foo/bar -1
-ancestor /foo /foo:/bar/ -1
-ancestor /foo /foo/:/bar/ -1
-ancestor /foo /foo::/bar/ -1
-ancestor /foo /:/foo:/bar/ 0
-ancestor /foo /foo:/:/bar/ 0
-ancestor /foo /:/bar/:/foo 0
-ancestor /foo/bar "" -1
-ancestor /foo/bar / 0
-ancestor /foo/bar /fo -1
-ancestor /foo/bar foo -1
-ancestor /foo/bar /foo 4
-ancestor /foo/bar /foo/ 4
-ancestor /foo/bar /foo/ba -1
-ancestor /foo/bar /:/fo 0
-ancestor /foo/bar /foo:/foo/ba 4
-ancestor /foo/bar /bar -1
-ancestor /foo/bar /bar/ -1
-ancestor /foo/bar /fo: -1
-ancestor /foo/bar :/fo -1
-ancestor /foo/bar /foo:/bar/ 4
-ancestor /foo/bar /:/foo:/bar/ 4
-ancestor /foo/bar /foo:/:/bar/ 4
-ancestor /foo/bar /:/bar/:/fo 0
-ancestor /foo/bar /:/bar/ 0
-ancestor /foo/bar .:/foo/. 4
-ancestor /foo/bar .:/foo/.:.: 4
-ancestor /foo/bar /foo/./:.:/bar 4
-ancestor /foo/bar .:/bar -1
-
 test_expect_success 'strip_path_suffix' '
test c

[PATCH 5/8] longest_ancestor_length(): always add a slash to the end of prefixes

2012-09-26 Thread Michael Haggerty
Previously we stripped off any slashes that were present.  Instead,
add a slash if it is missing.  This removes the need for an extra
check that path has a slash following the prefix and makes the
handling of the root directory more natural, making the way clear to
use string_list_longest_prefix().

Signed-off-by: Michael Haggerty 
---
 path.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/path.c b/path.c
index 6da7029..90ef53d 100644
--- a/path.c
+++ b/path.c
@@ -570,7 +570,7 @@ int normalize_path_copy(char *dst, const char *src)
 
 static int normalize_path_callback(struct string_list_item *item, void 
*cb_data)
 {
-   char buf[PATH_MAX+1];
+   char buf[PATH_MAX+2];
const char *ceil = item->string;
int len = strlen(ceil);
 
@@ -580,8 +580,10 @@ static int normalize_path_callback(struct string_list_item 
*item, void *cb_data)
if (normalize_path_copy(buf, buf) < 0)
return 0;
len = strlen(buf);
-   if (len > 0 && buf[len-1] == '/')
-   buf[--len] = '\0';
+   if (len == 0 || buf[len-1] != '/') {
+   buf[len++] = '/';
+   buf[len++] = '\0';
+   }
free(item->string);
item->string = xstrdup(buf);
return 1;
@@ -616,9 +618,8 @@ int longest_ancestor_length(const char *path, const char 
*prefix_list)
int len = strlen(ceil);
 
if (!strncmp(path, ceil, len) &&
-   path[len] == '/' &&
-   len > max_len) {
-   max_len = len;
+   len - 1 > max_len) {
+   max_len = len - 1;
}
}
 
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/8] longest_ancestor_length(): use string_list_longest_prefix()

2012-09-26 Thread Michael Haggerty
Use string_list_longest_prefix() in the implementation of
longest_ancestor_length(), instead of an equivalent loop.

Signed-off-by: Michael Haggerty 
---
 path.c | 15 ---
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/path.c b/path.c
index 90ef53d..5cace83 100644
--- a/path.c
+++ b/path.c
@@ -605,23 +605,16 @@ static int normalize_path_callback(struct 
string_list_item *item, void *cb_data)
 int longest_ancestor_length(const char *path, const char *prefix_list)
 {
struct string_list prefixes = STRING_LIST_INIT_DUP;
-   int i, max_len = -1;
+   int max_len;
+   const char *longest_prefix;
 
if (prefix_list == NULL || !strcmp(path, "/"))
return -1;
 
string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
filter_string_list(&prefixes, 0, normalize_path_callback, NULL);
-
-   for (i = 0; i < prefixes.nr; i++) {
-   const char *ceil = prefixes.items[i].string;
-   int len = strlen(ceil);
-
-   if (!strncmp(path, ceil, len) &&
-   len - 1 > max_len) {
-   max_len = len - 1;
-   }
-   }
+   longest_prefix = string_list_longest_prefix(&prefixes, path);
+   max_len = longest_prefix ? strlen(longest_prefix) - 1 : -1;
 
string_list_clear(&prefixes, 0);
return max_len;
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/8] longest_ancestor_length(): explicitly filter list before loop

2012-09-26 Thread Michael Haggerty
Separate the step of filtering and normalizing elements of the
prefixes list from the iteration that looks for the longest prefix.
This will help keep the function testable after we not only normalize
the paths, but also convert them into real paths.

Signed-off-by: Michael Haggerty 
---
 path.c | 32 +---
 1 file changed, 21 insertions(+), 11 deletions(-)

diff --git a/path.c b/path.c
index 969bc17..6da7029 100644
--- a/path.c
+++ b/path.c
@@ -568,6 +568,25 @@ int normalize_path_copy(char *dst, const char *src)
return 0;
 }
 
+static int normalize_path_callback(struct string_list_item *item, void 
*cb_data)
+{
+   char buf[PATH_MAX+1];
+   const char *ceil = item->string;
+   int len = strlen(ceil);
+
+   if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+   return 0;
+   memcpy(buf, ceil, len+1);
+   if (normalize_path_copy(buf, buf) < 0)
+   return 0;
+   len = strlen(buf);
+   if (len > 0 && buf[len-1] == '/')
+   buf[--len] = '\0';
+   free(item->string);
+   item->string = xstrdup(buf);
+   return 1;
+}
+
 /*
  * path = Canonical absolute path
  * prefix_list = Colon-separated list of absolute paths
@@ -584,28 +603,19 @@ int normalize_path_copy(char *dst, const char *src)
 int longest_ancestor_length(const char *path, const char *prefix_list)
 {
struct string_list prefixes = STRING_LIST_INIT_DUP;
-   char buf[PATH_MAX+1];
int i, max_len = -1;
 
if (prefix_list == NULL || !strcmp(path, "/"))
return -1;
 
string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
+   filter_string_list(&prefixes, 0, normalize_path_callback, NULL);
 
for (i = 0; i < prefixes.nr; i++) {
const char *ceil = prefixes.items[i].string;
int len = strlen(ceil);
 
-   if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
-   continue;
-   memcpy(buf, ceil, len+1);
-   if (normalize_path_copy(buf, buf) < 0)
-   continue;
-   len = strlen(buf);
-   if (len > 0 && buf[len-1] == '/')
-   buf[--len] = '\0';
-
-   if (!strncmp(path, buf, len) &&
+   if (!strncmp(path, ceil, len) &&
path[len] == '/' &&
len > max_len) {
max_len = len;
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/8] longest_ancestor_length(): use string_list_split()

2012-09-26 Thread Michael Haggerty

Signed-off-by: Michael Haggerty 
---
 path.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/path.c b/path.c
index cbbdf7d..969bc17 100644
--- a/path.c
+++ b/path.c
@@ -12,6 +12,7 @@
  */
 #include "cache.h"
 #include "strbuf.h"
+#include "string-list.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -582,19 +583,22 @@ int normalize_path_copy(char *dst, const char *src)
  */
 int longest_ancestor_length(const char *path, const char *prefix_list)
 {
+   struct string_list prefixes = STRING_LIST_INIT_DUP;
char buf[PATH_MAX+1];
-   const char *ceil, *colon;
-   int len, max_len = -1;
+   int i, max_len = -1;
 
if (prefix_list == NULL || !strcmp(path, "/"))
return -1;
 
-   for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
-   for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
-   len = colon - ceil;
+   string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
+
+   for (i = 0; i < prefixes.nr; i++) {
+   const char *ceil = prefixes.items[i].string;
+   int len = strlen(ceil);
+
if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
continue;
-   strlcpy(buf, ceil, len+1);
+   memcpy(buf, ceil, len+1);
if (normalize_path_copy(buf, buf) < 0)
continue;
len = strlen(buf);
@@ -608,6 +612,7 @@ int longest_ancestor_length(const char *path, const char 
*prefix_list)
}
}
 
+   string_list_clear(&prefixes, 0);
return max_len;
 }
 
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/8] Introduce new function real_path_if_valid()

2012-09-26 Thread Michael Haggerty
The function is like real_path(), except that it returns NULL on error
instead of dying.

Signed-off-by: Michael Haggerty 
---
 abspath.c | 5 +
 cache.h   | 1 +
 2 files changed, 6 insertions(+)

diff --git a/abspath.c b/abspath.c
index a7ab8e9..5748b91 100644
--- a/abspath.c
+++ b/abspath.c
@@ -146,6 +146,11 @@ const char *real_path(const char *path)
return real_path_internal(path, 1);
 }
 
+const char *real_path_if_valid(const char *path)
+{
+   return real_path_internal(path, 0);
+}
+
 static const char *get_pwd_cwd(void)
 {
static char cwd[PATH_MAX + 1];
diff --git a/cache.h b/cache.h
index a58df84..b0d75bc 100644
--- a/cache.h
+++ b/cache.h
@@ -714,6 +714,7 @@ static inline int is_absolute_path(const char *path)
 }
 int is_directory(const char *);
 const char *real_path(const char *path);
+const char *real_path_if_valid(const char *path);
 const char *absolute_path(const char *path);
 const char *relative_path(const char *abs, const char *base);
 int normalize_path_copy(char *dst, const char *src);
-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/8] Introduce new static function real_path_internal()

2012-09-26 Thread Michael Haggerty
It accepts a new parameter, die_on_error.  If die_on_error is false,
it simply cleans up after itself and returns NULL rather than dying.

Signed-off-by: Michael Haggerty 
---
 abspath.c | 93 ---
 1 file changed, 72 insertions(+), 21 deletions(-)

diff --git a/abspath.c b/abspath.c
index 05f2d79..a7ab8e9 100644
--- a/abspath.c
+++ b/abspath.c
@@ -15,15 +15,26 @@ int is_directory(const char *path)
 #define MAXDEPTH 5
 
 /*
- * Use this to get the real path, i.e. resolve links. If you want an
- * absolute path but don't mind links, use absolute_path.
+ * Return the real path (i.e., absolute path, with symlinks resolved
+ * and extra slashes removed) equivalent to the specified path.  (If
+ * you want an absolute path but don't mind links, use
+ * absolute_path().)  The return value is a pointer to a static
+ * buffer.
+ *
+ * The input and all intermediate paths must be shorter than MAX_PATH.
+ * The directory part of path (i.e., everything up to the last
+ * dir_sep) must denote a valid, existing directory, but the last
+ * component need not exist.  If die_on_error is set, then die with an
+ * informative error message if there is a problem.  Otherwise, return
+ * NULL on errors (without generating any output).
  *
  * If path is our buffer, then return path, as it's already what the
  * user wants.
  */
-const char *real_path(const char *path)
+static const char *real_path_internal(const char *path, int die_on_error)
 {
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
+   char *retval = NULL;
char cwd[1024] = "";
int buf_index = 1;
 
@@ -35,11 +46,19 @@ const char *real_path(const char *path)
if (path == buf || path == next_buf)
return path;
 
-   if (!*path)
-   die("The empty string is not a valid path");
+   if (!*path) {
+   if (die_on_error)
+   die("The empty string is not a valid path");
+   else
+   goto error_out;
+   }
 
-   if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
-   die ("Too long path: %.*s", 60, path);
+   if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {
+   if (die_on_error)
+   die("Too long path: %.*s", 60, path);
+   else
+   goto error_out;
+   }
 
while (depth--) {
if (!is_directory(buf)) {
@@ -54,20 +73,36 @@ const char *real_path(const char *path)
}
 
if (*buf) {
-   if (!*cwd && !getcwd(cwd, sizeof(cwd)))
-   die_errno ("Could not get current working 
directory");
+   if (!*cwd && !getcwd(cwd, sizeof(cwd))) {
+   if (die_on_error)
+   die_errno("Could not get current 
working directory");
+   else
+   goto error_out;
+   }
 
-   if (chdir(buf))
-   die_errno ("Could not switch to '%s'", buf);
+   if (chdir(buf)) {
+   if (die_on_error)
+   die_errno("Could not switch to '%s'", 
buf);
+   else
+   goto error_out;
+   }
+   }
+   if (!getcwd(buf, PATH_MAX)) {
+   if (die_on_error)
+   die_errno("Could not get current working 
directory");
+   else
+   goto error_out;
}
-   if (!getcwd(buf, PATH_MAX))
-   die_errno ("Could not get current working directory");
 
if (last_elem) {
size_t len = strlen(buf);
-   if (len + strlen(last_elem) + 2 > PATH_MAX)
-   die ("Too long path name: '%s/%s'",
-   buf, last_elem);
+   if (len + strlen(last_elem) + 2 > PATH_MAX) {
+   if (die_on_error)
+   die("Too long path name: '%s/%s'",
+   buf, last_elem);
+   else
+   goto error_out;
+   }
if (len && !is_dir_sep(buf[len-1]))
buf[len++] = '/';
strcpy(buf + len, last_elem);
@@ -77,10 +112,18 @@ const char *real_path(const char *path)
 
if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {
ssize_t len = readlink(buf, next_buf, PATH_MAX);
-   if (len < 0)
-   d

[PATCH 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks

2012-09-26 Thread Michael Haggerty
This series fixes longest_ancestor_length() so that it works even if
prefix_list contains entries that involve symlinks.  The basic goal of
the series is to call real_path() on each of the entries so that a
textual comparison of the potential prefix to the front of path
correctly decides whether the path is located inside of the entry.
But along the way some other things had to be changed:

* real_path() die()s if the path passed to it is invalid, whereas it
  is allowed for GIT_CEILING_DIRECTORIES to contain invalid paths.  So
  create a new function real_path_if_valid() that returns NULL for
  invalid paths.

* Changing longest_ancestor_length() to call real_path_if_valid()
  would make the former very difficult to test (because the tests
  would depend on the contents of the whole filesystem).  Therefore,
  rewrite longest_ancestor_length() in terms of functions
  string_list_split(), string_list_longest_prefix(), and
  real_path_if_valid() which are tested individually.

The net results of these changes are that:

1. t1504 used to have to canonicalize TRASH_DIRECTORY to make itself
   work even if the --root directory contains symlinks.  This
   canonicalization is no longer necessary (and has been removed).

2. t4035, which used to fail if the --root directory contained
   symlinks, now works correctly in that situation.

After this change, all tests pass if the --root directory does *not*
contain symlinks, but t9903 still fails if the --root directory
contains symlinks.  I haven't analyzed the cause of t9903's failure,
but it does not appear to be related to the GIT_CEILING_DIRECTORIES
feature.

On the mailing list I suggested *purposely* inserting symlinks into
the "trash directory.*" paths to test symlink handling more
systematically.  This patch series does *NOT* make that change.

Michael Haggerty (8):
  Introduce new static function real_path_internal()
  Introduce new function real_path_if_valid()
  longest_ancestor_length(): use string_list_split()
  longest_ancestor_length(): explicitly filter list before loop
  longest_ancestor_length(): always add a slash to the end of prefixes
  longest_ancestor_length(): use string_list_longest_prefix()
  longest_ancestor_length(): resolve symlinks before comparing paths
  t1504: stop resolving symlinks in GIT_CEILING_DIRECTORIES

 abspath.c   | 98 ++---
 cache.h |  1 +
 path.c  | 54 ---
 t/t0060-path-utils.sh   | 64 
 t/t1504-ceiling-dirs.sh | 67 +
 5 files changed, 144 insertions(+), 140 deletions(-)

-- 
1.7.11.3

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] revision: add --reflog-message= to grep reflog messages

2012-09-26 Thread Junio C Hamano
Nguyen Thai Ngoc Duy  writes:

> On Wed, Sep 26, 2012 at 9:07 PM, Junio C Hamano  wrote:
>> Nguyễn Thái Ngọc Duy   writes:
>>
>>> Both "git log" and "git reflog show" recognize this option.
>>>
>>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>>> ---
>>
>> How well does it interact with --grep and/or --all-match?
>
> Good point. It currently works like and operator. But people might
> expect to combine them in different ways.

The current commit_match() runs grep_buffer() on commit->buffer.  It
probably makes sense to instead notice from opt that we are running
log with "-g", prepare a temporary strbuf and add in the reflog
message to the string in commit->buffer, and run grep_buffer() on
that temporary strbuf on it.

I personally think it is sufficient ot just reuse --grep on
concatenation of commit->buffer with "Reflog message: checkout:
moving from as/check-ignore to pu".

If you really want to go fancier, you could add --grep-reflog option
that behaves like the existing --author and --committer options to
add "header match" elements to the grep expression, splice a fake
"reflog " header to the string copied from commit->buffer, e.g.
prepare something like this in your temporary strbuf:

tree b4429f218782165faf101ccb0f4ba1cdd6d1d349
parent de5cd03876e546d6d264ab28a01daa978f3eae78
parent b378e5a25658e07e6d0c0f4db79e87cb21de5489
author Junio C Hamano  1348616180 -0700
committer Junio C Hamano  1348616180 -0700
reflog checkout: moving from as/check-ignore to pu

Merge branch 'jk/lua-hackery' into pu

* jk/lua-hackery:
  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

that way, you can take advantage of the existing logic used for the
author/committer match that matches only in the commit object
header.

Again, I personally doubt the fancier option is worth it, but the
starting point may look something like this.

 revision.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git c/revision.c w/revision.c
index ae12e11..b0f4d5b 100644
--- c/revision.c
+++ w/revision.c
@@ -2212,8 +2212,20 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
 {
if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
return 1;
-   return grep_buffer(&opt->grep_filter,
-  commit->buffer, strlen(commit->buffer));
+
+   if (opt->reflog_info) {
+   int retval;
+   struct strbuf buf = STRBUF_INIT;
+   strbuf_addf(&buf, "reflog %s\n", opt->reflog_info->message);
+   strbuf_addstr(&buf, commit->buffer);
+   retval = grep_buffer(&opt->grep_filter,
+buf.buf, buf.len);
+   strbuf_release(&buf);
+   return retval;
+   } else {
+   return grep_buffer(&opt->grep_filter,
+  commit->buffer, strlen(commit->buffer));
+   }
 }
 
 static inline int want_ancestry(struct rev_info *revs)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: bash completion with colour hints

2012-09-26 Thread Simon Oosthoek
On 26/09/12 17:24, Ramkumar Ramachandra wrote:
> Hi Simon,
> 
> Could you follow the guidelines in Documentation/SubmittingPatches, so
> that the patch can be considered for inclusion?

Hi Ram, thanks for your feedback.

I gather now that this file is part of the entire git tree ;-)

this is my first time to submit a patch to git and frankly I coded this
up very quickly and did only a small test.

I read the guide and now I have some questions:

- It suggests to use the oldest commit that contains the "bug" and can
support the fix. This would be the very first mention of __git_ps1
function I think commit d3d717a4ad0c8d7329e79f7d0313baec57c6b585
However, I guess that although I have been using something similar since
about 2009, I should at least base it on the relatively new
git-prompt.sh file, is this a correct interpretation of the guide?
(BTW, I wonder how this will affect ultimately the current master?)

- I read that git-prompt.sh is meant to support bash and zsh, I have
only tested it on bash. Should I attempt to test it on zsh or is there a
kind person with zsh as his/her shell to test it for me?

My instinct is to just apply my patch to the current master, but I'm
open to starting from a different base, but I'm too new to the tree to
know which one, any suggestions?



> 
>> --- git-orig-bak 2012-09-26 16:39:47.0 +0200
>> +++ git-bashcompletion   2012-09-26 16:50:57.0 +0200
>> @@ -59,6 +59,9 @@
>>  #   per-repository basis by setting the bash.showUpstream config
>>  #   variable.
>>  #
>> +#   If you would like an additional hint in colour in your prompt
>> +#   set GIT_PS1_SHOWCOLORHINT to a nonempty value. Currently
>> +#   the colours are hardcoded in the function...
> 
> Nit: I think it's spelt "color" everywhere else in git.
> 

I can adapt ;-)

>> +local c_red=' [31m'
>> +local c_yellow=' [33m'
>> +local c_lblue=' [1,34m'
>> +local c_green=' [32m'
>> +local c_purple=' [35m'
>> +local c_cyan=' [36m'
>> +local c_clear=' [0m'
>> +local printf_format="${1:- (%s)}"
>> +
>> +if [ -n "${GIT_PS1_SHOWCOLORHINT-}" ]; then
>> +if [ "$w" != "*" ]; then
>> +printf_format="$c_green$printf_format$c_clear"
>> +else
>> +printf_format="$c_red$printf_format$c_clear"
>> +fi
>> +if [ -n "$i" ]; then
>> +i="$c_yellow$i$c_clear"
>> +fi
>> +if [ -n "$s" ]; then
>> +s="$c_lblue$i$c_clear"
>> +fi
>> +if [ -n "$u" ]; then
>> +u="$c_purple$i$c_clear"
>> +fi
>> +fi
>> +
>>  local f="$w$i$s$u"
>> -printf "${1:- (%s)}" "$c${b##refs/heads/}${f:+ $f}$r$p"
>> +echo $(printf "$printf_format" "$c${b##refs/heads/}${f:+ 
>> $f}$r$p")

I'm still in some doubt over this last line, including the color codes
is confusing me...

I'll go ahead and try some more polishing and whatever more comes as
suggestions. Is it ok to bother the list with intermediate stuff in this
thread?

Cheers

Simon
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git die message written to stdout?

2012-09-26 Thread Frans Klaver

On Wed, 26 Sep 2012 18:01:39 +0200, 乙酸鋰  wrote:


Is git die() messages written to stdout?
Should it be written to stderr?


As far as I know git die() routines write their output to stderr. Do you  
have an indication that that might not be the case?


Frans



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: A generalization of git blame

2012-09-26 Thread Junio C Hamano
xm...@cs.wisc.edu writes:

> Another question is that is it possible to include my tool as a git
> built-in tool in the future?

It largely depends on how the user would interact with your program,
which is totally unclear as we haven't seen any part of it.  I do
not think we have enough information to answer the question at this
point.

> I know that my tool is still not good for any release. But I would
> like to share my work with other people if other people are
> interested.

If it is a trivial script that largely depends on what we already
ship, I would not mind carrying it in contrib/.  If it is anything
substantial and substantially useful, however, I would suspect that
you are better off not be in in my tree, but rather want to be
independent.  Finishing it to be useful for your purpose, publishing
it somewhere people can take a look at and adding a pointer to
https://git.wiki.kernel.org/index.php/InterfacesFrontendsAndTools is
probably where you would want to start.

> And if it is possible, I think I will have a stronger motivation
> to make my tool more robust and useful.

I've seen from time to time people ask "I am thinking of doing this;
will a patch be accepted?  If so, I'll work on it." before showing
any work, and my response always has been:

 (1) We won't know how useful and interesting your contribution be
 for our audience, until we see it; and

 (2) If you truly believe in your work (find it useful, find writing
 it fun, etc.), that would be incentive enough for you to work
 on it, whether or not the result will land in my tree.  You
 should instead aim for something so brilliant that we would
 come to you begging for your permission to include it in our
 project.

I think it applies to your inquiry as well.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Configuring the location of ~/.gitconfig

2012-09-26 Thread David Aguilar
On Wed, Sep 26, 2012 at 7:14 AM, Ramkumar Ramachandra
 wrote:
> Hi,
>
> I'd like to configure the location of ~/.gitconfig through an
> environment variable.  My usecase is a simple enough: I have a
> repository with all my dotfiles, and I don't want to symlink
> ~/dotfiles/.gitconfig from $HOME after cloning it.  Does anyone else
> think the feature will be useful?
>
> A couple of similar examples:
> 1. The git templates directory is configurable via the
> GIT_TEMPLATE_DIR variable.
> 2. The location of ~/.zshrc, ~/.zlogin etc is configurable via the
> ZDOTDIR variable in ZSH.

There was some work recently to teach git about the XDG_CONFIG_HOME variable.

Would that help, or are the XDG variables too global for your usage?
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetool--lib: Allow custom commands to override built-ins

2012-09-26 Thread David Aguilar
On Wed, Sep 26, 2012 at 7:06 AM, Junio C Hamano  wrote:
> Ramkumar Ramachandra  writes:
>
>> Hi David,
>>
>> David Aguilar wrote:
>>>  diff_cmd () {
>>> -   merge_tool_cmd="$(get_merge_tool_cmd "$1")"
>>> -   if test -z "$merge_tool_cmd"
>>> -   then
>>> -   status=1
>>> -   break
>>> -   fi
>>> -   ( eval $merge_tool_cmd )
>>> -   status=$?
>>> +   status=1
>>> return $status
>>>  }
>>
>> Nit: Why not return 1, instead of setting $status and returning it?
>
> Perhaps because the caller "run_merge_tool" pays attention to
> $status that is a global variable?
>
> Have you traced the call chain?

Exactly.  I would like to eliminate globals whenever possible,
but this particular topic involved refactoring which aimed to keep
existing behavior w.r.t these variables unchanged.
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4] Teach rm to remove submodules unless they contain a git directory

2012-09-26 Thread Jens Lehmann
Currently using "git rm" on a submodule - populated or not - fails with
this error:
fatal: git rm: '': Is a directory
This made sense in the past as there was no way to remove a submodule
without possibly removing unpushed parts of the submodule's history
contained in its .git directory too, so erroring out here protected the
user from possible loss of data.

But submodules cloned with a recent git version do not contain the .git
directory anymore, they use a gitfile to point to their git directory
which is safely stored inside the superproject's .git directory. The work
tree of these submodules can safely be removed without loosing history, so
let's teach git to do so.

Using rm on an unpopulated submodule now removes the empty directory from
the work tree and the gitlink from the index. If the submodule's directory
is missing from the work tree, it will still be removed from the index.

Using rm on a populated submodule using a gitfile will apply the usual
checks for work tree modification adapted to submodules (unless forced).
For a submodule that means that the HEAD is the same as recorded in the
index, no tracked files are modified and no untracked files that aren't
ignored are present in the submodules work tree (ignored files are deemed
expendable and won't stop a submodule's work tree from being removed).
That logic has to be applied in all nested submodules too.

Using rm on a submodule which has its .git directory inside the work trees
top level directory will just error out like it did before to protect the
repository, even when forced. In the future git could either provide a
message informing the user to convert the submodule to use a gitfile or
even attempt to do the conversion itself, but that is not part of this
change.

Signed-off-by: Jens Lehmann 
---


Changes since v3:
- Added get_ours_cache_pos() helper to only check stage 2 of a conflict
- Added tests for modified submodules in the conflict case


 Documentation/git-rm.txt |  15 +++
 builtin/rm.c | 125 ++---
 submodule.c  |  80 +++
 submodule.h  |   2 +
 t/t3600-rm.sh| 343 +++
 5 files changed, 550 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 5d31860..882cb11 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -107,6 +107,21 @@ as well as modifications of existing paths.
 Typically you would first remove all tracked files from the working
 tree using this command:

+Submodules
+~~
+Only submodules using a gitfile (which means they were cloned
+with a git version 1.7.8 or newer) will be removed from the work
+tree, as their repository lives inside the .git directory of the
+superproject. If a submodule (or one of those nested inside it)
+still uses a .git directory, `git rm` will fail - no matter if forced
+or not - to protect the submodule's history.
+
+A submodule is considered up-to-date when the HEAD is the same as
+recorded in the index, no tracked files are modified and no untracked
+files that aren't ignored are present in the submodules work tree.
+Ignored files are deemed expendable and won't stop a submodule's work
+tree from being removed.
+
 
 git ls-files -z | xargs -0 rm -f
 
diff --git a/builtin/rm.c b/builtin/rm.c
index b384c4c..99dcbe6 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -9,6 +9,7 @@
 #include "cache-tree.h"
 #include "tree-walk.h"
 #include "parse-options.h"
+#include "submodule.h"

 static const char * const builtin_rm_usage[] = {
N_("git rm [options] [--] ..."),
@@ -17,9 +18,58 @@ static const char * const builtin_rm_usage[] = {

 static struct {
int nr, alloc;
-   const char **name;
+   struct {
+   const char *name;
+   char is_submodule;
+   } *entry;
 } list;

+static int get_ours_cache_pos(const char *path, int pos)
+{
+   int i = -pos - 1;
+
+   while ((i < active_nr) && !strcmp(active_cache[i]->name, path)) {
+   if (ce_stage(active_cache[i]) == 2)
+   return i;
+   i++;
+   }
+   return -1;
+}
+
+static int check_submodules_use_gitfiles(void)
+{
+   int i;
+   int errs = 0;
+
+   for (i = 0; i < list.nr; i++) {
+   const char *name = list.entry[i].name;
+   int pos;
+   struct cache_entry *ce;
+   struct stat st;
+
+   pos = cache_name_pos(name, strlen(name));
+   if (pos < 0) {
+   pos = get_ours_cache_pos(name, pos);
+   if (pos < 0)
+   continue;
+   }
+   ce = active_cache[pos];
+
+   if (!S_ISGITLINK(ce->ce_mode) ||
+   (lstat(ce->name, &st) < 0) ||
+   is_empty_dir(name))
+   continue;
+
+   if (!sub

Discussion around a --bindtodev option for git-daemon

2012-09-26 Thread Ronan Bignaux
I wrote this little patch to add a restrict option to bind only to a
specific network interface.

I'd not deal with --inetd since there are some bugs in xinetd with ipv6
( and no more maintener ) , systemd/upstart are also Linux centric and
subject to controversy...

"listen" option was not more useful in my case because it need ip as
parameter (you need fix ip or custom script for example).

It's not ready for inclusion but ready for discussion.

PROS :
* Do the job

CONS :
* Linux only
* root only

What do you think about such option/implementation ?
-- 
Ronan Bignaux
Entrepreneur indépendant
Consultant informatique

ScourGE SARL
136 rue Branville
14000 CAEN
06.47.75.44.81

>From bfebe7fc838f83065fea04cf27613fe89e962a3a Mon Sep 17 00:00:00 2001
From: Bignaux Ronan 
Date: Thu, 20 Sep 2012 15:09:31 +0200
Subject: [PATCH] add option to bind to a specific interface


Signed-off-by: Bignaux Ronan 
---
 daemon.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/daemon.c b/daemon.c
index 4602b46..755fbd3 100644
--- a/daemon.c
+++ b/daemon.c
@@ -5,6 +5,10 @@
 #include "strbuf.h"
 #include "string-list.h"
 
+#ifdef SO_BINDTODEVICE
+#include 
+#endif
+
 #ifndef HOST_NAME_MAX
 #define HOST_NAME_MAX 256
 #endif
@@ -31,7 +35,7 @@ static const char daemon_usage[] =
 "   [--reuseaddr] [--pid-file=]\n"
 "   [--(enable|disable|allow-override|forbid-override)=]\n"
 "   [--access-hook=]\n"
-"   [--inetd | [--listen=] [--port=]\n"
+"   [--inetd | [--listen=] [--port=] [--bindtodev=]\n"
 "  [--detach] [--user= [--group=]]\n"
 "   [...]";
 
@@ -64,6 +68,7 @@ static char *hostname;
 static char *canon_hostname;
 static char *ip_address;
 static char *tcp_port;
+static struct ifreq ifr;
 
 static void logreport(int priority, const char *err, va_list params)
 {
@@ -875,6 +880,15 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 			continue;
 		}
 
+		if (ifr.ifr_name) {
+			if (setsockopt(sockfd, SOL_SOCKET, SO_BINDTODEVICE, (void *) &ifr,
+	sizeof(ifr)) < 0) {
+logerror("Could not set SO_BINDTODEVICE: %s", strerror(errno));
+close(sockfd);
+continue;
+			}
+		}
+
 #ifdef IPV6_V6ONLY
 		if (ai->ai_family == AF_INET6) {
 			int on = 1;
@@ -1194,6 +1208,11 @@ int main(int argc, char **argv)
 continue;
 			}
 		}
+		if (!prefixcmp(arg, "--bindtodev=")) {
+			memset(&ifr, 0, sizeof(ifr));
+			strncpy (ifr.ifr_name, arg + 12 ,IFNAMSIZ);
+			continue;
+		}
 		if (!strcmp(arg, "--serve")) {
 			serve_mode = 1;
 			continue;
-- 
1.7.12

<>

Re: [RFC PATCH] add t3420-rebase-topology

2012-09-26 Thread Martin von Zweigbergk
[+Chris Webb regarding "git rebase --root"]

First of all, thanks for a meticulous review!

On Tue, Sep 18, 2012 at 12:53 AM, Johannes Sixt  wrote:
> Am 9/18/2012 8:31, schrieb Martin von Zweigbergk:
>
> Since here and in the following tests the test cases and test descriptions
> vary in the same way, wouldn't it make sense to factor the description out
> as well?

Definitely. I just couldn't think of a good way of doing it, so thanks
for great and concrete suggestions!

> (Watch your quoting, though.)

Switched to putting the test body in double quotes as you suggested in
your examples and used single quotes for strings within the test body.

>> +run () {
>> +echo '
>> + reset &&
>> + git rebase '"$@"' --keep-empty p h &&
>> + test_range p.. "f g h"
>> +'
>> +}
>> +test_expect_success 'rebase --keep-empty keeps empty even if already in 
>> upstream' "$(run)"
>> +test_expect_failure 'rebase -m --keep-empty keeps empty even if already in 
>> upstream' "$(run -m)"
>> +test_expect_failure 'rebase -i --keep-empty keeps empty even if already in 
>> upstream' "$(run -i)"
>> +test_expect_failure 'rebase -p --keep-empty keeps empty even if already in 
>> upstream' "$(run -p)"
>
> "is in upstream" is decided by the patch text. If an empty commit is
> already in upstream, this adds another one with the same or a different
> commit message and authorship information. Dubious, but since it is
> opt-in, it should be OK.

Yes, it is a little dubious. See
http://thread.gmane.org/gmane.comp.version-control.git/203097/focus=203159
and Junio's answer, which I think makes sense.

>> +run () {
>> +echo '
>> + reset &&
>> + git rebase '"$@"' j w &&
>> + test_range j.. "E n H" || test_range j.. "n H E"
>> +'
>
> Chaining tests with || is dangerous: you do not know whether the first
> failed because the condition is not satisfied or because of some other
> failure.

Good point. Thanks.

> Why is this needed in the first place? Shouldn't the history be
> deterministic, provided that the commit timestamps are all distinct?

It may be deterministic, but it's not specified, I think, so I didn't
want to depend on the order. Thinking more about it, though, I think
it's good to protect the current behavior from patches that change the
order of the parents. Although it may not be incorrect to change the
order, it would at least protect against accidental changes.

It turns out that "rebase -i" goes through the commits in
--topo-order, while the others use default order, I think. Which
flavor should pass the test case and which should fail (and be fixed)?
I would personally prefer to say that "rebase -i" is correct in using
--topo-order and that the others should be fixed. Again, it's not
specified, but I would hate to have them behave differently.

>> +run () {
>> +echo '
>> + reset &&
>> + git rebase '"$@"' --root c &&
>> + ! same_revision HEAD c &&
>> + test_range c "a b c"
>> +'
>> +}
>> +test_expect_success 'rebase --root is not a no-op' "$(run)"
>> +test_expect_success 'rebase -m --root is not a no-op' "$(run -m)"
>> +test_expect_success 'rebase -i --root is not a no-op' "$(run -i)"
>> +test_expect_success 'rebase -p --root is not a no-op' "$(run -p)"
>
> Why? Is it more like "--root implies --force"?

It doesn't currently exactly imply --force, but the effect is the
same. Also see my reply to Junio's email in this thread.

Maybe Chris has some thoughts on this?

>> +run () {
>> +echo '
>> + reset &&
>> + git rebase '"$@"' --root --onto e y &&
>> + test_range e.. "x y"
>> +'
>> +}
>> +test_expect_success 'rebase --root --onto' "$(run)"
>> +test_expect_failure 'rebase -m --root --onto' "$(run -m)"
>> +test_expect_success 'rebase -i --root --onto' "$(run -i)"
>> +test_expect_success 'rebase -p --root --onto' "$(run -p)"
>
> Where does this rebase start? Ah, --root stands in for the "upstream"
> argument, hence, y is the tip to rebase. Right? Then it makes sense.

Thanks for pointing that out. I changed the order to "git rebase
--onto e --root y". I hope that makes it clearer.

>> +test_expect_success 'rebase -p re-creates merge from upstream' '
>> + reset &&
>> + git rebase -p k w &&
>> + same_revision HEAD^ H &&
>> + same_revision HEAD^2 k
>> +'
>
> IMO, this tests the wrong thing. You have this history:
>
>  ---j---E---k
>  \   \
>   n---H---w
>
> where E is the second parent of w. What does it mean to rebase w onto k?
> IMO, it is a meaningless operation, and the outcome is irrelevant.
>
> It would make sense to test that this history results after the upstream
> at H moved forward:
>
>  ---j---E---k
>  \   \
>   n---H   \
>\   \
> z---w'
>
> That is, w began a topic by mergeing the sidebranch E; then upstream
> advanced to z, and now you rebase the topic to the new upstream.

Fair enough. Changed accordingly.

>> +test_expect_success 'rebase -p re-creates internal merge' '
>> + reset &&
>> + git reba

git die message written to stdout?

2012-09-26 Thread 乙酸鋰
Is git die() messages written to stdout?
Should it be written to stderr?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: A generalization of git blame

2012-09-26 Thread xmeng

> "Philip Oakley"  writes:
>
>>> To get ground truth of authorship for each line, I start with
>>> git-blame.
>>> But later I find this is not sufficient because the last commit may
>>> only
>>> add comments or may only change a small part of the line, so that I
>>> shouldn't attribute the line of code to the last author.
>>
>> I would suggest there is:
>> - White space adjustment
>> - Comment or documentation (assumes you can parse the 'code' to decide
>> that it isn't executable code)
>> - word changes within expressions
>> - complete replacement of line (whole statement?)
>
> You are being generous by listing easier cases ;-) I'd add a couple
> more that are more problematic if your approach does not consider
> semantics.
>
>  - A function gained a new parameter, to which pretty much everbody
>passes the same default value.
>
>   -void fn(int a, int b, int c)
>   +void fn(int a, int b, int c, int d)
>{
>   +   if (d) {
>   +   ...
>   +   return;
>   +   }
>   ...
>}
>
>  void frotz(void)
>{
>   ...
> - fn(a, b, c);
> + fn(a, b, c, 0);
>   ...
> - fn(a, b, d);
> + fn(a, b, d, 1);
>   ...
>
>The same commit that changed the above call site must have
>changed the definition of function "fn" and defined what the new
>fourth parameter means.  It is likely that, when the default
>value most everybody passes (perhaps "0") is given, "fn" does
>what it used to do, and a different value may trigger a new
>behaviour of "fn".  It could be argued that the former call
>should not be blamed for this commit, while the latter callsite
>should.
>
>  - A variable was renamed, and the meaning of a line suddenly
>changed, even though the text of that line did not change at all.
>
>static int foo;
>  ...
> -int xyzzy(int foo)
>   +int xyzzy(int bar)
>{
>   ... some complex computation that
> ... involves foo and bar, resulting in
> ... updating of foo comes here ...
>   return foo * 2;
>}
>
>Whom to blame the behaviour of (i.e. returned value from) the
>function?  The "return foo * 2" never changed with this patch,
>but the patch _is_ responsible for changing the behaviour.
>
>As the OP is interested in tracking the origin of the _binary_,
>this case is even more interesting, as the generated machine code
>to compute the foo * 2 would likely to be very different before
>and after the patch.
>
>

Thanks for both your great suggestions. Current my approach doesn't
consider semantics yet and this should be an interesting to do.

Another question is that is it possible to include my tool as a git
built-in tool in the future? I know that my tool is still not good for any
release. But I would like to share my work with other people if other
people are interested. And if it is possible, I think I will have a
stronger motivation to make my tool more robust and useful.

Thanks


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


bash completion with colour hints

2012-09-26 Thread Simon Oosthoek

Hi Shawn

I only recently found the __git_ps1 function, but it wasn't directly 
able to replace my own contraption. I've modified the version I found 
after installing bash-completion in debian 6.


The patch is attached, it contains an escape character, so it is hard to 
include in plain text. I've gzipped it for convenience...


This is only a first step, I had a hard time figuring out what exactly 
the one-letter variables were doing (and still a bit unclear), so I'm 
sure this can be improved!


Anyway, the functionality of this patch is to show the output in green 
if the repo is up to date and red or other colours if it isn't.


Cheers

Simon


bash_git_completion_colour.patch.gz
Description: application/gzip


Re: [PATCH] revision: add --reflog-message= to grep reflog messages

2012-09-26 Thread Nguyen Thai Ngoc Duy
On Wed, Sep 26, 2012 at 9:07 PM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy   writes:
>
>> Both "git log" and "git reflog show" recognize this option.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>
> How well does it interact with --grep and/or --all-match?

Good point. It currently works like and operator. But people might
expect to combine them in different ways.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] revision: add --reflog-message= to grep reflog messages

2012-09-26 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> Both "git log" and "git reflog show" recognize this option.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

How well does it interact with --grep and/or --all-match?

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] mergetool--lib: Allow custom commands to override built-ins

2012-09-26 Thread Junio C Hamano
Ramkumar Ramachandra  writes:

> Hi David,
>
> David Aguilar wrote:
>>  diff_cmd () {
>> -   merge_tool_cmd="$(get_merge_tool_cmd "$1")"
>> -   if test -z "$merge_tool_cmd"
>> -   then
>> -   status=1
>> -   break
>> -   fi
>> -   ( eval $merge_tool_cmd )
>> -   status=$?
>> +   status=1
>> return $status
>>  }
>
> Nit: Why not return 1, instead of setting $status and returning it?

Perhaps because the caller "run_merge_tool" pays attention to
$status that is a global variable?

Have you traced the call chain?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 14/14] Add git-check-ignore sub-command

2012-09-26 Thread Junio C Hamano
Johannes Sixt  writes:

>> These days, we do not add random subcommands willy-nilly (I still
>> doubt if check-ignore needs to be a separate debugging command, or a
>> new mode of operation of ls-files or something), so the approach to
>> use a blacklist makes more sense.  "help -a" is designed to show
>> whatever the users throw in their ~/bin (assuming that is on $PATH)
>> under git-whatever name, and we _do_ want to complete "git wh"
>> to that custom command, so a whitelist-based solution is unwieldy to
>> construct.
>
> We already have 'git check-attr', but it is obviously not among the
> autocompleted commands, otherwise the above test would not have passed.
> IMO, 'git check-ignore' falls into the same category as 'git check-attr'
> with regard to completion.

Exactly.

Actually I think what happened was that the submitter didn't have
change to contrib/completion/ in earlier private versions, saw the
test fail and updated t9902 without thinking.  The patch posted has
addition to contrib/completion/ that blacklists check-ignore the
same way as check-attr, which made the change to t9902 unnecessary
but because the update was done without thinking, it wasn't even
realized that the test would have passed without the patch to it.

Reverting the part that touches t9902 should still pass, I would
think.

 t/t9902-completion.sh | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git c/t/t9902-completion.sh w/t/t9902-completion.sh
index cce51ac..92d7eb4 100755
--- c/t/t9902-completion.sh
+++ w/t/t9902-completion.sh
@@ -213,19 +213,19 @@ test_expect_success 'general options' '
 '
 
 test_expect_success 'general options plus command' '
-   test_completion "git --version checko" "checkout " &&
-   test_completion "git --paginate checko" "checkout " &&
-   test_completion "git --git-dir=foo checko" "checkout " &&
-   test_completion "git --bare checko" "checkout " &&
+   test_completion "git --version check" "checkout " &&
+   test_completion "git --paginate check" "checkout " &&
+   test_completion "git --git-dir=foo check" "checkout " &&
+   test_completion "git --bare check" "checkout " &&
test_completion "git --help des" "describe " &&
-   test_completion "git --exec-path=foo checko" "checkout " &&
-   test_completion "git --html-path checko" "checkout " &&
-   test_completion "git --no-pager checko" "checkout " &&
-   test_completion "git --work-tree=foo checko" "checkout " &&
-   test_completion "git --namespace=foo checko" "checkout " &&
-   test_completion "git --paginate checko" "checkout " &&
-   test_completion "git --info-path checko" "checkout " &&
-   test_completion "git --no-replace-objects checko" "checkout "
+   test_completion "git --exec-path=foo check" "checkout " &&
+   test_completion "git --html-path check" "checkout " &&
+   test_completion "git --no-pager check" "checkout " &&
+   test_completion "git --work-tree=foo check" "checkout " &&
+   test_completion "git --namespace=foo check" "checkout " &&
+   test_completion "git --paginate check" "checkout " &&
+   test_completion "git --info-path check" "checkout " &&
+   test_completion "git --no-replace-objects check" "checkout "
 '
 
 test_done
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] revision: add --reflog-message= to grep reflog messages

2012-09-26 Thread Nguyễn Thái Ngọc Duy
Both "git log" and "git reflog show" recognize this option.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Itch: how to show reflogs for checkout operation only?

 Instead of ignoring when -g is not given, we might want to imply -g.

 Still itch: grep highlight! For all applicable areas: commit headers
 including reflog messages, commit body, diff.

 Documentation/rev-list-options.txt |  5 +
 revision.c | 30 ++
 revision.h |  1 +
 3 files changed, 36 insertions(+)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 1fc2a18..aeaa58c 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -51,6 +51,11 @@ endif::git-rev-list[]
commits whose author matches any of the given patterns are
chosen (similarly for multiple `--committer=`).
 
+--reflog-message=::
+   Limit the commits output to ones with reflog messages that
+   match the specified pattern (regular expression). Ignored unless
+   --walk-reflogs is given.
+
 --grep=::
 
Limit the commits output to ones with log message that
diff --git a/revision.c b/revision.c
index ae12e11..ee55bb2 100644
--- a/revision.c
+++ b/revision.c
@@ -1053,6 +1053,11 @@ void init_revisions(struct rev_info *revs, const char 
*prefix)
revs->grep_filter.header_tail = &(revs->grep_filter.header_list);
revs->grep_filter.regflags = REG_NEWLINE;
 
+   revs->reflog_filter.status_only = 1;
+   revs->reflog_filter.pattern_tail = &(revs->reflog_filter.pattern_list);
+   revs->reflog_filter.header_tail = &(revs->reflog_filter.header_list);
+   revs->reflog_filter.regflags = REG_NEWLINE;
+
diff_setup(&revs->diffopt);
if (prefix && !revs->diffopt.prefix) {
revs->diffopt.prefix = prefix;
@@ -1298,6 +1303,12 @@ static void add_message_grep(struct rev_info *revs, 
const char *pattern)
add_grep(revs, pattern, GREP_PATTERN_BODY);
 }
 
+static void add_reflog_grep(struct rev_info *revs, const char *ptn)
+{
+   append_grep_pattern(&revs->reflog_filter, ptn,
+   "command line", 0, GREP_PATTERN);
+}
+
 static int handle_revision_opt(struct rev_info *revs, int argc, const char 
**argv,
   int *unkc, const char **unkv)
 {
@@ -1600,15 +1611,23 @@ static int handle_revision_opt(struct rev_info *revs, 
int argc, const char **arg
return argcount;
} else if (!strcmp(arg, "--grep-debug")) {
revs->grep_filter.debug = 1;
+   } else if ((argcount = parse_long_opt("reflog-message",
+ argv, &optarg))) {
+   add_reflog_grep(revs, optarg);
+   return argcount;
} else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
revs->grep_filter.regflags |= REG_EXTENDED;
+   revs->reflog_filter.regflags |= REG_EXTENDED;
} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
revs->grep_filter.regflags |= REG_ICASE;
+   revs->reflog_filter.regflags |= REG_ICASE;
DIFF_OPT_SET(&revs->diffopt, PICKAXE_IGNORE_CASE);
} else if (!strcmp(arg, "--fixed-strings") || !strcmp(arg, "-F")) {
revs->grep_filter.fixed = 1;
+   revs->reflog_filter.fixed = 1;
} else if (!strcmp(arg, "--all-match")) {
revs->grep_filter.all_match = 1;
+   revs->reflog_filter.all_match = 1;
} else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
if (strcmp(optarg, "none"))
git_log_output_encoding = xstrdup(optarg);
@@ -1891,6 +1910,7 @@ int setup_revisions(int argc, const char **argv, struct 
rev_info *revs, struct s
diff_setup_done(&revs->diffopt);
 
compile_grep_patterns(&revs->grep_filter);
+   compile_grep_patterns(&revs->reflog_filter);
 
if (revs->reverse && revs->reflog_info)
die("cannot combine --reverse with --walk-reflogs");
@@ -2242,6 +2262,16 @@ enum commit_action get_commit_action(struct rev_info 
*revs, struct commit *commi
((revs->max_parents >= 0) && (n > revs->max_parents)))
return commit_ignore;
}
+   if (revs->reflog_info &&
+   revs->reflog_filter.pattern_list) {
+   struct strbuf sb = STRBUF_INIT;
+   int ignore;
+   get_reflog_message(&sb, revs->reflog_info);
+   ignore = !grep_buffer(&revs->reflog_filter, sb.buf, sb.len);
+   strbuf_release(&sb);
+   if (ignore)
+   return commit_ignore;
+   }
if (!commit_match(commit, revs))
return commit_ignore;
if (revs->prune && revs->dense) {
diff --git a/revision.h b/revision.h
index a95bd0b..0ebe34b 100

Re: Problem completing remotes when .git/remotes exits

2012-09-26 Thread SZEDER Gábor
On Tue, Sep 25, 2012 at 04:43:59PM -0700, Junio C Hamano wrote:
> SZEDER Gábor  writes:
> 
> > -   test -d "$d/remotes" && ls -1 "$d/remotes"
> > +   test -d "$d/remotes" && command ls -1 "$d/remotes"
> 
> Yuck.  For normal scripts, nobody sane would define "alias" for
> non-interactive environments, but because these things work in an
> interactive environment, we have to protect ourselves from user
> aliases.  Not just "ls", but "test" we see above may misbehave X-<.

Right, however, while ls is frequently aliased (my ubuntu box has
alias ls='ls --color=auto' in /etc/skel/.bashrc by default, but that's
not an issue in this case), I think aliasing test is just crazy.
Yeah, it's possible, but if we go down that route, then we should also
worry about the [ builtin being aliased:

$ if [ -r nonexisting ] ; then echo found ; fi
$ alias [='echo using aliased ['
$ if [ -r nonexisting ] ; then echo found ; fi
using aliased [ -r nonexisting ]
found
$

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 5/5] Support "**" in .gitignore and .gitattributes patterns using wildmatch()

2012-09-26 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 Documentation/gitignore.txt|  3 +++
 attr.c |  4 +++-
 dir.c  |  5 -
 t/t0003-attributes.sh  | 17 +
 t/t3001-ls-files-others-exclude.sh | 11 +++
 5 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt
index c1f692a..eb81d31 100644
--- a/Documentation/gitignore.txt
+++ b/Documentation/gitignore.txt
@@ -93,6 +93,9 @@ PATTERN FORMAT
For example, "Documentation/{asterisk}.html" matches
"Documentation/git.html" but not "Documentation/ppc/ppc.html"
or "tools/perf/Documentation/perf.html".
++
+Contrary to fnmatch(3), git matches "**" to anything including
+slashes, similar to rsync(1).
 
  - A leading slash matches the beginning of the pathname.
For example, "/{asterisk}.c" matches "cat-file.c" but not
diff --git a/attr.c b/attr.c
index b52efb5..15ebaa1 100644
--- a/attr.c
+++ b/attr.c
@@ -12,6 +12,7 @@
 #include "exec_cmd.h"
 #include "attr.h"
 #include "dir.h"
+#include "wildmatch.h"
 
 const char git_attr__true[] = "(builtin)true";
 const char git_attr__false[] = "\0(builtin)false";
@@ -663,7 +664,8 @@ static int path_matches(const char *pathname, int pathlen,
return 0;
if (baselen != 0)
baselen++;
-   return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+   return (ignore_case && iwildmatch(pattern, pathname + baselen)) ||
+   (!ignore_case && wildmatch(pattern, pathname + baselen));
 }
 
 static int macroexpand_one(int attr_nr, int rem);
diff --git a/dir.c b/dir.c
index 240bf0c..92cda82 100644
--- a/dir.c
+++ b/dir.c
@@ -8,6 +8,7 @@
 #include "cache.h"
 #include "dir.h"
 #include "refs.h"
+#include "wildmatch.h"
 
 struct path_simplify {
int len;
@@ -573,7 +574,9 @@ int excluded_from_list(const char *pathname,
namelen -= prefix;
}
 
-   if (!namelen || !fnmatch_icase(exclude, name, FNM_PATHNAME))
+   if (!namelen ||
+   ((ignore_case && iwildmatch(exclude, name)) ||
+(!ignore_case && wildmatch(exclude, name
return to_exclude;
}
return -1; /* undecided */
diff --git a/t/t0003-attributes.sh b/t/t0003-attributes.sh
index febc45c..6c3c554 100755
--- a/t/t0003-attributes.sh
+++ b/t/t0003-attributes.sh
@@ -232,4 +232,21 @@ test_expect_success 'bare repository: test 
info/attributes' '
attr_check subdir/a/i unspecified
 '
 
+test_expect_success '"**" test' '
+   cd .. &&
+   echo "**/f foo=bar" >.gitattributes &&
+   cat <<\EOF >expect &&
+f: foo: unspecified
+a/f: foo: bar
+a/b/f: foo: bar
+a/b/c/f: foo: bar
+EOF
+   git check-attr foo -- "f" >actual 2>err &&
+   git check-attr foo -- "a/f" >>actual 2>>err &&
+   git check-attr foo -- "a/b/f" >>actual 2>>err &&
+   git check-attr foo -- "a/b/c/f" >>actual 2>>err &&
+   test_cmp expect actual &&
+   test_line_count = 0 err
+'
+
 test_done
diff --git a/t/t3001-ls-files-others-exclude.sh 
b/t/t3001-ls-files-others-exclude.sh
index c8fe978..67c8bcf 100755
--- a/t/t3001-ls-files-others-exclude.sh
+++ b/t/t3001-ls-files-others-exclude.sh
@@ -214,4 +214,15 @@ test_expect_success 'subdirectory ignore (l1)' '
test_cmp expect actual
 '
 
+
+test_expect_success 'ls-files with "**" patterns' '
+   cat <<\EOF >expect &&
+one/a.1
+one/two/a.1
+three/a.1
+EOF
+   git ls-files -o -i --exclude "**/a.1" >actual
+   test_cmp expect actual
+'
+
 test_done
-- 
1.7.12.1.406.g6ab07c4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 4/5] Integrate wildmatch to git

2012-09-26 Thread Nguyễn Thái Ngọc Duy
This makes wildmatch.c part of libgit.a and builds test-wildmatch; the
dependency on libpopt in the original has been replaced with the use
of our parse-options. Global variables in test-wildmatch are marked
static to avoid sparse warnings.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Ramsay Jones 
Signed-off-by: Junio C Hamano 
---
 .gitignore   |  1 +
 Makefile |  3 ++
 t/t3070-wildmatch.sh | 27 
 test-wildmatch.c | 88 ++--
 wildmatch.c  | 26 +---
 5 files changed, 75 insertions(+), 70 deletions(-)
 create mode 100755 t/t3070-wildmatch.sh

diff --git a/.gitignore b/.gitignore
index 68fe464..54b1b3b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -196,6 +196,7 @@
 /test-sigchain
 /test-subprocess
 /test-svn-fe
+/test-wildmatch
 /common-cmds.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index 26b697d..d6235e6 100644
--- a/Makefile
+++ b/Makefile
@@ -504,6 +504,7 @@ TEST_PROGRAMS_NEED_X += test-sha1
 TEST_PROGRAMS_NEED_X += test-sigchain
 TEST_PROGRAMS_NEED_X += test-subprocess
 TEST_PROGRAMS_NEED_X += test-svn-fe
+TEST_PROGRAMS_NEED_X += test-wildmatch
 
 TEST_PROGRAMS = $(patsubst %,%$X,$(TEST_PROGRAMS_NEED_X))
 
@@ -676,6 +677,7 @@ LIB_H += userdiff.h
 LIB_H += utf8.h
 LIB_H += varint.h
 LIB_H += walker.h
+LIB_H += wildmatch.h
 LIB_H += wt-status.h
 LIB_H += xdiff-interface.h
 LIB_H += xdiff/xdiff.h
@@ -807,6 +809,7 @@ LIB_OBJS += utf8.o
 LIB_OBJS += varint.o
 LIB_OBJS += version.o
 LIB_OBJS += walker.o
+LIB_OBJS += wildmatch.o
 LIB_OBJS += wrapper.o
 LIB_OBJS += write_or_die.o
 LIB_OBJS += ws.o
diff --git a/t/t3070-wildmatch.sh b/t/t3070-wildmatch.sh
new file mode 100755
index 000..c4da26c
--- /dev/null
+++ b/t/t3070-wildmatch.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+test_description='wildmatch tests'
+
+. ./test-lib.sh
+
+test_wildmatch() {
+test_expect_success "wildmatch $*" "
+   test-wildmatch $* ../t3070/wildtest.txt >actual &&
+   echo 'No wildmatch errors found.' >expected &&
+   test_cmp expected actual
+"
+}
+
+test_wildmatch -x1
+test_wildmatch -x1 -e1
+test_wildmatch -x1 -else
+test_wildmatch -x2
+test_wildmatch -x2 -ese
+test_wildmatch -x3
+test_wildmatch -x3 -e1
+test_wildmatch -x4
+test_wildmatch -x4 -e2e
+test_wildmatch -x5
+test_wildmatch -x5 -es
+
+test_done
diff --git a/test-wildmatch.c b/test-wildmatch.c
index 88585c2..bb726c8 100644
--- a/test-wildmatch.c
+++ b/test-wildmatch.c
@@ -19,34 +19,38 @@
 
 /*#define COMPARE_WITH_FNMATCH*/
 
-#define WILD_TEST_ITERATIONS
-#include "lib/wildmatch.c"
+#include "cache.h"
+#include "parse-options.h"
+#include "wildmatch.h"
 
-#include 
+#ifndef MAXPATHLEN
+#define MAXPATHLEN 1024
+#endif
+#ifdef NO_STRLCPY
+#include "compat/strlcpy.c"
+#define strlcpy gitstrlcpy
+#endif
 
 #ifdef COMPARE_WITH_FNMATCH
 #include 
 
-int fnmatch_errors = 0;
+static int fnmatch_errors = 0;
 #endif
 
-int wildmatch_errors = 0;
-char number_separator = ',';
+static int wildmatch_errors = 0;
 
 typedef char bool;
 
-int output_iterations = 0;
-int explode_mod = 0;
-int empties_mod = 0;
-int empty_at_start = 0;
-int empty_at_end = 0;
-
-static struct poptOption long_options[] = {
-  /* longName, shortName, argInfo, argPtr, value, descrip, argDesc */
-  {"iterations", 'i', POPT_ARG_NONE,   &output_iterations, 0, 0, 0},
-  {"empties",'e', POPT_ARG_STRING, 0, 'e', 0, 0},
-  {"explode",'x', POPT_ARG_INT,&explode_mod, 0, 0, 0},
-  {0,0,0,0, 0, 0, 0}
+static int explode_mod = 0;
+static int empties_mod = 0;
+static int empty_at_start = 0;
+static int empty_at_end = 0;
+static char *empties;
+
+static struct option options[] = {
+  OPT_STRING('e', "empties", &empties, "", ""),
+  OPT_INTEGER('x', "explode", &explode_mod, ""),
+  OPT_END(),
 };
 
 /* match just at the start of string (anchored tests) */
@@ -100,51 +104,33 @@ run_test(int line, bool matches,
fnmatch_errors++;
 }
 #endif
-if (output_iterations) {
-   printf("%d: \"%s\" iterations = %d\n", line, pattern,
-  wildmatch_iteration_count);
-}
 }
 
 int
 main(int argc, char **argv)
 {
 char buf[2048], *s, *string[2], *end[2];
-const char *arg;
 FILE *fp;
-int opt, line, i, flag[2];
-poptContext pc = poptGetContext("wildtest", argc, (const char**)argv,
-   long_options, 0);
-
-while ((opt = poptGetNextOpt(pc)) != -1) {
-   switch (opt) {
- case 'e':
-   arg = poptGetOptArg(pc);
-   empties_mod = atoi(arg);
-   if (strchr(arg, 's'))
-   empty_at_start = 1;
-   if (strchr(arg, 'e'))
-   empty_at_end = 1;
-   if (!explode_mod)
-   explode_mod = 1024;
-   break;
- default:
-   fprintf(stderr, "%s: %s\n",
-   poptBadOption(pc, POPT_BADOPTION_NOALIAS),
-   poptStrerror(opt));
-   exit(1);
-   }
+int line, i, flag[2];
+

[PATCH 3/5] compat/wildmatch: fix case-insensitive matching

2012-09-26 Thread Nguyễn Thái Ngọc Duy
dowild() does case insensitive matching by lower-casing the text. That
means lower case letters in patterns imply case-insensitive matching,
but upper case means exact matching.

We do not want that subtlety. Lower case pattern too so iwildmatch()
always does what we expect it to do.

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 wildmatch.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/wildmatch.c b/wildmatch.c
index e824eb2..c7f7f9f 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -81,6 +81,8 @@ static int dowild(const uchar *p, const uchar *text,
}
if (force_lower_case && ISUPPER(t_ch))
t_ch = tolower(t_ch);
+   if (force_lower_case && ISUPPER(p_ch))
+   p_ch = tolower(p_ch);
switch (p_ch) {
  case '\\':
/* Literal match with following character.  Note that the test
-- 
1.7.12.1.406.g6ab07c4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/5] compat/wildmatch: remove static variable force_lower_case

2012-09-26 Thread Nguyễn Thái Ngọc Duy
One place less to worry about thread safety

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 wildmatch.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/wildmatch.c b/wildmatch.c
index f3a1731..e824eb2 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -57,11 +57,10 @@
 int wildmatch_iteration_count;
 #endif
 
-static int force_lower_case = 0;
-
 /* Match pattern "p" against the a virtually-joined string consisting
  * of "text" and any strings in array "a". */
-static int dowild(const uchar *p, const uchar *text, const uchar*const *a)
+static int dowild(const uchar *p, const uchar *text,
+ const uchar*const *a, int force_lower_case)
 {
 uchar p_ch;
 
@@ -121,7 +120,7 @@ static int dowild(const uchar *p, const uchar *text, const 
uchar*const *a)
t_ch = *text;
continue;
}
-   if ((matched = dowild(p, text, a)) != FALSE) {
+   if ((matched = dowild(p, text, a, force_lower_case)) != FALSE) {
if (!special || matched != ABORT_TO_STARSTAR)
return matched;
} else if (!special && t_ch == '/')
@@ -291,7 +290,7 @@ int wildmatch(const char *pattern, const char *text)
 #ifdef WILD_TEST_ITERATIONS
 wildmatch_iteration_count = 0;
 #endif
-return dowild((const uchar*)pattern, (const uchar*)text, nomore) == TRUE;
+return dowild((const uchar*)pattern, (const uchar*)text, nomore, 0) == 
TRUE;
 }
 
 /* Match the "pattern" against the forced-to-lower-case "text" string. */
@@ -302,9 +301,7 @@ int iwildmatch(const char *pattern, const char *text)
 #ifdef WILD_TEST_ITERATIONS
 wildmatch_iteration_count = 0;
 #endif
-force_lower_case = 1;
-ret = dowild((const uchar*)pattern, (const uchar*)text, nomore) == TRUE;
-force_lower_case = 0;
+ret = dowild((const uchar*)pattern, (const uchar*)text, nomore, 1) == TRUE;
 return ret;
 }
 
@@ -331,7 +328,7 @@ int wildmatch_array(const char *pattern, const char*const 
*texts, int where)
 if (!text)
return FALSE;
 
-if ((matched = dowild(p, text, a)) != TRUE && where < 0
+if ((matched = dowild(p, text, a, 0)) != TRUE && where < 0
  && matched != ABORT_ALL) {
while (1) {
if (*text == '\0') {
@@ -339,7 +336,7 @@ int wildmatch_array(const char *pattern, const char*const 
*texts, int where)
return FALSE;
continue;
}
-   if (*text++ == '/' && (matched = dowild(p, text, a)) != FALSE
+   if (*text++ == '/' && (matched = dowild(p, text, a, 0)) != FALSE
 && matched != ABORT_TO_STARSTAR)
break;
}
-- 
1.7.12.1.406.g6ab07c4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/5] Import wildmatch from rsync

2012-09-26 Thread Nguyễn Thái Ngọc Duy
These files are from rsync.git commit
f92f5b166e3019db42bc7fe1aa2f1a9178cd215d, which was the last commit
before rsync turned GPL-3. All files are imported as-is and
no-op. Adaptation is done in a separate patch.

rsync.git   ->  git.git
lib/wildmatch.[ch]  wildmatch.[ch]
wildtest.c  test-wildmatch.c
wildtest.txtt/t3070/wildtest.txt

Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 
---
 t/t3070/wildtest.txt | 165 +++
 test-wildmatch.c | 222 +++
 wildmatch.c  | 368 +++
 wildmatch.h  |   6 +
 4 files changed, 761 insertions(+)
 create mode 100644 t/t3070/wildtest.txt
 create mode 100644 test-wildmatch.c
 create mode 100644 wildmatch.c
 create mode 100644 wildmatch.h

diff --git a/t/t3070/wildtest.txt b/t/t3070/wildtest.txt
new file mode 100644
index 000..42c1678
--- /dev/null
+++ b/t/t3070/wildtest.txt
@@ -0,0 +1,165 @@
+# Input is in the following format (all items white-space separated):
+#
+# The first two items are 1 or 0 indicating if the wildmat call is expected to
+# succeed and if fnmatch works the same way as wildmat, respectively.  After
+# that is a text string for the match, and a pattern string.  Strings can be
+# quoted (if desired) in either double or single quotes, as well as backticks.
+#
+# MATCH FNMATCH_SAME "text to match" 'pattern to use'
+
+# Basic wildmat features
+1 1 foofoo
+0 1 foobar
+1 1 '' ""
+1 1 foo???
+0 1 foo??
+1 1 foo*
+1 1 foof*
+0 1 foo*f
+1 1 foo*foo*
+1 1 foobar *ob*a*r*
+1 1 aaabababab *ab
+1 1 foo*   foo\*
+0 1 foobar foo\*bar
+1 1 f\oo   f\\oo
+1 1 ball   *[al]?
+0 1 ten[ten]
+1 1 ten**[!te]
+0 1 ten**[!ten]
+1 1 tent[a-g]n
+0 1 tent[!a-g]n
+1 1 tont[!a-g]n
+1 1 tont[^a-g]n
+1 1 a]ba[]]b
+1 1 a-ba[]-]b
+1 1 a]ba[]-]b
+0 1 aaba[]-]b
+1 1 aaba[]a-]b
+1 1 ]  ]
+
+# Extended slash-matching features
+0 1 foo/baz/barfoo*bar
+1 1 foo/baz/barfoo**bar
+0 1 foo/barfoo?bar
+0 1 foo/barfoo[/]bar
+0 1 foo/barf[^eiu][^eiu][^eiu][^eiu][^eiu]r
+1 1 foo-barf[^eiu][^eiu][^eiu][^eiu][^eiu]r
+0 1 foo**/foo
+1 1 /foo   **/foo
+1 1 bar/baz/foo**/foo
+0 1 bar/baz/foo*/foo
+0 0 foo/bar/baz**/bar*
+1 1 deep/foo/bar/baz   **/bar/*
+0 1 deep/foo/bar/baz/  **/bar/*
+1 1 deep/foo/bar/baz/  **/bar/**
+0 1 deep/foo/bar   **/bar/*
+1 1 deep/foo/bar/  **/bar/**
+1 1 foo/bar/baz**/bar**
+1 1 foo/bar/baz/x  */bar/**
+0 0 deep/foo/bar/baz/x */bar/**
+1 1 deep/foo/bar/baz/x **/bar/*/*
+
+# Various additional tests
+0 1 acrt   a[c-c]st
+1 1 acrt   a[c-c]rt
+0 1 ]  [!]-]
+1 1 a  [!]-]
+0 1 '' \
+0 1 \  \
+0 1 /\ */\
+1 1 /\ */\\
+1 1 foofoo
+1 1 @foo   @foo
+0 1 foo@foo
+1 1 [ab]   \[ab]
+1 1 [ab]   [[]ab]
+1 1 [ab]   [[:]ab]
+0 1 [ab]   [[::]ab]
+1 1 [ab]   [[:digit]ab]
+1 1 [ab]   [\[:]ab]
+1 1 ?a?b   \??\?b
+1 1 abc\a\b\c
+0 1 foo''
+1 1 foo/bar/baz/to **/t[o]
+
+# Character class tests
+1 1 a1B[[:alpha:]][[:digit:]][[:upper:]]
+0 1 a  [[:digit:][:upper:][:space:]]
+1 1 A  [[:digit:][:upper:][:space:]]
+1 1 1  [[:digit:][:upper:][:space:]]
+0 1 1  [[:digit:][:upper:][:spaci:]]
+1 1 ' '[[:digit:][:upper:][:space:]]
+0 1 .  [[:digit:][:upper:][:space:]]
+1 1 .  [[:digit:][:punct:][:space:]]
+1 1 5  [[:xdigit:]]
+1 1 f  [[:xdigit:]]
+1 1 D  [[:xdigit:]]
+1 1 _  
[[:alnum:][:alpha:][:blank:][:cntrl:][:digit:][:graph:][:lower:][:print:][:punct:][:space:][:upper:][:xdigit:]]
+#1 1 � 
[^[:alnum:][:alpha:][:blank:][:cntrl:][:digit:][:graph:][:lower:][:print:][:punct:][:space:][:upper:][:xdigit:]]
+1 1   
[^[:alnum:][:alpha:][:blank:][:digit:][:graph:][:lower:][:print:][:punct:][:space:][:upper:][:xdigit:]]
+1 1 .  
[^[:alnum:][:alpha:][:blank:][:cntrl:][:digit:][:lower:][:space:][:upper:][:xdigit:]]
+1 1 5  [a-c[:digit:]x-z]
+1 1

[PATCH 0/5] wildmatch series update

2012-09-26 Thread Nguyễn Thái Ngọc Duy
No functional changes. Just incorporate changes from Ramsay and Johannes.

Nguyễn Thái Ngọc Duy (5):
  Import wildmatch from rsync
  compat/wildmatch: remove static variable force_lower_case
  compat/wildmatch: fix case-insensitive matching
  Integrate wildmatch to git
  Support "**" in .gitignore and .gitattributes patterns using
wildmatch()

 .gitignore |   1 +
 Documentation/gitignore.txt|   3 +
 Makefile   |   3 +
 attr.c |   4 +-
 dir.c  |   5 +-
 t/t0003-attributes.sh  |  17 ++
 t/t3001-ls-files-others-exclude.sh |  11 ++
 t/t3070-wildmatch.sh   |  27 +++
 t/t3070/wildtest.txt   | 165 +
 test-wildmatch.c   | 208 ++
 wildmatch.c| 355 +
 wildmatch.h|   6 +
 12 files changed, 803 insertions(+), 2 deletions(-)
 create mode 100755 t/t3070-wildmatch.sh
 create mode 100644 t/t3070/wildtest.txt
 create mode 100644 test-wildmatch.c
 create mode 100644 wildmatch.c
 create mode 100644 wildmatch.h

-- 
1.7.12.1.406.g6ab07c4

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: DWIM .git repository discovery

2012-09-26 Thread Nguyen Thai Ngoc Duy
On Wed, Sep 26, 2012 at 11:21 AM, Junio C Hamano  wrote:
> Nguyen Thai Ngoc Duy  writes:
>
>> I often find myself attempting to examine another repository,
>> especially in projects that are closely related but put in different
>> git repos. It's usually just a diff or log command
>>
>> git log --patch ../path/to/another/repo/path/to/file.c
>
> I personally do not think it is _too_ bad to internally do
>
> (cd ../path/to/another/repo/path/to &&
>  git log --patch file.c)
>

As long as the .git discovery and path rewriting can be done
automatically, that'd be nice. But..

> but I doubt it is worth the numerous implications (I am not talking
> about implementation complexity at all, but the conceptual burden).
>
> For example, where in the working tree of the other project should
> the command run?  The output from "log -p" happens to be always
> relative to the top of the working tree, but that does not
> necessarily hold true for other subcommands.

Returned paths should always be relative to cwd (well except diff/log
which are prefixed by [ab]/). cd'ing internally like above makes it
more confusing imo. Take grep for example, I find it natural for "git
grep foo -- ../other/repo/bar/" to return "../other/repo/bar/foo.c
...".

Prefix currently does not take "../blah" form, but I see no reasons
why it can't/shouldn't. $(cwd) is most likely outside the other
project's working directory. An exception running from inside a
submodule and examining the parent repository.

For too long relative paths, we could even display in ":/" pathspec
notation. Users who don't recognize them either look up documentation,
or gradually learn to drop the ":/" part, even without know what it's
for.

Repo modification commands like git-add could cause greater confusion
(I added and committed it (on the other repo), but when I pushed (on
this repo), the changes aren't there). We could and probably should
avoid dwim-ing these cases.

A good way to make it clear you're accessing another repository is
prompt (y/n) from users. I don't think pressing Ctrl-C to abort the
command is too much hassle (the command would fail anyway with
"outside repository" message, without dwim).

> A worse thing is that there is an oddball case "diff[ --no-index]"
> that changes behaviour depending on the pathspec points at inside or
> outside the repository.

Yep. But not many commands do this fortunately (diff and grep only?)

> I think that this is a road to insanity; anybody who thinks along
> this line is already on the other side of the line, I would have to
> say ;-).

We could go slowly and stop before being diagnosed insane. I mean the
trick can be opted in for a command subset where it makes sense to do
so.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CRLF, LF ... CR ?

2012-09-26 Thread Jens Bauer
Hi David and Junio.

Woops, that's what happens when deleting a block of lines in a message...

The CR/LF/CRLF implementation depends a lot on if git is reading a stream or 
reading from memory.

I'd like to correct the last line to read...
Worst case is, if a file contains mixed CR, LF and CRLF, such as a text-file, 
that contains all 3 kinds of line endings (because 3 different people have been 
editing the file).


Love
Jens

On Wed, 26 Sep 2012 12:12:39 +0200, Jens Bauer wrote:
> The implementation would be dependent on on how git is currently 
> handling lines.
> Worst case is, if it's mixed CR, LF and CRLF, such as a text-file, 
> that contains all 3 kinds of line endings (because 3 different people 
> have been editing the file).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CRLF, LF ... CR ?

2012-09-26 Thread Jens Bauer
Hi David and Junio.

At first, I was planning to reply that I'd probably not be qualified for that.
But to tell the truth, I have been writing a lot of CR/LF/CRLF code throughout 
the years, so maybe I could do it.
Unfortunately, I have to go slow about programming, because I burned myself out 
a number of times, so programming "work" is not compatible with me; though 
programming "hobby" is.

The implementation would be dependent on on how git is currently handling lines.
Worst case is, if it's mixed CR, LF and CRLF, such as a text-file, that 
contains all 3 kinds of line endings (because 3 different people have been 
editing the file).


Love
Jens

On Wed, 26 Sep 2012 01:42:02 -0700, David Aguilar wrote:
> On Thu, Sep 13, 2012 at 9:51 PM, Junio C Hamano  wrote:
>> David Aguilar  writes:
>> 
>>> git doesn't really even support LF.
>> 
>> At the storage level that is correct, but the above is a bit of
>> stretch.  It may not be "support", but git _does_ rely on LF when
>> running many text oriented operations (a rough rule of thumb is
>> "does 'a line' in a file matter to the operation?").  Think about
>> "git diff" and "git blame".
> 
> Thanks for the thorough explanation.  You're 100% correct, as always.
> 
> I'll be honest: I had a small bias when responding.
> I didn't want anyone to think a "autocr" feature would be useful,
> so I played the "git is really simple" angle hoping it would
> put a kabosh on the idea.  That was a little silly of me.
> 
> That said, perhaps the "autocrlf" code is simple enough that it
> could be easily tweaked to also handle this special case, but
> I am not familiar with the code enough to say.  My gut feeling
> was that it was too narrow a use case.  I guess if someone[*]
> wanted to whip up a patch then it would be a different story,
> but it doesn't seem to be the itch of anyone around here so far.
> 
> [*] Jens, that could be you ;-)
> 
> cheers,
> -- 
> David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Can git pull from a mercurial repository?

2012-09-26 Thread Georgi Chorbadzhiyski
Around 09/26/2012 11:46 AM, Max Horn scribbled:
> On 26.09.2012, at 09:38, Georgi Chorbadzhiyski wrote:
>> Around 09/25/2012 05:15 PM, Max Horn scribbled:
>>> I think there is a lot of demand for a "git-hg" bridge, a way to seemlessly 
>>> access a Mercurial repository as if it was a git repository. A converse to 
>>> hg-git 
>>
>> I've already mentioned this, but such a tool already exists and it
>> is working very well (IMHO): http://offbytwo.com/git-hg/
> 
> I guess this is a matter of perspective. It doesn't work at all for me 
> because it does not really support pushing. (It does have a "push" command, 
> but at least last time I looked, it was utterly broken; see also 
>  for a discussion 
> (not written by me!). I'd be happy to learn that has changed, though I just 
> looked, and it still uses "hg convert", so I don't see how it possibly could 
> work...

I have not tested push (I'm using git-hg to sync hg repo and develop
using git, no pushing back to hg, just sending patches).

According to git-hg README "Push supported added as well although it
is still experimental". You should report the "push" bugs to the
author(s) they may be able to fix them.

-- 
Georgi Chorbadzhiyski
http://georgi.unixsol.org/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Can git pull from a mercurial repository?

2012-09-26 Thread Max Horn

On 26.09.2012, at 09:38, Georgi Chorbadzhiyski wrote:

> Around 09/25/2012 05:15 PM, Max Horn scribbled:
>> I think there is a lot of demand for a "git-hg" bridge, a way to seemlessly 
>> access a Mercurial repository as if it was a git repository. A converse to 
>> hg-git 
> 
> I've already mentioned this, but such a tool already exists and it
> is working very well (IMHO): http://offbytwo.com/git-hg/

I guess this is a matter of perspective. It doesn't work at all for me because 
it does not really support pushing. (It does have a "push" command, but at 
least last time I looked, it was utterly broken; see also 
 for a discussion 
(not written by me!). I'd be happy to learn that has changed, though I just 
looked, and it still uses "hg convert", so I don't see how it possibly could 
work...

Cheers,
Max--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: CRLF, LF ... CR ?

2012-09-26 Thread David Aguilar
On Thu, Sep 13, 2012 at 9:51 PM, Junio C Hamano  wrote:
> David Aguilar  writes:
>
>> git doesn't really even support LF.
>
> At the storage level that is correct, but the above is a bit of
> stretch.  It may not be "support", but git _does_ rely on LF when
> running many text oriented operations (a rough rule of thumb is
> "does 'a line' in a file matter to the operation?").  Think about
> "git diff" and "git blame".

Thanks for the thorough explanation.  You're 100% correct, as always.

I'll be honest: I had a small bias when responding.
I didn't want anyone to think a "autocr" feature would be useful,
so I played the "git is really simple" angle hoping it would
put a kabosh on the idea.  That was a little silly of me.

That said, perhaps the "autocrlf" code is simple enough that it
could be easily tweaked to also handle this special case, but
I am not familiar with the code enough to say.  My gut feeling
was that it was too narrow a use case.  I guess if someone[*]
wanted to whip up a patch then it would be a different story,
but it doesn't seem to be the itch of anyone around here so far.

[*] Jens, that could be you ;-)

cheers,
-- 
David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Can git pull from a mercurial repository?

2012-09-26 Thread Georgi Chorbadzhiyski
Around 09/25/2012 05:15 PM, Max Horn scribbled:
> I think there is a lot of demand for a "git-hg" bridge, a way to seemlessly 
> access a Mercurial repository as if it was a git repository. A converse to 
> hg-git 

I've already mentioned this, but such a tool already exists and it
is working very well (IMHO): http://offbytwo.com/git-hg/

-- 
Georgi Chorbadzhiyski
http://georgi.unixsol.org/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: DWIM .git repository discovery

2012-09-26 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 26.09.2012 06:21:
> Nguyen Thai Ngoc Duy  writes:
> 
>> I often find myself attempting to examine another repository,
>> especially in projects that are closely related but put in different
>> git repos. It's usually just a diff or log command
>>
>> git log --patch ../path/to/another/repo/path/to/file.c
> 
> I personally do not think it is _too_ bad to internally do
> 
>   (cd ../path/to/another/repo/path/to &&
>git log --patch file.c)
> 
> but I doubt it is worth the numerous implications (I am not talking
> about implementation complexity at all, but the conceptual burden).

Yeah, but doing the above from an alias or help script should be fine
and can be tailored to your use case. Say, a script like

arg1="$1"
shift
cd $(dirname "$arg1")
git "$@" $(basename "$args1")

should cover a couple of use cases already. (Error checking is for the
faint of heart only.)

> For example, where in the working tree of the other project should
> the command run?  The output from "log -p" happens to be always
> relative to the top of the working tree, but that does not
> necessarily hold true for other subcommands.
> 
> A worse thing is that there is an oddball case "diff[ --no-index]"
> that changes behaviour depending on the pathspec points at inside or
> outside the repository.
> 
> I think that this is a road to insanity; anybody who thinks along
> this line is already on the other side of the line, I would have to
> say ;-).
> 

Don't we all just walk on the line?

Michael
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html