Re: Unable to shrink repository size

[Fredrik Gustafsson]

On Wed, Mar 05, 2014 at 08:55:30PM -0600, Robert Dailey wrote:
> What I'd like to do is somehow hunt down the largest commit (*not*
> blob) in the entire history of the repository to hopefully find out
> where huge directories have been checked in.
> 
> I can't do a search for largest file (which most google results seem
> to show to do) since the culprit is really thousands of unnecessary
> files checked into a single subdirectory somewhere in history.
> 
> Can anyone offer me some advice to help me reduce the size of my repo
> further? Thanks.

I'm not sure if you can do this with git. However since git is a command
line application it's pretty easy to script it with sh. The negative
part beeing the lack of speed, but since this is a one-time thing I
don't think that it matters.

Since you told us that it is a commit with a huge number of files that
you're looking for, I took that approach instead of calculating the size
of each commit, since that would be more expensive to do.

for commit in $(git log --pretty=oneline | cut -d " " -f 1)
do
nbr=$(git show --pretty="format:" --name-only $commit | wc -l)
echo "$nbr: $commit"
done | sort | tail -1

This will give you the commit with most files changed. (Although, there
will be a +1 error on the number of files edited).

-- 
Med vänlig hälsning
Fredrik Gustafsson

tel: 0733-608274
e-post: iv...@iveqy.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

RFC GSoC idea: git configuration caching (needs co-mentor!)

[Michael Haggerty]

I just wrote up the idea that fell out of the discussion [1] about the
other configuration features that I proposed.  As far as I am concerned,
it can be merged as soon as somebody volunteers as a co-mentor.  The
idea is embodied in a pull request against the git.github.io repository
[2]; the text is also appended below for your convenience.

Michael

[1] http://article.gmane.org/gmane.comp.version-control.git/242952
[2] https://github.com/git/git.github.io/pull/7

### git configuration API improvements

There are many places in Git that need to read a configuration value.
Currently, each such site calls `git_config()`, which reads and parses
the configuration files every time that it is called.  This is
wasteful, because it results in the configuration files being
processed multiple times during a single `git` invocation.  It also
prevents the implementation of potential new features, like adding
syntax to allow a configuration file to unset a previously-set value.

This goal of this project is to make configuration work as follows:

* Read the configuration from files once and cache the results in an
  appropriate data structure in memory.

* Change `git_config()` to iterate through the pre-read values in
  memory rather than re-reading the configuration files.

* Add new API calls that allow the cache to be inquired easily and
  efficiently.  Rewrite other functions like `git_config_int()` to be
  cache-aware.

* Rewrite callers to use the new API wherever possible.

You will need to consider how to handle other config API entry points
like `git_config_early()` and `git_config_from_file()`, as well as how
to invalidate the cache correctly in the case that the configuration
is changed while `git` is executing.

See
[this mailing list
thread](http://article.gmane.org/gmane.comp.version-control.git/242952)
for some discussion about this and related ideas.

 - Language: C
 - Difficulty: medium
 - Possible mentors: Michael Haggerty

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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: Unable to shrink repository size

[Elijah Newren]

On Wed, Mar 5, 2014 at 6:55 PM, Robert Dailey  wrote:
> What I'd like to do is somehow hunt down the largest commit (*not*
> blob) in the entire history of the repository to hopefully find out
> where huge directories have been checked in.
>
> I can't do a search for largest file (which most google results seem
> to show to do) since the culprit is really thousands of unnecessary
> files checked into a single subdirectory somewhere in history.
>
> Can anyone offer me some advice to help me reduce the size of my repo
> further? Thanks.

If you are trying to see which commits changed the most files (and/or
lines), you may find
   git log --oneline --shortstat
helpful.  A quick search through that output may help you decide where
to dig further.


Also, I'm sure someone else here probably has a better idea, but
here's a quick hack I came up with to look at "commit sizes":

mkdir tmp && for i in $(git rev-list --all); do git branch -f dummy $i
&& git bundle create tmp/$i.bundle dummy --not dummy^@; done

Follow that up with a quick "ls -alrSh tmp/" and you can see the
"size" of each commit.


Hope that helps,
Elijah
--
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

Unable to shrink repository size

[Robert Dailey]

I have a git-svn clone that I've been working on which is a full and
complete conversion of our SVN repository at work.

It started out as 1.4GB (git count-objects -v, looking at
'size-pack'). I have run the following script to clean up a directory
in the repo history that I suspect are huge (we had a third party
library checked in that, uncompressed, was about 1.2GB in size):

---
files=$@
echo "Removing: $files..."
git filter-branch --index-filter "git rm -rf --cached --ignore-unmatch
$files" -- --all

# remove the temporary history git-filter-branch otherwise leaves
behind for a long time
rm -rf .git/refs/original/ && git reflog expire --expire=now --all &&
git gc --aggressive --prune=now
---

Even though I seem to have removed it, the repository size (looking at
'size-pack' again) only went down about 200MB, so it's at 1.2GB now.
There is about 3-5 years of commit history in this repository.

What I'd like to do is somehow hunt down the largest commit (*not*
blob) in the entire history of the repository to hopefully find out
where huge directories have been checked in.

I can't do a search for largest file (which most google results seem
to show to do) since the culprit is really thousands of unnecessary
files checked into a single subdirectory somewhere in history.

Can anyone offer me some advice to help me reduce the size of my repo
further? 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] submodule : Add --no-separate-git-dir option to add and update command.

[Henri GEIST]

Le mercredi 05 mars 2014 à 19:13 +0100, Jens Lehmann a écrit :
> Am 03.03.2014 21:34, schrieb Henri GEIST:
> > Le lundi 03 mars 2014 à 17:45 +, Jens Lehmann a écrit :
> >> Am 03.03.2014 14:47, schrieb Henri GEIST:
> >>> This new option prevent git submodule  to clone the missing
> >>> submodules with the --separate-git-dir option.
> >>> Then the submodule will be regular repository and their gitdir will not
> >>> be placed in the superproject gitdir/modules directory.
> >>
> >> And what is your motivation for this? After all submodules containing
> >> a .git directory are second class citizens (because they can never be
> >> safely removed by regular git commands).
> >>
> > 
> > I recognize most people will prefer to have the .git directory separate.
> > And I do not intend to make this option the default.
> > 
> > My reasons are:
> > 
> >   - As it is not clearly stated in the doc that the gitdir is separate.
> > The first time I have copied one module to an USB key I had a big
> > surprise.
> 
> Oops! Could you please help us by hinting how the documentation
> could be improved here?
> 

Of course.
There is nothing in Documentation/git-submodule.txt to inform that submodules
clones are different from regular clones.
I will write and propose a patch for the documentation.
But maybe in a new thread.


> >   - This will not change anything for people not using it.
> 
> I do not agree, as they'll be seeing a new option and might use
> it to "go backward" as Junio explained in his answer.
> 
> >   - I use an other patch which I plane to send later which enable multiple
> > level of superproject to add a gitlink to the same submodule.
> > And in this case the superproject containing the separate gitdir will be
> > arbitrary and depend on the processing order of the
> > 'git submodule update --recursive' command.
> 
> I don't understand that. How is that different from using different
> names (and thus different separate gitdirs) for that duplicated
> submodule? After all, the .git directory is just moved somewhere
> else in the superproject's work tree, and as the name defaults to
> the path of the submodule ...
> 

I think I should give an example.
If I have a hierarchy like this :

superproject/submodule/subsubmodule

What I often do is:

superproject --> submodule --> subsubmodule
 | ^
 '-'

Where '-->' is a gitlink.


That mean .gitmodules and index of the superproject contain both submodule and
submodule/subsubmodule.
And also mean (and that is the point) subsubmodule is a direct 'child' of both
superproject and submodule.
In this case where should the separate gitdir of subsubmodule be placed ?
  - In superproject/modules/submodule/subsubmodule ?
  - In superproject/submodule/modules/subsubmodule ?
  - Depending on the 'git submodule update' command order ?
  - Or both ?


> >   - I have written this for myself and have using it since 2012 and send it 
> > in
> > the hope it could be useful for someone else even if it is only a few
> > people. But if its not the case no problem I will keep using it for 
> > myself.
> 
> Thanks.





signature.asc
Description: This is a digitally signed message part

Re: [PATCH v6 00/11] Add interpret-trailers builtin

[Ramsay Jones]

On 04/03/14 19:47, Christian Couder wrote:
> This patch series implements a new command:
> 
> git interpret-trailers
> 

[snip]

Minor problem: this series causes sparse to complain, thus:

trailer.c:642:6: warning: symbol 'process_trailers' was not \
declared. Should it be static?

The following patch, on top of the 'pu' branch, fixes it:

--- >8 ---
Subject: [PATCH] trailer.c: suppress sparse warning

Check that the public interface, as declared in the trailer.h header
file, is consistent with the actual implementation. Add an #include
of the header file into the implementation file.

Noticed by sparse ("'process_trailers'  was not declared. Should it
be static?").

Signed-off-by: Ramsay Jones 
---
 trailer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/trailer.c b/trailer.c
index b5de616..95d5874 100644
--- a/trailer.c
+++ b/trailer.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "run-command.h"
 #include "argv-array.h"
+#include "trailer.h"
 /*
  * Copyright (c) 2013 Christian Couder 
  */
-- 
1.9.0
--- 8< ---

However, for this to work, in addition to squashing the above patch into
your patch #6, you would need to move the creation of the trailer.h header
file from patch 07/11 ("trailer: add interpret-trailers command") to patch
06/11 ("trailer: put all the processing together and print"), where it should
have been anyway! :-D 

HTH

ATB,
Ramsay Jones

> Christian Couder (11):
>   Add data structures and basic functions for commit trailers
>   trailer: process trailers from stdin and arguments
>   trailer: read and process config information
>   trailer: process command line trailer arguments
>   trailer: parse trailers from stdin
>   trailer: put all the processing together and print
>   trailer: add interpret-trailers command
>   trailer: add tests for "git interpret-trailers"
>   trailer: execute command from 'trailer..command'
>   trailer: add tests for commands in config file
>   Documentation: add documentation for 'git interpret-trailers'
> 
>  .gitignore   |   1 +
>  Documentation/git-interpret-trailers.txt | 123 ++
>  Makefile |   2 +
>  builtin.h|   1 +
>  builtin/interpret-trailers.c |  33 ++
>  git.c|   1 +
>  t/t7513-interpret-trailers.sh| 261 
>  trailer.c| 661 
> +++
>  trailer.h|   6 +
>  9 files changed, 1089 insertions(+)
>  create mode 100644 Documentation/git-interpret-trailers.txt
>  create mode 100644 builtin/interpret-trailers.c
>  create mode 100755 t/t7513-interpret-trailers.sh
>  create mode 100644 trailer.c
>  create mode 100644 trailer.h
> 

--
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/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Henri GEIST]

Le mercredi 05 mars 2014 à 19:00 +0100, Jens Lehmann a écrit :
> Am 05.03.2014 00:01, schrieb Henri GEIST:
> > Permit to do a 'git clone --recursive' through git-gui.
> 
> I really like where this is heading!
> 
> Some minor issues:
> 
> - I think we should be more verbose in the commit message,
>   including that and why the default should be "on". Maybe
>   like this?
> 
>   "Permit to do a 'git clone --recursive' through git-gui.
>   Add a 'recursive' checkbox in the clone menu which allows
>   users to clone a repository and all its submodules in one
>   go (unless the 'update' flag is set to "none" in the
>   .gitmodules file for a submodule, in that case that
>   specific submodule is not cloned automatically).
> 
>   Enable this new option per default, as most users want to
>   clone all submodules too when cloning the superproject
>   (This is currently not possible without leaving git gui
>   or adding a custom tool entry for that)."
> 

Ok for me.

> 
> - I'd rather change the button text from "Recursive (For
>   submodules)" to something like "Recursively clone
>   submodules too" or such.
> 

Perfect

> 
> - Wouldn't it be easier to pass the '--recurse-submodules"
>   option to the "git clone" call for the superproject instead
>   of adding the _do_clone_submodules() function doing a
>   subsequent "git submodule update --init --recursive"? That
>   is also be more future proof with respect to the autoclone
>   config option we have in mind (which would add that behavior
>   for "git clone" itself, making the call you added redundant).
> 

That is what I planned to do at beginning.
But git-gui never call git clone anywhere.
It make the clone step by step with a long and complicated list of
commands just like a Tcl rewrite of git-clone.
Have a look on the function _do_clone2 in choose_repository.tcl.

As I suspect there should be a good reason for this that I did not
understand I have choose to not refactoring it.
And in fact looking in the code 'git clone --recursive' do nothing
else than calling 'git submodule update --init --recursive' like I
have done to complete this rewrite of 'git-clone'.


> 
> > Signed-off-by: Henri GEIST 
> > ---
> > I have set the default checkbox state to 'true' by default has all my gui 
> > users
> > use it all the time this way.
> > But as it change the default behavior you may prefer to set it to 'false' by
> > default.
> > 
> >  git-gui/lib/choose_repository.tcl |   34 --
> >  1 file changed, 32 insertions(+), 2 deletions(-)
> > 
> > diff --git a/git-gui/lib/choose_repository.tcl 
> > b/git-gui/lib/choose_repository.tcl
> > index 3c10bc6..47d436b 100644
> > --- a/git-gui/lib/choose_repository.tcl
> > +++ b/git-gui/lib/choose_repository.tcl
> > @@ -18,6 +18,7 @@ field local_path   {} ; # Where this repository is 
> > locally
> >  field origin_url   {} ; # Where we are cloning from
> >  field origin_name  origin ; # What we shall call 'origin'
> >  field clone_type hardlink ; # Type of clone to construct
> > +field recursive  true ; # Recursive cloning flag
> >  field readtree_err; # Error output from read-tree (if any)
> >  field sorted_recent   ; # recent repositories (sorted)
> >  
> > @@ -525,6 +526,11 @@ method _do_clone {} {
> > foreach r $w_types {
> > pack $r -anchor w
> > }
> > +   ${NS}::checkbutton $args.type_f.recursive \
> > +   -text [mc "Recursive (For submodules)"] \
> > +   -variable @recursive \
> > +   -onvalue true -offvalue false
> > +   pack $args.type_f.recursive
> > grid $args.type_l $args.type_f -sticky new
> >  
> > grid columnconfigure $args 1 -weight 1
> > @@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
> > fileevent $fd readable [cb _readtree_wait $fd]
> >  }
> >  
> > +method _do_validate_submodule_cloning {ok} {
> > +   if {$ok} {
> > +   $o_cons done $ok
> > +   set done 1
> > +   } else {
> > +   _clone_failed $this [mc "Cannot clone submodules."]
> > +   }
> > +}
> > +
> > +method _do_clone_submodules {} {
> > +   if {$recursive eq {true}} {
> > +   destroy $w_body
> > +   set o_cons [console::embed \
> > +   $w_body \
> > +   [mc "Cloning submodules"]]
> > +   pack $w_body -fill both -expand 1 -padx 10
> > +   $o_cons exec \
> > +   [list git submodule update --init --recursive] \
> > +   [cb _do_validate_submodule_cloning]
> > +   } else {
> > +   set done 1
> > +   }
> > +}
> > +
> >  method _readtree_wait {fd} {
> > set buf [read $fd]
> > $o_cons update_meter $buf
> > @@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
> > fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
> > fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
> > } else {
> > -   set done 1
> > +   _do_clone_submodules $this
> > }
> >  }
> >  
> 

[PATCH] git-prompt.sh: make '+' work for unborn branches

[Maurice Bos]

For unborn branches, it now compares the index against the empty tree.
(Just like git status does.)

Signed-off-by: Maurice Bos 
---
 contrib/completion/git-prompt.sh | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 7b732d2..f656838 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -407,12 +407,14 @@ __git_ps1 ()
if [ -n "${GIT_PS1_SHOWDIRTYSTATE-}" ] &&
   [ "$(git config --bool bash.showDirtyState)" != "false" ]
then
-   git diff --no-ext-diff --quiet --exit-code || w="*"
-   if [ -n "$short_sha" ]; then
-   git diff-index --cached --quiet HEAD -- || i="+"
-   else
+   local treeish=HEAD
+   if [ -z "$short_sha" ]; then
i="#"
+   # the empty tree
+   treeish=4b825dc642cb6eb9a060e54bf8d69288fbee4904
fi
+   git diff --no-ext-diff --quiet --exit-code || w="*"
+   git diff-index --cached --quiet $treeish -- || i="$i+"
fi
if [ -n "${GIT_PS1_SHOWSTASHSTATE-}" ] &&
   [ -r "$g/refs/stash" ]; then
-- 
1.8.5.rc0.23.gaa27064

--
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 v6 08/11] trailer: add tests for "git interpret-trailers"

[Junio C Hamano]

Christian Couder  writes:

> +# We want one trailing space at the end of each line.
> +# Let's use sed to make sure that these spaces are not removed
> +# by any automatic tool.
> +test_expect_success 'setup 3' '
> + sed -e "s/ Z\$/ /" >complex_message_trailers <<-\EOF
> +Fixes: Z
> +Acked-by: Z
> +Reviewed-by: Z
> +Signed-off-by: Z
> +EOF
> +'

It is a bit disappointing to see that these are flushed to the left,
when the here-doc redirection uses "<<-\EOF" (not "<<\EOF") to allow
you to indent, i.e.

sed ... <<-\EOF
foo
bar
EOF

or even more readable:

sed ... <<-\EOF
foo
bar
EOF

no?
--
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 v6 09/11] trailer: execute command from 'trailer..command'

[Junio C Hamano]

Christian Couder  writes:

> diff --git a/trailer.c b/trailer.c
> index ab93c16..67e7baf 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -490,12 +544,22 @@ static struct trailer_item 
> *process_command_line_args(int argc, const char **arg
> ...
> + /* Add conf commands that don't use $ARG */
> + for (item = first_conf_item; item; item = item->next) {
> + if (item->conf.command && !item->conf.command_uses_arg)
> + {

Opening brace of a block sits on the same line as the closing
condition of the control.  I.e.

if (...) {
--
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 v6 05/11] trailer: parse trailers from stdin

[Junio C Hamano]

Christian Couder  writes:

> diff --git a/trailer.c b/trailer.c
> index 5d69c00..e0e066f 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -50,6 +50,13 @@ static size_t alnum_len(const char *buf, size_t len)
>   return len;
>  }
>  
> +static inline int contains_only_spaces(const char *str)
> +{
> + const char *s;
> + for (s = str; *s && isspace(*s); s++);

Have an empty statement on a separate line for readability.  I.e.

for (...)
; /* keep skipping */

> @@ -471,3 +478,72 @@ static struct trailer_item 
> *process_command_line_args(int argc, const char **arg
> ...
> +static void process_stdin(struct trailer_item **in_tok_first,
> +   struct trailer_item **in_tok_last)
> +{
> ...
> + /* Print non trailer lines as is */
> + for (i = 0; lines[i] && i < start; i++) {
> + printf("%s", lines[i]->buf);
> + }

Needless brace pair.
--
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 v6 04/11] trailer: process command line trailer arguments

[Junio C Hamano]

Christian Couder  writes:

> diff --git a/trailer.c b/trailer.c
> index 5b8e28b..5d69c00 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -378,3 +378,96 @@ static int git_trailer_config(const char *conf_key, 
> const char *value, void *cb)
> ...
> +static struct trailer_item *new_trailer_item(struct trailer_item *conf_item,
> +  char* tok, char* val)

Asterisk sticks to the variable, not the type.

> +static struct trailer_item *create_trailer_item(const char *string)
> +{
> ...
> + return new_trailer_item(NULL, strbuf_detach(&tok, NULL), 
> strbuf_detach(&val, NULL));;

Overlong line.  Perhaps that helped you to miss the double-semicolon
at the end.
--
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 v6 02/11] trailer: process trailers from stdin and arguments

[Junio C Hamano]

Christian Couder  writes:

> Implement the logic to process trailers from stdin and arguments.
>
> At the beginning trailers from stdin are in their own in_tok
> doubly linked list, and trailers from arguments are in their own
> arg_tok doubly linked list.
>
> The lists are traversed and when an arg_tok should be "applied",
> it is removed from its list and inserted into the in_tok list.
>
> Signed-off-by: Christian Couder 
> ---

Thanks.

This round is marked as the sixth, but I still see quite a many
style issues, which I expect not to see from long timers without
being told.  Somewhat disappointing...

>  trailer.c | 197 
> ++
>  1 file changed, 197 insertions(+)
>
> diff --git a/trailer.c b/trailer.c
> index db93a63..a0124f2 100644
> --- a/trailer.c
> +++ b/trailer.c
> @@ -47,3 +47,200 @@ static size_t alnum_len(const char *buf, size_t len)
> ...
> + if ((where == WHERE_AFTER ? in_tok->next : 
> in_tok->previous) == arg_tok)

Overlong line that does not have to be.

if ((where == WHERE_AFTER
 ? in_tok->next : in_tok->previous) == arg_tok)

or something?

> +static void update_last(struct trailer_item **last)
> +{
> + if (*last)
> + while((*last)->next != NULL)

Style.  SP between control keyword and the expression.

> + *last = (*last)->next;
> +}
> +
> +static void update_first(struct trailer_item **first)
> +{
> + if (*first)
> + while((*first)->previous != NULL)

Ditto.

> +static void process_trailers_lists(struct trailer_item **in_tok_first,
> ...
> + /* Process input from end to start */
> + for (in_tok = *in_tok_last; in_tok; in_tok = in_tok->previous) {
> + process_input_token(in_tok, arg_tok_first, WHERE_AFTER);
> + }

Needless brace pair.

> + /* Process input from start to end */
> + for (in_tok = *in_tok_first; in_tok; in_tok = in_tok->next) {
> + process_input_token(in_tok, arg_tok_first, WHERE_BEFORE);
> + }

Ditto.
--
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: difftool sends malformed path to exernal tool on Windows

[Paul Lotz]

David,

We did succeed in getting a script to work.  The local Git guru started with 
your script (he independently sent me something very similar) and added some 
tricks to make things work.

The contents of the shell script ended up as:
___
#!/bin/bash

# Method to determine absolute path
# The -W option on the pwd command is necessary to return the Windows version 
of the path.
# Leaving off the -W option will result in a conversion of temp directory to a 
Linux-specific 'tmp' path.
# Piping the result through tr '/' '\\' translates the forward slashes to 
backslashes.
# Windows understands forward slashes, but LVCompare.exe does not.
abspath () {
(
DIR=$(dirname "$1")
FN=$(basename "$1")
cd "$DIR"
printf "%s/%s" "$(pwd -W)" "$FN" | tr '/' '\\'
)
}

lvcompare="C:\\Program Files (x86)\National Instruments\\Shared\\LabVIEW 
Compare\\LVCompare.exe"
local=$(abspath "$1")
remote=$(abspath "$2")
exec "$lvcompare" -nobdpos -nofppos "$local" "$remote"
# For the options, see 
http://zone.ni.com/reference/en-XX/help/371361H-01/lvhowto/configlvcomp_thirdparty/.
___

This works, but the solution seems to me nontrivial.

I will post this solution in Git with LabVIEW threads tomorrow, but I thought 
I'd see if you have any suggestions before I do that.

Thanks much!

Paul

-Original Message-
From: David Aguilar [mailto:dav...@gmail.com] 
Sent: Wednesday, March 5, 2014 1:25 AM
To: Paul Lotz
Cc: 'Git Mailing List'
Subject: Re: difftool sends malformed path to exernal tool on Windows

On Mon, Mar 03, 2014 at 04:24:15PM -0700, Paul Lotz wrote:
> David,
> 
> OK, I did as you suggested, and the results were revealing.
> 
> First, I replaced "echo" with "cat".  Result: The contents of both files 
> appeared in the Git Bash Window.
> 
> Then I tried calling LVCompare from the Git Bash and Windows Command Prompt 
> windows with variations on the paths.
> 
> Here are the most relevant results:
> First from the Windows Command Prompt:
> 1) This command works:
> C:\LSST_TS\SystemSW\M2AADT>"C:\Program Files (x86)\National 
> Instruments\Shared\L abVIEW Compare\LVCompare.exe" 
> C:\Users\Paul\AppData\Local\Temp\Typedefs_TestStat
> us_Before.ctl C:\LSST_TS\SystemSW\M2AADT\Typedefs\TestStatus.ctl
> [General note:
> I saved a copy of the temp file and replaced the hex string with the 
> string 'Before' to make the file stick around.  The paths are 
> otherwise the same.]

This is aligns with: 
http://zone.ni.com/reference/en-XX/help/371361H-01/lvhowto/configlvcomp_thirdparty/

"lvcompare.exe  ..."

The key thing is the mention of absolute paths.

What is happening is that lvcompare.exe (or likely it's a Windows thing) 
changes its current directory to its installation directory under Progra~1.

That means the relative paths passed in by difftool won't be found.

The way to fix it is to redirect your difftool config to a script that makes 
all paths absolute.  This script can then call the real lvcompare.exe.

You just need to tweak the lvcompare part in your .gitconfig to look like this:

[difftool "lvcompare"]
cmd = ~/bin/lvcompare.sh \"$LOCAL\" \"$REMOTE\"


... and install an executable lvcompare.sh shell script in in your $HOME/bin.  
Something like this:

#!/bin/sh

abspath () {
(
cd "$(dirname "$1")" &&
printf "%s/%s" "$(pwd)" "$(basename "$1")"
)
}

lvcompare="C:\\Program Files (x86)\National Instruments\\Shared\\LabVIEW 
Compare\\LVCompare.exe"
local=$(abspath "$1")
remote=$(abspath "$2")
exec "$lvcompare" "$local" "$remote"

> 2) C:\LSST_TS\SystemSW\M2AADT>"C:\Program Files (x86)\National 
> Instruments\Shared\L abVIEW Compare\LVCompare.exe" 
> C:\Users\Paul\AppData\Local\Temp\Typedefs_TestStat
> us_Before.ctl Typedefs\TestStatus.ctl
> 
> Result: Error message with reference to C:\Program Files 
> (x86)\National Instruments\Shared\L abVIEW 
> Compare\supportVIs\_prolvcmp.llb\Typedefs\TestStatus.ctl
> 
> Observation: The second path has to be the full path, not the relative path 
> we get back using "echo".

Yes, that's what it looks like.
--
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 v4] commit.c: use skip_prefix() instead of starts_with()

[Junio C Hamano]

tanay abhra  writes:

> On Wed, Mar 5, 2014 at 3:41 AM, Junio C Hamano  wrote:
>
>> Junio C Hamano  writes:
>>
>> >> +found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
>> >> +if(!found) {
>>
>> Missing SP between the control keyword and parenthesized expression
>> the keyword uses.
>>
>> I've fixed this (and the broken indentation) locally and queued the
>> result to 'pu', so no need to resend to correct this one.
>>
>> Thanks.
>>
>>
> Sorry about the indentation, it was due to not setting the tab to eight
> spaces. I found your mail late, so I had already
> sent a revised patch [1]. Please ignore that mail.
>
> Also , what should be my next step ,should I present a rough draft of a
> proposal , or tackle other bugs on the mailing list?

As far as I, as the maintainer of the project, am concerned [*1*],
we are done and I expect/require nothing more from you on this
change.

The MicroProject page says:

... If you've already done a microproject and are itching to do
more, then get involved in other ways, like finding and fixing
other problems in the code, or improving the documentation or
code comments, or helping to review other people's patches on
the mailing list, or answering questions on the mailing list or
in IRC, or writing new tests, etc., etc. In short, start doing
things that other Git developers do!

FYI, [GSoC 2014 timeline] (Google for it) tells us that would-be
students are supposed to be discussing project ideas with their
mentoring organizations now, in preparation for the actual student
application that begins on Mar 10 and ends on Mar 21.


[Footnote]

*1* I should mention that I am not involved in GSoC administration
and student selection for the Git project.
--
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 1/2] i18n: proposed command missing leading dash

[Junio C Hamano]

Jiang Xin  writes:

> 2014-03-05 2:40 GMT+08:00 Junio C Hamano :
>> From: Sandy Carter 
>> Date: Mon,  3 Mar 2014 09:55:53 -0500
>>
>> Add missing leading dash to proposed commands in french output when
>> using the command:
>> git branch --set-upstream remotename/branchname
>> and when upstream is gone
>>
>> Signed-off-by: Sandy Carter 
>> Signed-off-by: Junio C Hamano 
>> ---
>>
>>  * Forwarding to the i18n coordinator.  I don't do French, but this
>>seems trivially correct.
>
> Applied to maint branch of git://github.com/git-l10n/git-po, and can be
> merged to master directly.

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: [BUG] Halt during fetch on MacOS

[Conley Owens]

On Fri, Feb 28, 2014 at 3:26 PM, Conley Owens  wrote:
> $ git --version  # This is just the git from MacPorts
> git version 1.8.5.5
> $ sw_vers
> ProductName:Mac OS X
> ProductVersion: 10.8.5
> BuildVersion:   12F45

OK, I've tried using my own build from master, and I still get the same results.

I've done a little more investigation and discovered it always hangs at:
`atexit(notify_parent);` in `run-command.c:start_command`
when running:
trace: run_command: 'git-remote-https' 'aosp'
'https://android.googlesource.com/platform/external/tinyxml2'

Could this have to do with the atexit implementation?  (eg. limit on
the number of functions that can be registered, etc)

$ cc -v
Apple clang version 4.1 (tags/Apple/clang-421.11.66) (based on LLVM 3.1svn)
Target: x86_64-apple-darwin12.5.0
Thread model: posix

>
> test.sh
> "
> #!/bin/bash
> rungit() {
> mkdir $1
> GIT_DIR=$1 git init --bare
> echo '[remote "aosp"]' > $1/config
> echo 'url =
> https://android.googlesource.com/platform/external/tinyxml2' >>
> $1/config
> GIT_DIR=$1 git fetch aosp +refs/heads/master:refs/remotes/aosp/master
> rm -rf $1
> }
>
> for i in $(seq 1 100)
> do
> rungit testdir$i &
> done
> """
> $ ./test.sh  # Warning! This script fetches ~40MB of data
>
> When everything cools, you can see that there are some fetches hanging
> (typically).
> $ ps | grep 'git fetch'
> ...
> 63310 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63314 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63319 ttys0040:00.01 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63407 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63414 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> 63420 ttys0040:00.00 git fetch aosp
> +refs/heads/master:refs/remotes/aosp/master
> ...
>
> You can look at the parent process of each and see that one half
> spawned the other half, or you can look at the environment variables
> for each to see that there are two processes operating in the same
> directory for each directory where there's an issue.
> $ echo "$(for pid in $(ps | grep 'git fetch' | grep -o '^[0-9]*'); do
> ps -p $pid -wwwE | grep 'GIT_DIR=[^ ]*' -o; done)" | sort
> GIT_DIR=testdir14
> GIT_DIR=testdir14
> GIT_DIR=testdir32
> GIT_DIR=testdir32
> GIT_DIR=testdir47
> GIT_DIR=testdir47
>
> I've searched through the mailing list, but this doesn't seem to be a
> known issue.  I've only seen this occur on macs (and with a good deal
> of regularity).  It doesn't occur on my Ubuntu box.
>
> ~cco3
--
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 v3] Replace memcpy with hashcpy when dealing hash copy globally

[Junio C Hamano]

Sun He  writes:

> Replacing memcpy with hashcpy is more directly and elegant.

Can we justify the change without being subjective?

> Leave ppc/sha1.c alone, as it is an isolated component.
> Pull cache.h(actually ../cache.h) in just for one memcpy
> there is not proper.

That is not the reason why that memcpy of 20-byte must stay as it
is.  If for whatever reason we later choose to switch to using
another hash function, say MD5, to come up with the object names,
the majority of memcpy(..., 20) must change to copy 16 bytes, and it
makes sense to contain that implementation-specific knowledge of
twenty behind the hashcpy() abstraction.  The 20-byte memcpy() call
in ppc/sha1.c, however, is an implementation of *THE* SHA-1
algorithm, whose output is and will always be 20-byte.  It will not
change when we decide to replace what hash function is used to name
our objects (which would result in updating the implementation of
hashcpy()).  That is the reason why you shouldn't touch that one.
It has to be explicitly 20 byte, without ever getting affected by
what length our hashcpy() may choose to copy.

Perhaps...

We invented hashcpy() to keep the abstraction of "object
name" behind it.  Use it instead of calling memcpy() with
hard-coded 20-byte length when moving object names between
pieces of memory.

Leave ppc/sha1.c as-is, because the function *is* about
*the* SHA-1 hash algorithm whose output is and will always
be 20-byte.

or something.

> Find the potential places with memcpy by the bash command:
>  $ find . | xargs grep "memcpy.*\(.*20.*\)"

If you are planning to hack on git, learn how to use it first ;-)

$ git grep 'memcpy.*, 20)'
--
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: fnmatch vs regex

[Jacopo Notarstefano]

On Wed, Mar 5, 2014 at 8:55 PM, Vincenzo di Cicco  wrote:
> Hi there, I'm NaN.
> Recently I enrolled to this mailing list thanks to the GSoC.
> I've looked the Ideas Page but -unfortunately- some projects are very
> difficult for me.

Hi Vincenzo!

I also got interested in contributing to Git because of GSoC. I
encourage you to try doing a microproject: the coding part can be
*really* trivial. For example, mine just involved deleting lines
(after, of course, explaining why they weren't needed anymore). If you
are having trouble following the instructions in
Documentation/SubmittingPatches I recommend this video:
https://www.youtube.com/watch?v=LLBrBBImJt4.
--
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: fnmatch vs regex

[Junio C Hamano]

Vincenzo di Cicco  writes:

> But: why the decision to support the Blob Pattern instead of the
> Regular Expressions?

s/Blob/glob/;

Matching pathnames using fnmatch/glob is a fine UNIX tradition;
because we generally consider refnames also as "pathname-like"
things, we use fnmatch for them, too (what we actually use is
wildmatch, which can be thought of as a natural evolution of it).
--
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: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

[Junio C Hamano]

Andrew Wong  writes:

> ... merge hints in the future as well.

I actually wish we did not have to add any hints in the first place.

> Having one advice config/variable
> for every single situation seems a bit overkill, and we would end up
> with too many variables.

That goes without saying.
--
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 0/3] push: detect local refspec errors early

[Jeff King]

On Wed, Mar 05, 2014 at 12:51:06PM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > We can't fully process the refspecs until we have talked to the other
> > side, because they may involve matching refs from the remote; I don't
> > think git even really looks at them until after we've connected.
> >
> > But I think there are some obvious cases, like a bogus left-hand side
> > (i.e., what you have here) that cannot ever succeed, no matter what the
> > other side has. We could sanity check the refspecs before doing anything
> > else.
> 
> The user's wallclock time is more important than machine cycles,
> checking things we could check before having the user do things is a
> good principle to follow.
> 
> I wish that the solution did not have to involve doing the same
> computation twice, but I do not think there is a clean way around
> that in this codepath.

Yeah, there are two inefficiencies here:

  1. We parse the refspecs twice. In theory we could parse them once,
 feed the result to check_push_refspecs, then again to
 match_push_refspecs. That wouldn't be too hard a refactor.

  2. We match the "src" side to local refs twice. Getting rid of this
 would involve splitting match_push_refs into two (analyzing the
 "src" half and the "dst" half), somehow storing the intermediate
 the two calls, and only contacting the remote after the first step
 is done. This is probably trickier.

I'd be happy if somebody wanted to do those cleanups on top, but I don't
personally have an interest in spending time on them. The amount of
duplicated computation we're talking about here is not very much (and
the number of refspecs tends to be small, anyway).

-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: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

[Andrew Wong]

On Wed, Mar 5, 2014 at 1:29 PM, Junio C Hamano  wrote:
> If the user said "git merge" while another "git merge" is still
> outstanding, we would want to say "You have not concluded your
> previous merge" and die, and you presumably want to add the same
> "how to abort" message there.  Such a codepath is unlikely to be
> covered by existing advice.resolveConflict, and it sounds more
> natural, at least to me, to use a separate variable to squelch only
> the new "how to abort" part.

Alright, I don't mind using a separate variable. Though I think it'd
be good to keep the variable name slightly more general though, like
"advice.mergeHints", so that we could use the variable to cover other
merge hints in the future as well. Having one advice config/variable
for every single situation seems a bit overkill, and we would end up
with too many variables.
--
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 0/3] push: detect local refspec errors early

[Junio C Hamano]

Jeff King  writes:

> We can't fully process the refspecs until we have talked to the other
> side, because they may involve matching refs from the remote; I don't
> think git even really looks at them until after we've connected.
>
> But I think there are some obvious cases, like a bogus left-hand side
> (i.e., what you have here) that cannot ever succeed, no matter what the
> other side has. We could sanity check the refspecs before doing anything
> else.

The user's wallclock time is more important than machine cycles,
checking things we could check before having the user do things is a
good principle to follow.

I wish that the solution did not have to involve doing the same
computation twice, but I do not think there is a clean way around
that in this codepath.

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: [RFC/PATCH] diff: simplify cpp funcname regex

[Junio C Hamano]

Jeff King  writes:

> types, we simply look for an identifier at the start of the
> line that contains a "(", meaning it is either a function
> definition or a function call, and then not containing ";"
> which would indicate it is a call or declaration.

It is not worth worrying about:

foo(arg,
another);

that is not indented, so I think that simplicity is good.

> For example, for top-level changes
> outside functions, we might find:
>
>   N_("some text that is long"
>
> that is part of:
>
>   const char *foo =
>   N_("some text that is long"
>   "and spans multiple lines");

Unfortunate, but cannot be avoided.

>
> Before this change, we would skip past it (using the cpp regex, that is;
> the default one tends to find the same line) and either report nothing,
> or whatever random function was before us. So it's a behavior change,
> but the existing behavior is really no better.

True.
--
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] disable grafts during fetch/push/bundle

[Junio C Hamano]

Jeff King  writes:

> On Wed, Mar 05, 2014 at 11:18:17AM -0800, Junio C Hamano wrote:
>
>> Given that we discourage "grafts" strongly and "replace" less so
>> (but still discourage it), telling the users that biting the bullet
>> and rewriting the history is _the_ permanent solution, I think it is
>> understandable why nobody has bothered to.
>
> Perhaps the patch below would help discourage grafts more?
>
> The notable place in the documentation where grafts are still used is
> git-filter-branch.txt.  But since the example there is about cementing
> rewritten history, it might be OK to leave.

Thanks; I agree with the reasoning.
--
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 v4 24/27] prune: strategies for linked checkouts

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

> + if (get_device_or_die(path) != get_device_or_die(get_git_dir())) {
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s/locked", sb_repo.buf);
> + write_file(sb.buf, 1, "located on a different file system\n");
> + keep_locked = 1;
> + } else {
> + strbuf_reset(&sb);
> + strbuf_addf(&sb, "%s/link", sb_repo.buf);
> + (void)link(sb_git.buf, sb.buf);
> + }

Just in case you did not realize, casting the return away with
(void) will not squelch this out of the compiler:

builtin/checkout.c: In function 'prepare_linked_checkout':
builtin/checkout.c:947:3: error: ignoring return value of 'link', declared 
with attribute warn_unused_result [-Werror=unused-result]

It still feels fishy to see "we attempt to link but we do not care
if it works or not" to me, with or without the "unused result"
issue.
--
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

fnmatch vs regex

[Vincenzo di Cicco]

Hi there, I'm NaN.
Recently I enrolled to this mailing list thanks to the GSoC.
I've looked the Ideas Page but -unfortunately- some projects are very
difficult for me.
I've looked the source code and I've seen that to perform a search
with a pattern to the branches list (and other commands) git uses
fnmatch() and so supports the glob pattern.
I haven't never massively used the branches or the tags to have the
necessity to filter in a particolar way the results, and the asterisk
has always worked very well.
But: why the decision to support the Blob Pattern instead of the
Regular Expressions?
With your experiences can a patch like this improve this side?

Thanks for the awesome work,
NaN
--
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] upload-pack: allow shallow fetching from a read-only repository

[Junio C Hamano]

Duy Nguyen  writes:

> If only there is a way to pass this info without a temporary
> file. Multiplexing it to pack-objects' stdin should work. It may be
> ugly, but it's probably the safest way.
>
> Wait it does not look that ugly. We can feed "--shallow " lines
> before sending want/have/edge objects. Something like this seems to
> work (just ran a few shallow-related tests, not the whole test suite)

Sounds like it is a much better approach to the issue.
--
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 v4 18/27] setup.c: support multi-checkout repo setup

[Junio C Hamano]

Nguyễn Thái Ngọc Duy   writes:

>  core.worktree::
>   Set the path to the root of the working tree.
> + If GIT_COMMON_DIR environment variable is set, core.worktree
> + is ignored and not used for determining the root of working tree.

Just thinking aloud to see if I got the full implication of the
above right...

If we find ourselves in the multi-checkout mode because we saw
.git/commondir on the filesystem, it is clear that the root of the
working tree is the parent directory of that .git directory.

If the reason we think we are in the multi-checkout mode is not
because of .git/commondir but because $GIT_COMMON_DIR is set, should
we assume the same relationship between the root of the working tree
and the GIT_DIR (however we find it) when the environment variable
$GIT_WORK_TREE is not set?  Or should that configuration be an error?
With $GIT_DIR set without $GIT_WORK_TREE set, the user is telling us
that the $cwd is the root of the working tree, so perhaps we should
do the same?

--
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] use strchrnul() in place of strchr() and strlen()

[Rohit Mani]

Avoid scanning strings twice, once with strchr() and then with
strlen(), by using strchrnul(). Update the conditional expressions
involving the return value of strchrnul() with a check for '\0'.

Signed-off-by: Rohit Mani 
---
I plan to apply for the GSoC.

 archive.c|4 ++--
 cache-tree.c |   16 +++-
 diff.c   |9 +++--
 fast-import.c|   35 ++-
 match-trees.c|   11 ---
 parse-options.c  |5 +
 pretty.c |5 ++---
 remote-testsvn.c |4 ++--
 ws.c |7 ++-
 9 files changed, 37 insertions(+), 59 deletions(-)

diff --git a/archive.c b/archive.c
index 346f3b2..d196215 100644
--- a/archive.c
+++ b/archive.c
@@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,
/* Remotes are only allowed to fetch actual refs */
if (remote) {
char *ref = NULL;
-   const char *colon = strchr(name, ':');
-   int refnamelen = colon ? colon - name : strlen(name);
+   const char *colon = strchrnul(name, ':');
+   int refnamelen = colon - name;
 
if (!dwim_ref(name, refnamelen, sha1, &ref))
die("no such ref: %.*s", refnamelen, name);
diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..21a13cf 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -121,11 +121,11 @@ void cache_tree_invalidate_path(struct cache_tree *it, 
const char *path)
 
if (!it)
return;
-   slash = strchr(path, '/');
+   slash = strchrnul(path, '/');
it->entry_count = -1;
-   if (!slash) {
+   if (*slash == '\0') {
int pos;
-   namelen = strlen(path);
+   namelen = slash - path;
pos = subtree_pos(it, path, namelen);
if (0 <= pos) {
cache_tree_free(&it->down[pos]->cache_tree);
@@ -554,9 +554,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
const char *slash;
struct cache_tree_sub *sub;
 
-   slash = strchr(path, '/');
-   if (!slash)
-   slash = path + strlen(path);
+   slash = strchrnul(path, '/');
/* between path and slash is the name of the
 * subtree to look for.
 */
@@ -564,10 +562,10 @@ static struct cache_tree *cache_tree_find(struct 
cache_tree *it, const char *pat
if (!sub)
return NULL;
it = sub->cache_tree;
-   if (slash)
-   while (*slash && *slash == '/')
+   if (*slash != '\0')
+   while (*slash != '\0' && *slash == '/')
slash++;
-   if (!slash || !*slash)
+   if (*slash == '\0' || !*slash)
return it; /* prefix ended with slashes */
path = slash;
}
diff --git a/diff.c b/diff.c
index e800666..83a039b 100644
--- a/diff.c
+++ b/diff.c
@@ -3365,14 +3365,11 @@ static int opt_arg(const char *arg, int arg_short, 
const char *arg_long, int *va
if (c != '-')
return 0;
arg++;
-   eq = strchr(arg, '=');
-   if (eq)
-   len = eq - arg;
-   else
-   len = strlen(arg);
+   eq = strchrnul(arg, '=');
+   len = eq - arg;
if (!len || strncmp(arg, arg_long, len))
return 0;
-   if (eq) {
+   if (*eq != '\0') {
int n;
char *end;
if (!isdigit(*++eq))
diff --git a/fast-import.c b/fast-import.c
index 4fd18a3..449595d 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1485,14 +1485,12 @@ static int tree_content_set(
unsigned int i, n;
struct tree_entry *e;
 
-   slash1 = strchr(p, '/');
-   if (slash1)
-   n = slash1 - p;
-   else
-   n = strlen(p);
+   slash1 = strchrnul(p, '/');
+   n = slash1 - p;
+
if (!n)
die("Empty path component found in input");
-   if (!slash1 && !S_ISDIR(mode) && subtree)
+   if (*slash1 == '\0' && !S_ISDIR(mode) && subtree)
die("Non-directories cannot have subtrees");
 
if (!root->tree)
@@ -1501,7 +1499,7 @@ static int tree_content_set(
for (i = 0; i < t->entry_count; i++) {
e = t->entries[i];
if (e->name->str_len == n && !strncmp_icase(p, 
e->name->str_dat, n)) {
-   if (!slash1) {
+   if (*slash1 == '\0') {
if (!S_ISDIR(mode)
&& e->versions[1].mode == mode
&& 
!hashcmp(e->versions[1].sha1, sha1))
@@ -1552,7 +1550,7 @@ static int tree_content_set(
e->versions[0].mode = 0;
hash

Re: [PATCH] disable grafts during fetch/push/bundle

[Jeff King]

On Wed, Mar 05, 2014 at 11:18:17AM -0800, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote:
> >
> >> ... the plan, at least in my mind, has always been exactly that: grafts
> >> were a nice little attempt but is broken---if you really wanted to
> >> muck with the history without rewriting (which is still discouraged,
> >> by the way), do not use "graft", but use "replace".
> >
> > I certainly had in the back of my mind that grafts were a lesser form of
> > "replace", and that eventually we could get rid of the former. Perhaps
> > my question should have been: "why haven't we deprecated grafts yet?".
> 
> Given that we discourage "grafts" strongly and "replace" less so
> (but still discourage it), telling the users that biting the bullet
> and rewriting the history is _the_ permanent solution, I think it is
> understandable why nobody has bothered to.

Perhaps the patch below would help discourage grafts more?

The notable place in the documentation where grafts are still used is
git-filter-branch.txt.  But since the example there is about cementing
rewritten history, it might be OK to leave.

I used "outdated" below. We could also up the ante to "deprecated".

-- >8 --
Subject: [PATCH] docs: mark info/grafts as outdated

We should be encouraging people to use git-replace instead.

Signed-off-by: Jeff King 
---
 Documentation/gitrepository-layout.txt | 4 
 Documentation/glossary-content.txt | 4 
 2 files changed, 8 insertions(+)

diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index aa03882..17d2ea6 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -176,6 +176,10 @@ info/grafts::
per line describes a commit and its fake parents by
listing their 40-byte hexadecimal object names separated
by a space and terminated by a newline.
++
+Note that the grafts mechanism is outdated and can lead to problems
+transferring objects between repositories; see linkgit:git-replace[1]
+for a more flexible and robust system to do the same thing.
 
 info/exclude::
This file, by convention among Porcelains, stores the
diff --git a/Documentation/glossary-content.txt 
b/Documentation/glossary-content.txt
index 378306f..be0858c 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -176,6 +176,10 @@ current branch integrates with) obviously do not work, as 
there is no
you can make Git pretend the set of <> a 
<> has
is different from what was recorded when the commit was
created. Configured via the `.git/info/grafts` file.
++
+Note that the grafts mechanism is outdated and can lead to problems
+transferring objects between repositories; see linkgit:git-replace[1]
+for a more flexible and robust system to do the same thing.
 
 [[def_hash]]hash::
In Git's context, synonym for <>.
-- 
1.8.5.2.500.g8060133

--
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] disable grafts during fetch/push/bundle

[Junio C Hamano]

Jeff King  writes:

> On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote:
>
>> ... the plan, at least in my mind, has always been exactly that: grafts
>> were a nice little attempt but is broken---if you really wanted to
>> muck with the history without rewriting (which is still discouraged,
>> by the way), do not use "graft", but use "replace".
>
> I certainly had in the back of my mind that grafts were a lesser form of
> "replace", and that eventually we could get rid of the former. Perhaps
> my question should have been: "why haven't we deprecated grafts yet?".

Given that we discourage "grafts" strongly and "replace" less so
(but still discourage it), telling the users that biting the bullet
and rewriting the history is _the_ permanent solution, I think it is
understandable why nobody has bothered to.

--
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: Negation in refspecs

[Jeff King]

On Wed, Mar 05, 2014 at 10:06:26AM -0800, Mickey Killianey wrote:

> Is there any syntax to support partial negations of refspecs, such as:
> 
> +refs/heads/*:refs/remotes/origin/*
> !refs/heads/dont-pull:
> !:refs/remotes/origin/dont-push
> 
> If not now, is negation something that might be possible/reasonable in
> a future version of Git, or is it difficult/unlikely to change?

No, it doesn't exist now. It's something that people have asked for
occasionally, but I don't think anybody is actively working on it.

I posted a rough patch here:

  http://thread.gmane.org/gmane.comp.version-control.git/240997/focus=241057

about a month ago, but I do not have any immediate plans to take it
further (that email details some of the issues). If you're interested in
working on it, I think people are receptive to the idea; it just needs a
clean implementation.

-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: New directory lost by git am

[Junio C Hamano]

Jeff King  writes:

> But I have not thought hard about it, so maybe there is a good reason
> not to (it is a little weird just because the resulting index is a
> partial application of the patch).

Originally ".rej" was a deliberate attempt to be "not very Git but
more like 'patch'", so I wouldn't be surprised if the combination
between "--reject" and "--index" did not work, in the sense that we
did not add such a partial change to the index.

I do not offhand think of a reason to forbid the combination,
though, as long as we make sure that "git apply --index --reject"
still exits with failure to prevent a partial application to be
auto-committed.
--
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] push: detect local refspec errors early

[Jeff King]

When pushing, we do not even look at our push refspecs until
after we have made contact with the remote receive-pack and
gotten its list of refs. This means that we may go to some
work, including asking the user to log in, before realizing
we have simple errors like "git push origin matser".

We cannot catch all refspec problems, since fully evaluating
the refspecs requires knowing what the remote side has. But
we can do a quick sanity check of the local side and catch a
few simple error cases.

Signed-off-by: Jeff King 
---
 remote.c   | 25 +
 remote.h   |  1 +
 t/t5529-push-errors.sh | 48 
 transport.c|  8 ++--
 4 files changed, 80 insertions(+), 2 deletions(-)
 create mode 100755 t/t5529-push-errors.sh

diff --git a/remote.c b/remote.c
index b6586c0..8471fb1 100644
--- a/remote.c
+++ b/remote.c
@@ -1374,6 +1374,31 @@ static void prepare_ref_index(struct string_list 
*ref_index, struct ref *ref)
 }
 
 /*
+ * Given only the set of local refs, sanity-check the set of push
+ * refspecs. We can't catch all errors that match_push_refs would,
+ * but we can catch some errors early before even talking to the
+ * remote side.
+ */
+int check_push_refs(struct ref *src, int nr_refspec, const char 
**refspec_names)
+{
+   struct refspec *refspec = parse_push_refspec(nr_refspec, refspec_names);
+   int ret = 0;
+   int i;
+
+   for (i = 0; i < nr_refspec; i++) {
+   struct refspec *rs = refspec + i;
+
+   if (rs->pattern || rs->matching)
+   continue;
+
+   ret |= match_explicit_lhs(src, rs, NULL, NULL);
+   }
+
+   free_refspec(nr_refspec, refspec);
+   return ret;
+}
+
+/*
  * Given the set of refs the local repository has, the set of refs the
  * remote repository has, and the refspec used for push, determine
  * what remote refs we will update and with what value by setting
diff --git a/remote.h b/remote.h
index fb7647f..917d383 100644
--- a/remote.h
+++ b/remote.h
@@ -166,6 +166,7 @@ extern int query_refspecs(struct refspec *specs, int nr, 
struct refspec *query);
 char *apply_refspecs(struct refspec *refspecs, int nr_refspec,
 const char *name);
 
+int check_push_refs(struct ref *src, int nr_refspec, const char **refspec);
 int match_push_refs(struct ref *src, struct ref **dst,
int nr_refspec, const char **refspec, int all);
 void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
diff --git a/t/t5529-push-errors.sh b/t/t5529-push-errors.sh
new file mode 100755
index 000..9871307
--- /dev/null
+++ b/t/t5529-push-errors.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='detect some push errors early (before contacting remote)'
+. ./test-lib.sh
+
+test_expect_success 'setup commits' '
+   test_commit one
+'
+
+test_expect_success 'setup remote' '
+   git init --bare remote.git &&
+   git remote add origin remote.git
+'
+
+test_expect_success 'setup fake receive-pack' '
+   FAKE_RP_ROOT=$(pwd) &&
+   export FAKE_RP_ROOT &&
+   write_script fake-rp <<-\EOF &&
+   echo yes >"$FAKE_RP_ROOT"/rp-ran
+   exit 1
+   EOF
+   git config remote.origin.receivepack "\"\$FAKE_RP_ROOT/fake-rp\""
+'
+
+test_expect_success 'detect missing branches early' '
+   echo no >rp-ran &&
+   echo no >expect &&
+   test_must_fail git push origin missing &&
+   test_cmp expect rp-ran
+'
+
+test_expect_success 'detect missing sha1 expressions early' '
+   echo no >rp-ran &&
+   echo no >expect &&
+   test_must_fail git push origin master~2:master &&
+   test_cmp expect rp-ran
+'
+
+test_expect_success 'detect ambiguous refs early' '
+   git branch foo &&
+   git tag foo &&
+   echo no >rp-ran &&
+   echo no >expect &&
+   test_must_fail git push origin foo &&
+   test_cmp expect rp-ran
+'
+
+test_done
diff --git a/transport.c b/transport.c
index ca7bb44..325f03e 100644
--- a/transport.c
+++ b/transport.c
@@ -1132,8 +1132,7 @@ int transport_push(struct transport *transport,
 
return transport->push(transport, refspec_nr, refspec, flags);
} else if (transport->push_refs) {
-   struct ref *remote_refs =
-   transport->get_refs_list(transport, 1);
+   struct ref *remote_refs;
struct ref *local_refs = get_local_heads();
int match_flags = MATCH_REFS_NONE;
int verbose = (transport->verbose > 0);
@@ -1142,6 +1141,11 @@ int transport_push(struct transport *transport,
int pretend = flags & TRANSPORT_PUSH_DRY_RUN;
int push_ret, ret, err;
 
+   if (check_push_refs(local_refs, refspec_nr, refspec) < 0)
+   return -1;
+
+   remote_refs = transport->get_refs_list(transport, 1);
+
if (flags & TRANSPORT_PUSH

[PATCH 2/3] match_explicit_lhs: allow a "verify only" mode

[Jeff King]

The match_explicit_lhs function has all of the logic
necessary to verify the refspecs without actually doing any
work. This patch lets callers pass a NULL "match" pointer to
indicate they want a "verify only" operation.

For the most part, we just need to avoid writing to the NULL
pointer. However, we also have to refactor the
try_explicit_object_name sub-function; it indicates success by
allocating and returning a new ref. Instead, we give it an
"out" parameter for the match and return a numeric status.

Signed-off-by: Jeff King 
---
 remote.c | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/remote.c b/remote.c
index 6aa9dd2..b6586c0 100644
--- a/remote.c
+++ b/remote.c
@@ -1031,11 +1031,13 @@ int count_refspec_match(const char *pattern,
}
}
if (!matched) {
-   *matched_ref = matched_weak;
+   if (matched_ref)
+   *matched_ref = matched_weak;
return weak_match;
}
else {
-   *matched_ref = matched;
+   if (matched_ref)
+   *matched_ref = matched;
return match;
}
 }
@@ -1055,18 +1057,25 @@ static struct ref *alloc_delete_ref(void)
return ref;
 }
 
-static struct ref *try_explicit_object_name(const char *name)
+static int try_explicit_object_name(const char *name,
+   struct ref **match)
 {
unsigned char sha1[20];
-   struct ref *ref;
 
-   if (!*name)
-   return alloc_delete_ref();
+   if (!*name) {
+   if (match)
+   *match = alloc_delete_ref();
+   return 0;
+   }
+
if (get_sha1(name, sha1))
-   return NULL;
-   ref = alloc_ref(name);
-   hashcpy(ref->new_sha1, sha1);
-   return ref;
+   return -1;
+
+   if (match) {
+   *match = alloc_ref(name);
+   hashcpy((*match)->new_sha1, sha1);
+   }
+   return 0;
 }
 
 static struct ref *make_linked_ref(const char *name, struct ref ***tail)
@@ -1103,17 +1112,18 @@ static int match_explicit_lhs(struct ref *src,
 {
switch (count_refspec_match(rs->src, src, match)) {
case 1:
-   *allocated_match = 0;
+   if (allocated_match)
+   *allocated_match = 0;
return 0;
case 0:
/* The source could be in the get_sha1() format
 * not a reference name.  :refs/other is a
 * way to delete 'other' ref at the remote end.
 */
-   *match = try_explicit_object_name(rs->src);
-   if (!*match)
+   if (try_explicit_object_name(rs->src, match) < 0)
return error("src refspec %s does not match any.", 
rs->src);
-   *allocated_match = 1;
+   if (allocated_match)
+   *allocated_match = 1;
return 0;
default:
return error("src refspec %s matches more than one.", rs->src);
-- 
1.8.5.2.500.g8060133

--
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] match_explicit: hoist refspec lhs checks into their own function

[Jeff King]

In preparation for being able to check the left-hand side of
our push refspecs separately, this pulls the examination of
them out into its own function. There should be no behavior
change.

Signed-off-by: Jeff King 
---
 remote.c | 49 ++---
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/remote.c b/remote.c
index e41251e..6aa9dd2 100644
--- a/remote.c
+++ b/remote.c
@@ -1096,12 +1096,36 @@ static char *guess_ref(const char *name, struct ref 
*peer)
return strbuf_detach(&buf, NULL);
 }
 
+static int match_explicit_lhs(struct ref *src,
+ struct refspec *rs,
+ struct ref **match,
+ int *allocated_match)
+{
+   switch (count_refspec_match(rs->src, src, match)) {
+   case 1:
+   *allocated_match = 0;
+   return 0;
+   case 0:
+   /* The source could be in the get_sha1() format
+* not a reference name.  :refs/other is a
+* way to delete 'other' ref at the remote end.
+*/
+   *match = try_explicit_object_name(rs->src);
+   if (!*match)
+   return error("src refspec %s does not match any.", 
rs->src);
+   *allocated_match = 1;
+   return 0;
+   default:
+   return error("src refspec %s matches more than one.", rs->src);
+   }
+}
+
 static int match_explicit(struct ref *src, struct ref *dst,
  struct ref ***dst_tail,
  struct refspec *rs)
 {
struct ref *matched_src, *matched_dst;
-   int copy_src;
+   int allocated_src;
 
const char *dst_value = rs->dst;
char *dst_guess;
@@ -1110,23 +1134,8 @@ static int match_explicit(struct ref *src, struct ref 
*dst,
return 0;
 
matched_src = matched_dst = NULL;
-   switch (count_refspec_match(rs->src, src, &matched_src)) {
-   case 1:
-   copy_src = 1;
-   break;
-   case 0:
-   /* The source could be in the get_sha1() format
-* not a reference name.  :refs/other is a
-* way to delete 'other' ref at the remote end.
-*/
-   matched_src = try_explicit_object_name(rs->src);
-   if (!matched_src)
-   return error("src refspec %s does not match any.", 
rs->src);
-   copy_src = 0;
-   break;
-   default:
-   return error("src refspec %s matches more than one.", rs->src);
-   }
+   if (match_explicit_lhs(src, rs, &matched_src, &allocated_src) < 0)
+   return -1;
 
if (!dst_value) {
unsigned char sha1[20];
@@ -1171,7 +1180,9 @@ static int match_explicit(struct ref *src, struct ref 
*dst,
return error("dst ref %s receives from more than one src.",
  matched_dst->name);
else {
-   matched_dst->peer_ref = copy_src ? copy_ref(matched_src) : 
matched_src;
+   matched_dst->peer_ref = allocated_src ?
+   matched_src :
+   copy_ref(matched_src);
matched_dst->force = rs->force;
}
return 0;
-- 
1.8.5.2.500.g8060133

--
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 0/3] push: detect local refspec errors early

[Jeff King]

On Wed, Mar 05, 2014 at 01:36:12PM +0400, Dmitry wrote:

> Here's my usecase. I have created a BranchWithVeryLongName and I want
> to have it pushed to origin. So I use this command with version
> 1.8.1.2:
> 
> git push origin BranchMistypedLongName
> 
> (note that I mistyped the branch name). The following happens:
> 1. git asks me for username and password
> 2. it authenticates on the origin server
> 3. it bails out with "error: sfc refspec BranchMistypedLongName does not 
> match any"
> 
> Can't git perhaps check that the branch exists before it asks me for
> credentials and just say there's no such branch?

We can't fully process the refspecs until we have talked to the other
side, because they may involve matching refs from the remote; I don't
think git even really looks at them until after we've connected.

But I think there are some obvious cases, like a bogus left-hand side
(i.e., what you have here) that cannot ever succeed, no matter what the
other side has. We could sanity check the refspecs before doing anything
else.

Here's a patch series that does that.

  [1/3]: match_explicit: hoist refspec lhs checks into their own function
  [2/3]: match_explicit_lhs: allow a "verify only" mode
  [3/3]: push: detect local refspec errors early

-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] disable grafts during fetch/push/bundle

[Jeff King]

On Wed, Mar 05, 2014 at 10:49:24AM -0800, Junio C Hamano wrote:

> > Perhaps the right response is "grafts are broken, use git-replace
> > instead". But then should we think about deprecating grafts?
> 
> I am sort of surprised to hear that question, actually ;-)
> 
> I didn't say that in the message you are responding to because I
> somehow thought that everybody has been in agreement with these two
> lines for a long while.  Ever since I suggested the "replace" as an
> alternative "grafts done right" and outlined how it should work to
> Christian while sitting next to him in one of the early GitTogether,
> the plan, at least in my mind, has always been exactly that: grafts
> were a nice little attempt but is broken---if you really wanted to
> muck with the history without rewriting (which is still discouraged,
> by the way), do not use "graft", but use "replace".

I certainly had in the back of my mind that grafts were a lesser form of
"replace", and that eventually we could get rid of the former. Perhaps
my question should have been: "why haven't we deprecated grafts yet?".

-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] disable grafts during fetch/push/bundle

[Junio C Hamano]

Jeff King  writes:

> On Tue, Mar 04, 2014 at 12:52:18PM -0800, Junio C Hamano wrote:
>
> I do not recall any past discussion on this topic, and searching the
> archive only shows people echoing what I said above. Is this something
> we've promised to work in the past?

The history lesson is coming solely from my recollection of a
discussion I and Linus had on the list back when we were doing the
original "graft" and thinking about the interaction between it and
the object/history transfer; especially the "only add new ones, but
hide the ones that the user wants to be hidden" part is something
suggested by Linus but we couldn't implement back then, IIRC.

> Perhaps the right response is "grafts are broken, use git-replace
> instead". But then should we think about deprecating grafts?

I am sort of surprised to hear that question, actually ;-)

I didn't say that in the message you are responding to because I
somehow thought that everybody has been in agreement with these two
lines for a long while.  Ever since I suggested the "replace" as an
alternative "grafts done right" and outlined how it should work to
Christian while sitting next to him in one of the early GitTogether,
the plan, at least in my mind, has always been exactly that: grafts
were a nice little attempt but is broken---if you really wanted to
muck with the history without rewriting (which is still discouraged,
by the way), do not use "graft", but use "replace".
--
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] cache_tree_find(): remove redundant checks

[Junio C Hamano]

David Kastrup  writes:

> Junio C Hamano  writes:
>
>> Michael Haggerty  writes:
>>
>>> while (*path) {
>>> -   const char *slash;
>>> struct cache_tree_sub *sub;
>>> +   const char *slash = strchr(path, '/');
>>>  
>>> -   slash = strchr(path, '/');
>>> if (!slash)
>>> slash = path + strlen(path);
>>
>> Isn't the above a strchrnul()?
>
> Yes.  I realized that previously, but since it's a GNU extension rather
> than part of the C standards, I discarded that idea.  Calling
>
> git grep strchrnul
>
> shows, however, that it _is_ used plentifully already.

Yes, we have a fallback definition in compat-util, I think.

> Still worth thinking about whether there is no better name than slash
> for something that indicated the end of the current path name segment.

end_of_path_component?  Sounds a bit too long ;-)
--
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: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

[Matthieu Moy]

Andrew Wong  writes:

> On Wed, Feb 26, 2014 at 3:38 PM, Jonathan Nieder  wrote:
>> Andrew Wong wrote:
>>
>>> --- a/builtin/merge.c
>>> +++ b/builtin/merge.c
>>> @@ -909,7 +909,8 @@ static int suggest_conflicts(int renormalizing)
>>> + "fix conflicts and then commit the result.\n"
>>> + "To abort the merge, use \"git merge --abort\".\n"));
[...]
> This means contexts are no longer only about "resolving conflict", so
> I was thinking of renaming advice.resolveConflict to something like
> advice.mergeHints.
>
> Any thoughts?

If the advice suggests merge --abort, then why not advice.abortMerge ?

But mergeHints makes sense to me also, as it would potentially encompass
more cases.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

[Junio C Hamano]

Andrew Wong  writes:

> On Wed, Feb 26, 2014 at 3:38 PM, Jonathan Nieder  wrote:
>> Andrew Wong wrote:
>>
>>> --- a/builtin/merge.c
>>> +++ b/builtin/merge.c
>>> @@ -909,7 +909,8 @@ static int suggest_conflicts(int renormalizing)
>>>   fclose(fp);
>>>   rerere(allow_rerere_auto);
>>>   printf(_("Automatic merge failed; "
>>> - "fix conflicts and then commit the result.\n"));
>>> + "fix conflicts and then commit the result.\n"
>>> + "To abort the merge, use \"git merge --abort\".\n"));
>>
>> Seems reasonable, but I worry about the command growing too noisy.
>>
>> Could this be guarded by an advice. setting?  (See advice.*
>> in git-config(1) for what I mean.)
>
> I was planning to use advice.resolveConflict, but as I went through
> merge.c, I noticed there could be a few other situations where we
> could print out the same message:
> 1. when prepare_to_commit() fails, due to hook error, editor error, or
> empty commit message
> 2. "git commit --no-commit"
>
> This means contexts are no longer only about "resolving conflict", so
> I was thinking of renaming advice.resolveConflict to something like
> advice.mergeHints.
>
> Any thoughts?

I have no strong opinion on the naming, other than that I doubt this
particular new "how to abort" message is worth the headache associated
with the "rename" which involves transition planning of deprecating
the old, supporting both for a while and then removing the old.

The existing message above in "suggest-conflicts" is about hinting
the user to first resolve the conflict before attempting to
continue, and that is perfectly in line with the existing use of
advice.resolveConfict in die_conflict() in git-pull that tells the
user there is an unresolved conflict.

On the other hand, the additional "how to abort" message does not
have to be limited to "you have conflicted paths in the index" case.

If the user said "git merge" while another "git merge" is still
outstanding, we would want to say "You have not concluded your
previous merge" and die, and you presumably want to add the same
"how to abort" message there.  Such a codepath is unlikely to be
covered by existing advice.resolveConflict, and it sounds more
natural, at least to me, to use a separate variable to squelch only
the new "how to abort" part.
--
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: New directory lost by git am

[Phillip Susi]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 3/5/2014 12:13 PM, Jeff King wrote:
> We apply the changes to "modified" and "new" to the working tree,
> but we do not stage anything in the index. I suspect this is
> because our invocation of "apply --index" (which is what is doing
> the real work with "--reject" here) bails before touching the
> index. In theory it should be able to update the index for files
> that applied cleanly and leave the other ones alone.

Yikes, that's really bad.

> But I have not thought hard about it, so maybe there is a good
> reason not to (it is a little weird just because the resulting
> index is a partial application of the patch).  The "am -3" path
> does what you want here, but it is much simpler: it knows it can
> represent the 3-way conflict in the index. So the index represents
> the complete state of the patch application at the end, including
> conflicts.

yes, but -3 fails if it can't find the parent blobs.



-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTF20HAAoJEI5FoCIzSKrwFfQIAJfPmplu7zskercvjnuZGCle
ccTzK0rYtrwQn/78Vrbc6kqcrQvbvtrqUMN4/ILJ5xeaO80Gzzz8pchBPNN8khhY
VBQiWUOrKzBH1vszveneva+gFUrLIWk2KI6T8lGTnYulvRVe38WRAwr/8qEClPX6
hUnYChM17WE+KTV39ezA6ww9ZAyOX7EHq87PJp5nVgBB4HkmkDmccfxYTFNB4FGg
PPqun8g0Fyytd+Qrsk2M5L6NsPUXi32wIt8EWcyPwU6QrQgKbuWK7QlVkcPPzecM
eHifKm8V1V0VKudm3S8jbaUDG2KnLIdMveXu/e9Hn7+YgDQh9zM1m7f+NVJDvjk=
=NAe9
-END PGP SIGNATURE-
--
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] submodule : Add --no-separate-git-dir option to add and update command.

[Jens Lehmann]

Am 03.03.2014 21:34, schrieb Henri GEIST:
> Le lundi 03 mars 2014 à 17:45 +, Jens Lehmann a écrit :
>> Am 03.03.2014 14:47, schrieb Henri GEIST:
>>> This new option prevent git submodule  to clone the missing
>>> submodules with the --separate-git-dir option.
>>> Then the submodule will be regular repository and their gitdir will not
>>> be placed in the superproject gitdir/modules directory.
>>
>> And what is your motivation for this? After all submodules containing
>> a .git directory are second class citizens (because they can never be
>> safely removed by regular git commands).
>>
> 
> I recognize most people will prefer to have the .git directory separate.
> And I do not intend to make this option the default.
> 
> My reasons are:
> 
>   - As it is not clearly stated in the doc that the gitdir is separate.
> The first time I have copied one module to an USB key I had a big
> surprise.

Oops! Could you please help us by hinting how the documentation
could be improved here?

>   - This will not change anything for people not using it.

I do not agree, as they'll be seeing a new option and might use
it to "go backward" as Junio explained in his answer.

>   - I use an other patch which I plane to send later which enable multiple
> level of superproject to add a gitlink to the same submodule.
> And in this case the superproject containing the separate gitdir will be
> arbitrary and depend on the processing order of the
> 'git submodule update --recursive' command.

I don't understand that. How is that different from using different
names (and thus different separate gitdirs) for that duplicated
submodule? After all, the .git directory is just moved somewhere
else in the superproject's work tree, and as the name defaults to
the path of the submodule ...

>   - I have written this for myself and have using it since 2012 and send it in
> the hope it could be useful for someone else even if it is only a few
> people. But if its not the case no problem I will keep using it for 
> myself.

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

Negation in refspecs

[Mickey Killianey]

Is there any syntax to support partial negations of refspecs, such as:

+refs/heads/*:refs/remotes/origin/*
!refs/heads/dont-pull:
!:refs/remotes/origin/dont-push

If not now, is negation something that might be possible/reasonable in
a future version of Git, or is it difficult/unlikely to change?

Mick
--
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/RFC] git-gui: Add a 'recursive' checkbox in the clone menu.

[Jens Lehmann]

Am 05.03.2014 00:01, schrieb Henri GEIST:
> Permit to do a 'git clone --recursive' through git-gui.

I really like where this is heading!

Some minor issues:

- I think we should be more verbose in the commit message,
  including that and why the default should be "on". Maybe
  like this?

  "Permit to do a 'git clone --recursive' through git-gui.
  Add a 'recursive' checkbox in the clone menu which allows
  users to clone a repository and all its submodules in one
  go (unless the 'update' flag is set to "none" in the
  .gitmodules file for a submodule, in that case that
  specific submodule is not cloned automatically).

  Enable this new option per default, as most users want to
  clone all submodules too when cloning the superproject
  (This is currently not possible without leaving git gui
  or adding a custom tool entry for that)."


- I'd rather change the button text from "Recursive (For
  submodules)" to something like "Recursively clone
  submodules too" or such.


- Wouldn't it be easier to pass the '--recurse-submodules"
  option to the "git clone" call for the superproject instead
  of adding the _do_clone_submodules() function doing a
  subsequent "git submodule update --init --recursive"? That
  is also be more future proof with respect to the autoclone
  config option we have in mind (which would add that behavior
  for "git clone" itself, making the call you added redundant).


> Signed-off-by: Henri GEIST 
> ---
> I have set the default checkbox state to 'true' by default has all my gui 
> users
> use it all the time this way.
> But as it change the default behavior you may prefer to set it to 'false' by
> default.
> 
>  git-gui/lib/choose_repository.tcl |   34 --
>  1 file changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/git-gui/lib/choose_repository.tcl 
> b/git-gui/lib/choose_repository.tcl
> index 3c10bc6..47d436b 100644
> --- a/git-gui/lib/choose_repository.tcl
> +++ b/git-gui/lib/choose_repository.tcl
> @@ -18,6 +18,7 @@ field local_path   {} ; # Where this repository is 
> locally
>  field origin_url   {} ; # Where we are cloning from
>  field origin_name  origin ; # What we shall call 'origin'
>  field clone_type hardlink ; # Type of clone to construct
> +field recursive  true ; # Recursive cloning flag
>  field readtree_err; # Error output from read-tree (if any)
>  field sorted_recent   ; # recent repositories (sorted)
>  
> @@ -525,6 +526,11 @@ method _do_clone {} {
>   foreach r $w_types {
>   pack $r -anchor w
>   }
> + ${NS}::checkbutton $args.type_f.recursive \
> + -text [mc "Recursive (For submodules)"] \
> + -variable @recursive \
> + -onvalue true -offvalue false
> + pack $args.type_f.recursive
>   grid $args.type_l $args.type_f -sticky new
>  
>   grid columnconfigure $args 1 -weight 1
> @@ -952,6 +958,30 @@ method _do_clone_checkout {HEAD} {
>   fileevent $fd readable [cb _readtree_wait $fd]
>  }
>  
> +method _do_validate_submodule_cloning {ok} {
> + if {$ok} {
> + $o_cons done $ok
> + set done 1
> + } else {
> + _clone_failed $this [mc "Cannot clone submodules."]
> + }
> +}
> +
> +method _do_clone_submodules {} {
> + if {$recursive eq {true}} {
> + destroy $w_body
> + set o_cons [console::embed \
> + $w_body \
> + [mc "Cloning submodules"]]
> + pack $w_body -fill both -expand 1 -padx 10
> + $o_cons exec \
> + [list git submodule update --init --recursive] \
> + [cb _do_validate_submodule_cloning]
> + } else {
> + set done 1
> + }
> +}
> +
>  method _readtree_wait {fd} {
>   set buf [read $fd]
>   $o_cons update_meter $buf
> @@ -982,7 +1012,7 @@ method _readtree_wait {fd} {
>   fconfigure $fd_ph -blocking 0 -translation binary -eofchar {}
>   fileevent $fd_ph readable [cb _postcheckout_wait $fd_ph]
>   } else {
> - set done 1
> + _do_clone_submodules $this
>   }
>  }
>  
> @@ -996,7 +1026,7 @@ method _postcheckout_wait {fd_ph} {
>   hook_failed_popup post-checkout $pch_error 0
>   }
>   unset pch_error
> - set done 1
> + _do_clone_submodules $this
>   return
>   }
>   fconfigure $fd_ph -blocking 0
> 

--
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] cache_tree_find(): remove redundant checks

[Junio C Hamano]

Junio C Hamano  writes:

> Michael Haggerty  writes:
>
>> I really wish we could mix declarations with statements because I think
>> it is a big help to readability.
> ...
> Unfortunately, I think we are in violent disagreement.

After re-reading the above, I realize that my statement may have
sounded a lot stronger than I intended it to.  If our codebase
allowed decl-after-stmt, that would change the equation and a
different style might help readability somewhat.

If decl-after-stmt were allowed, the group of lines that declare
variables at the beginning before the real logic begins do not even
have to be there, and "if some variables have initialization that
involve program logic that need to be read carefully, the
declaration at the beginning no longer can be coasted over as
boilerplate" complaint disappears.  The entire block can become the
logic, declaring variables as necessary at the point they are
required, without having to have a separate decl at the beginning.

Note that I am not advocating to allow decl-after-stmt; I do not
think the imagined readability benefit is worth it.
--
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 v3 3/6] cache_tree_find(): fix comment formatting

[Michael Haggerty]

Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index d00f4ef..408ee57 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -555,8 +555,9 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
struct cache_tree_sub *sub;
 
slash = strchrnul(path, '/');
-   /* between path and slash is the name of the
-* subtree to look for.
+   /*
+* Between path and slash is the name of the subtree
+* to look for.
 */
sub = find_subtree(it, path, slash - path, 0);
if (!sub)
-- 
1.9.0

--
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 v3 0/6] cache_tree_find() workover

[Michael Haggerty]

Who would have expected so much slop in one little function?  Here are
the changes coming out of a long email thread.

I feel a little bit silly submitting six separate patches, but they
really are independent.  And two of the changes were suggested by
other people, so splitting those out, at least, helps give credit
where it is due.  But feel free to squash the patches together if you
think it is best.

Changes since v2 (aside from the atomization):

* Use strchrnul() to initialize slash (as suggested by Junio)

* Don't initialize variable at declaration (as per Junio's
  preference).

Thanks to Junio and David for this workout.

Michael Haggerty (6):
  cache_tree_find(): remove redundant checks
  cache_tree_find(): initialize slash using strchrnul()
  cache_tree_find(): fix comment formatting
  cache_tree_find(): remove redundant check
  cache_tree_find(): remove early return
  cache_tree_find(): use path variable when passing over slashes

 cache-tree.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

-- 
1.9.0

--
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 v3 4/6] cache_tree_find(): remove redundant check

[Michael Haggerty]

If *slash == '/', then it is necessarily non-NUL.

Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cache-tree.c b/cache-tree.c
index 408ee57..39ad8c9 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -563,7 +563,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
if (!sub)
return NULL;
it = sub->cache_tree;
-   while (*slash && *slash == '/')
+   while (*slash == '/')
slash++;
if (!*slash)
return it; /* prefix ended with slashes */
-- 
1.9.0

--
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 v3 5/6] cache_tree_find(): remove early return

[Michael Haggerty]

There is no need for an early

return it;

from the loop if slash points at the end of the string, because that
is exactly what will happen when the while condition fails at the
start of the next iteration.

Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 39ad8c9..17db9f9 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -565,8 +565,6 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
it = sub->cache_tree;
while (*slash == '/')
slash++;
-   if (!*slash)
-   return it; /* prefix ended with slashes */
path = slash;
}
return it;
-- 
1.9.0

--
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 v3 6/6] cache_tree_find(): use path variable when passing over slashes

[Michael Haggerty]

The search for the end of the slashes is part of the update of the
path variable for the next iteration as opposed to an update of the
slash variable.  So iterate using path rather than slash.

Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 17db9f9..7f8d74d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -563,9 +563,10 @@ static struct cache_tree *cache_tree_find(struct 
cache_tree *it, const char *pat
if (!sub)
return NULL;
it = sub->cache_tree;
-   while (*slash == '/')
-   slash++;
+
path = slash;
+   while (*path == '/')
+   path++;
}
return it;
 }
-- 
1.9.0

--
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 v3 1/6] cache_tree_find(): remove redundant checks

[Michael Haggerty]

slash is initialized to a value that cannot be NULL.  So remove the
guards against slash == NULL later in the loop.

Suggested-by: David Kastrup 
Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 0bbec43..4d439bd 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -564,10 +564,9 @@ static struct cache_tree *cache_tree_find(struct 
cache_tree *it, const char *pat
if (!sub)
return NULL;
it = sub->cache_tree;
-   if (slash)
-   while (*slash && *slash == '/')
-   slash++;
-   if (!slash || !*slash)
+   while (*slash && *slash == '/')
+   slash++;
+   if (!*slash)
return it; /* prefix ended with slashes */
path = slash;
}
-- 
1.9.0

--
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 v3 2/6] cache_tree_find(): initialize slash using strchrnul()

[Michael Haggerty]

Suggested-by: Junio Hamano 
Signed-off-by: Michael Haggerty 
---
 cache-tree.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/cache-tree.c b/cache-tree.c
index 4d439bd..d00f4ef 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -554,9 +554,7 @@ static struct cache_tree *cache_tree_find(struct cache_tree 
*it, const char *pat
const char *slash;
struct cache_tree_sub *sub;
 
-   slash = strchr(path, '/');
-   if (!slash)
-   slash = path + strlen(path);
+   slash = strchrnul(path, '/');
/* between path and slash is the name of the
 * subtree to look for.
 */
-- 
1.9.0

--
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: New directory lost by git am

[Jeff King]

On Wed, Mar 05, 2014 at 11:47:12AM -0500, Phillip Susi wrote:

> > I can't get Chris's script to fail on any version of git. Can you
> > show us an example of a patch that does not behave (or better yet,
> > a reproduction recipe to generate the patch with "format-patch")?
> 
> AHA!  It requires a conflict.  There were simple conflicts in the NEWS
> file so I applied the patch with git am --reject and fixed up the
> NEWS, and ran git am --resolved.  The git am --reject fails to add the
> new directory to the index.

Thanks, I can reproduce here. I do not think it has anything to do with
being in a subdirectory; any new file does not get added to the index.
In fact, I do not think we update the index at all with "--reject". For
example, try this:

git init repo &&
cd repo &&

echo base >conflict &&
echo base >modified &&
git add . &&
git commit -m base &&

echo master >conflict &&
git add . &&
git commit -m master &&

git checkout -b other HEAD^ &&
echo other >conflict &&
echo other >modified &&
echo other >new &&
git add . &&
git commit -m other &&

git checkout master &&
git format-patch other -1 --stdout >patch &&
git am --reject patch

Running "git status -s" shows:

   M modified
   ?? conflict.rej
   ?? new
   ?? patch

We apply the changes to "modified" and "new" to the working tree, but we
do not stage anything in the index. I suspect this is because our
invocation of "apply --index" (which is what is doing the real work with
"--reject" here) bails before touching the index. In theory it should be
able to update the index for files that applied cleanly and leave the
other ones alone.

But I have not thought hard about it, so maybe there is a good reason
not to (it is a little weird just because the resulting index is a
partial application of the patch).  The "am -3" path does what you want
here, but it is much simpler: it knows it can represent the 3-way
conflict in the index. So the index represents the complete state of the
patch application at the end, including conflicts.

-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: New directory lost by git am

[Phillip Susi]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 3/5/2014 11:34 AM, Jeff King wrote:
> I don't think those steps are necessary for Chris's example. When
> he switches back to the master branch, git removes the subdirectory
> (the file is tracked in "temp" but not "master", so we remove it
> when switching branches, and then the directory is empty, so we
> clean it up, too). You can verify with an extra "ls" after the
> checkout but before the "am".

Right.

>>> * "git apply" parsed patches that add new files, generated by 
>>> programs other than Git, incorrectly.  This is an old breakage
>>> in v1.7.11.
>>> 
>>> Does that sound like your problem? If you can I'd suggest 
>>> updating, ideally to the recent 1.9.0 release but if you're
>>> feeling conservative try 1.8.3.4.
>> 
>> Vaguely, except for the "other than git" part.  This patch was 
>> generated by git-format-patch ( I didn't think apply handled
>> patches that weren't ).
> 
> I can't get Chris's script to fail on any version of git. Can you
> show us an example of a patch that does not behave (or better yet,
> a reproduction recipe to generate the patch with "format-patch")?

AHA!  It requires a conflict.  There were simple conflicts in the NEWS
file so I applied the patch with git am --reject and fixed up the
NEWS, and ran git am --resolved.  The git am --reject fails to add the
new directory to the index.


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTF1UOAAoJEI5FoCIzSKrwTD4H/35pUf8DFsbwPIVVQi+8I8e3
5NMHwQrHK3TPbZigVPBgVfwRCtOAxX656BPhninfhix99HWs00W5zGaFDwkymRNp
87EeU3LVcIjapqijszw9AqwBLvfm9uzXEus964hShCJVOmKBezQfl6Mvcrkn5Na1
UchJLkRzEoi6VUyUso8FH0xpL7JyjF08H19dtvXoUbrvrXYuN1Ys3UMBHXVEVdi+
5O924lo4+psgdjGZ3HUpclYRbKO0LS5IVMCxFRw5Q+EfARJQ7NXzv/csRXIKyms7
roCQqmQnnem71GHx6SQaepnY5pKuEnmmDaqXbCOqZdpyfo1CB7SFJDq/VXrbLyw=
=zS2r
-END PGP SIGNATURE-
--
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: New directory lost by git am

[Jeff King]

On Wed, Mar 05, 2014 at 09:26:43AM -0500, Phillip Susi wrote:

> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
> 
> On 3/5/2014 3:10 AM, Chris Packham wrote:
> > My example is creating a commit on the "temp" branch then applying
> > it to the "master" branch using git am.
> > 
> >> Do a reset HEAD~1 --hard, and git clean -x -f -d before git am.
> >> I didn't notice the missing file myself for some time because it
> >> is left in the working tree, just not added to the index and
> >> included in the commit.
> >> 
> 
> Right... so the file is left in the directory, even though it is not
> checked in.  A git status should show it is an unknown file, and a
> clean should remove it.

I don't think those steps are necessary for Chris's example. When he
switches back to the master branch, git removes the subdirectory (the
file is tracked in "temp" but not "master", so we remove it when
switching branches, and then the directory is empty, so we clean it up,
too). You can verify with an extra "ls" after the checkout but before
the "am".

> > * "git apply" parsed patches that add new files, generated by
> > programs other than Git, incorrectly.  This is an old breakage in
> > v1.7.11.
> > 
> > Does that sound like your problem? If you can I'd suggest
> > updating, ideally to the recent 1.9.0 release but if you're feeling
> > conservative try 1.8.3.4.
> 
> Vaguely, except for the "other than git" part.  This patch was
> generated by git-format-patch ( I didn't think apply handled patches
> that weren't ).

I can't get Chris's script to fail on any version of git. Can you show
us an example of a patch that does not behave (or better yet, a
reproduction recipe to generate the patch with "format-patch")?

-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: Bad git log behavior with multiple glob path arguments

[Jeremy Nickurak]

On Wed, Mar 5, 2014 at 3:01 AM, Duy Nguyen  wrote:
> On Wed, Mar 5, 2014 at 12:56 PM, Jeremy Nickurak  wrote:
>> git log seems to understand globs in the last path argument, and the
>> last path argument only. I didn't see anything in the git log man page
>> expressly saying this was to be expected, but it does seem like it
>> ought to work for all the arguments or none of them.
>
> What version did you use? We have a fix in the same area,
> e4ddb05 (tree_entry_interesting: match against all pathspecs -
> 2014-01-25), and it's in v1.8.5.5 and v1.9.0

So close! :) 1.8.5.3

>> Note that glob matching doesn't seem to occur unless '--' is included.
>
> do you mean "git log" does not run at all and complains about
> disambiguation, or it runs but nothing is filtered?

It complains about disambiguation:

$ mkdir -p ~/tmp; cd ~/tmp; git init; echo hello > hello.txt; git add
hello.txt; git commit -m hello; echo "Without --:"; git log --oneline
'*.txt'; echo "With --:"; git log --oneline -- '*.txt';
Reinitialized existing Git repository in /home/nickuj/tmp/.git/
On branch master
nothing to commit, working directory clean
Without --:
fatal: ambiguous argument '*.txt': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions, like this:
'git  [...] -- [...]'
With --:
78ff378 hello
--
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: [RFC 2/3] merge: Add hints to tell users about "git merge --abort"

[Andrew Wong]

On Wed, Feb 26, 2014 at 3:38 PM, Jonathan Nieder  wrote:
> Andrew Wong wrote:
>
>> --- a/builtin/merge.c
>> +++ b/builtin/merge.c
>> @@ -909,7 +909,8 @@ static int suggest_conflicts(int renormalizing)
>>   fclose(fp);
>>   rerere(allow_rerere_auto);
>>   printf(_("Automatic merge failed; "
>> - "fix conflicts and then commit the result.\n"));
>> + "fix conflicts and then commit the result.\n"
>> + "To abort the merge, use \"git merge --abort\".\n"));
>
> Seems reasonable, but I worry about the command growing too noisy.
>
> Could this be guarded by an advice. setting?  (See advice.*
> in git-config(1) for what I mean.)

I was planning to use advice.resolveConflict, but as I went through
merge.c, I noticed there could be a few other situations where we
could print out the same message:
1. when prepare_to_commit() fails, due to hook error, editor error, or
empty commit message
2. "git commit --no-commit"

This means contexts are no longer only about "resolving conflict", so
I was thinking of renaming advice.resolveConflict to something like
advice.mergeHints.

Any thoughts?
--
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: New directory lost by git am

[Phillip Susi]

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 3/5/2014 3:10 AM, Chris Packham wrote:
> My example is creating a commit on the "temp" branch then applying
> it to the "master" branch using git am.
> 
>> Do a reset HEAD~1 --hard, and git clean -x -f -d before git am.
>> I didn't notice the missing file myself for some time because it
>> is left in the working tree, just not added to the index and
>> included in the commit.
>> 

Right... so the file is left in the directory, even though it is not
checked in.  A git status should show it is an unknown file, and a
clean should remove it.

> Regardless of reproducing the issue a quick glance at the Release
> notes for 1.8.3.3 the following sticks out:
> 
> Fixes since v1.8.3.2 
> 
> * "git apply" parsed patches that add new files, generated by
> programs other than Git, incorrectly.  This is an old breakage in
> v1.7.11.
> 
> Does that sound like your problem? If you can I'd suggest
> updating, ideally to the recent 1.9.0 release but if you're feeling
> conservative try 1.8.3.4.

Vaguely, except for the "other than git" part.  This patch was
generated by git-format-patch ( I didn't think apply handled patches
that weren't ).


-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJTFzQiAAoJEI5FoCIzSKrwXEMH/iQdFdAApGnFCyLGA4l87d4M
ITLtyL632gHA39KnlEqtTc6TgFjQMrV3m8s9TeR6DiEI9qGvNvnP2E4JBFORZprk
RSJoCa9qMuAYOtSEtwrzbhMZpBN7hAZeJ7txP2KwZiGXoWjr4RSawjViQHhnXQEP
L9QMmwjWJBwZE/eklYg8W+Ov987uribGTgOL8Wx1iMME2C88VfyCWtg1ClTz3aEh
UeyAvyLxSA+YvS4xg+nUBAxXX8bFI0g53Yjf3Lt/1/EzdO67sH7CRChR67BmANWZ
NBoxBqenN6/qg0rfRojOnjGTtRpAJX48dgnEHulUJrixrmqBYXAbi8dNVW8KkiM=
=tzX5
-END PGP SIGNATURE-
--
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 1/2] i18n: proposed command missing leading dash

[Jiang Xin]

2014-03-05 2:40 GMT+08:00 Junio C Hamano :
> From: Sandy Carter 
> Date: Mon,  3 Mar 2014 09:55:53 -0500
>
> Add missing leading dash to proposed commands in french output when
> using the command:
> git branch --set-upstream remotename/branchname
> and when upstream is gone
>
> Signed-off-by: Sandy Carter 
> Signed-off-by: Junio C Hamano 
> ---
>
>  * Forwarding to the i18n coordinator.  I don't do French, but this
>seems trivially correct.

Applied to maint branch of git://github.com/git-l10n/git-po, and can be
merged to master directly.

-- 
Jiang Xin
--
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] commit.c: Replace starts_with() with skip_prefix()

[karthik nayak]

Hey Eric,
Sorry about not cc'ing you again , still figuring out "git send-email".


On Wed, Mar 5, 2014 at 7:36 PM, Karthik Nayak  wrote:
> Replaces all instances of starts_with() by skip_prefix(),
> which can not only be used to check presence of a prefix,
> but also used further on as it returns the string after the prefix,
> if the prefix is present.
>
> Signed-off-by: Karthik Nayak 
> ---
>
> Hey Eric,
> Here are the changes i have made in this Patch v2:
> 1. Edited the variables names to fit their usage
> 2. set my emacs to indent 8 tabs, so proper indentation
> 3. fixed my error where i increased the value by 1 in parse_signed_commit().
> Thanks again for your patience.
>
> ---
>  commit.c | 23 ---
>  1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/commit.c b/commit.c
> index 6bf4fe0..f006490 100644
> --- a/commit.c
> +++ b/commit.c
> @@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab 
> *author_date,
> struct ident_split ident;
> char *date_end;
> unsigned long date;
> +   const char *buf_split;
>
> if (!commit->buffer) {
> unsigned long size;
> @@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab 
> *author_date,
> return;
> }
>
> +   buf_split = skip_prefix(buf, "author ");
> +
> for (buf = commit->buffer ? commit->buffer : buffer;
>  buf;
>  buf = line_end + 1) {
> line_end = strchrnul(buf, '\n');
> -   if (!starts_with(buf, "author ")) {
> +   if (!buf_split) {
> if (!line_end[0] || line_end[1] == '\n')
> return; /* end of header */
> continue;
> }
> -   if (split_ident_line(&ident,
> -buf + strlen("author "),
> -line_end - (buf + strlen("author "))) ||
> +   if (split_ident_line(&ident, buf_split,
> +line_end - buf_split) ||
> !ident.date_begin || !ident.date_end)
> goto fail_exit; /* malformed "author" line */
> break;
> @@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1,
> char *buffer = read_sha1_file(sha1, &type, &size);
> int in_signature, saw_signature = -1;
> char *line, *tail;
> +   const char *line_split;
>
> if (!buffer || type != OBJ_COMMIT)
> goto cleanup;
> @@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,
> char *next = memchr(line, '\n', tail - line);
>
> next = next ? next + 1 : tail;
> +   line_split = skip_prefix(line, gpg_sig_header);
> if (in_signature && line[0] == ' ')
> sig = line + 1;
> -   else if (starts_with(line, gpg_sig_header) &&
> -line[gpg_sig_header_len] == ' ')
> -   sig = line + gpg_sig_header_len + 1;
> +   else if (line_split && line_split[0] == ' ')
> +   sig = line_split + 1;
> if (sig) {
> strbuf_add(signature, sig, next - sig);
> saw_signature = 1;
> @@ -1193,10 +1196,8 @@ static void parse_gpg_output(struct signature_check 
> *sigc)
> for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
> const char *found, *next;
>
> -   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
> -   /* At the very beginning of the buffer */
> -   found = buf + strlen(sigcheck_gpg_status[i].check + 
> 1);
> -   } else {
> +   found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
> +   if(!found) {
> found = strstr(buf, sigcheck_gpg_status[i].check);
> if (!found)
> continue;
> --
> 1.9.0.138.g2de3478
>
--
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 v2] commit.c: Replace starts_with() with skip_prefix()

[Karthik Nayak]

Replaces all instances of starts_with() by skip_prefix(),
which can not only be used to check presence of a prefix,
but also used further on as it returns the string after the prefix,
if the prefix is present.

Signed-off-by: Karthik Nayak 
---

Hey Eric,
Here are the changes i have made in this Patch v2:
1. Edited the variables names to fit their usage
2. set my emacs to indent 8 tabs, so proper indentation
3. fixed my error where i increased the value by 1 in parse_signed_commit().
Thanks again for your patience.

---
 commit.c | 23 ---
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/commit.c b/commit.c
index 6bf4fe0..f006490 100644
--- a/commit.c
+++ b/commit.c
@@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab 
*author_date,
struct ident_split ident;
char *date_end;
unsigned long date;
+   const char *buf_split;
 
if (!commit->buffer) {
unsigned long size;
@@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab 
*author_date,
return;
}
 
+   buf_split = skip_prefix(buf, "author ");
+
for (buf = commit->buffer ? commit->buffer : buffer;
 buf;
 buf = line_end + 1) {
line_end = strchrnul(buf, '\n');
-   if (!starts_with(buf, "author ")) {
+   if (!buf_split) {
if (!line_end[0] || line_end[1] == '\n')
return; /* end of header */
continue;
}
-   if (split_ident_line(&ident,
-buf + strlen("author "),
-line_end - (buf + strlen("author "))) ||
+   if (split_ident_line(&ident, buf_split,
+line_end - buf_split) ||
!ident.date_begin || !ident.date_end)
goto fail_exit; /* malformed "author" line */
break;
@@ -1098,6 +1100,7 @@ int parse_signed_commit(const unsigned char *sha1,
char *buffer = read_sha1_file(sha1, &type, &size);
int in_signature, saw_signature = -1;
char *line, *tail;
+   const char *line_split;
 
if (!buffer || type != OBJ_COMMIT)
goto cleanup;
@@ -,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,
char *next = memchr(line, '\n', tail - line);
 
next = next ? next + 1 : tail;
+   line_split = skip_prefix(line, gpg_sig_header);
if (in_signature && line[0] == ' ')
sig = line + 1;
-   else if (starts_with(line, gpg_sig_header) &&
-line[gpg_sig_header_len] == ' ')
-   sig = line + gpg_sig_header_len + 1;
+   else if (line_split && line_split[0] == ' ')
+   sig = line_split + 1;
if (sig) {
strbuf_add(signature, sig, next - sig);
saw_signature = 1;
@@ -1193,10 +1196,8 @@ static void parse_gpg_output(struct signature_check 
*sigc)
for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
const char *found, *next;
 
-   if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-   /* At the very beginning of the buffer */
-   found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-   } else {
+   found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
+   if(!found) {
found = strstr(buf, sigcheck_gpg_status[i].check);
if (!found)
continue;
-- 
1.9.0.138.g2de3478

--
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: Branch Name Case Sensitivity

[Lee Hopkins]

> Lee, could you improve your change in refs.c into a real patch, with a commit 
> message?
> (And please have a look at the indentation with TABs)
>
> A test case could be good, if time allows I can make a suggestion.

I will remove the refs.ignorecase flag and work on a test care or two,
it will have to wait a few days tho.

> (and everything else could and should go into another patch:
>  If we ever want Linux to ignore the case in refs,
>  to ease the cross-platform development with Windows.
>  Or if we allow Windows/Mac OS to handle case insensitive refs (by always 
> packing them)
>  to ease the co-working with e.g. Linux.
> )

I was actually planning on tying to add this to my changes if they
gained any traction. Why is another patch desirable?

> If the variable is not in 'core.' namespace, you should implement
> this check at the Porcelain level, allowing lower-level tools like
> update-ref as an escape hatch that let users bypass the restriction
> to be used to correct breakages; it would mean an unconditional "if
> !stricmp(), it is an error" in refs.c will not work well.
>
> I think it might be OK to have
>
> core.allowCaseInsentitiveRefs = {yes|no|warn}
>
> which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
> corresponds to 'error', in the previous suggestion), instead. If we
> wanted to prevent even lower-level tools like update-ref from
> bypassing the check, that is.

I also would not mind working on either of Junio's suggestions if one
is more desirable than what I already have.

-Lee
--
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] upload-pack: allow shallow fetching from a read-only repository

[Duy Nguyen]

On Wed, Mar 5, 2014 at 1:10 AM, Junio C Hamano  wrote:
> I notice that the original code, with or without this change, allows
> upload-pack spawned by daemon to attempt to write into GIT_DIR.
> As upload-pack is supposed to be a read-only operation, this is
> quite bad.
>
> Perhaps we should give server operators an option to run their
> daemon -> upload-pack chain to always write to a throw-away
> directory of their choice, without ever attempting to write to
> GIT_DIR it serves?

That would be setting TMPDIR before running git-daemon, I think.
Except places that we ignore TMPDIR like this one.

> How well is the access to the temporary shallow file controlled in
> your code (sorry, but I do not recall carefully reading your patch
> that added the mechanism with security issues in mind, so now I am
> asking)?  When it is redirected to TMPDIR (let's forget GIT_DIR for
> now---if an attacker can write into there, the repository is already
> lost), can an attacker race with us to cause us to overwrite we do
> not expect to?

I'm sorry to say that attackers were simply not a concern when I wrote
the patch. Not even that upload-pack is a read-only operation (so
obvious now that I think about this). I think racing is possible, yes.

> Even if it turns out that this patch is secure enough as-is, we
> definitely need to make sure that server operators, who want to keep
> their upload-pack truly a read-only operation, know that it is
> necessary to (1) keep the system user they run git-daemon under
> incapable of writing into GIT_DIR, and (2) make sure TMPDIR points
> at somewhere only git-daemon user and nobody else can write into,
> somewhere in the documentation.

If only there is a way to pass this info without a temporary
file. Multiplexing it to pack-objects' stdin should work. It may be
ugly, but it's probably the safest way.

Wait it does not look that ugly. We can feed "--shallow " lines
before sending want/have/edge objects. Something like this seems to
work (just ran a few shallow-related tests, not the whole test suite)

-- 8< --
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c733379..130097c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2467,6 +2467,14 @@ static void get_object_list(int ac, const char **av)
write_bitmap_index = 0;
continue;
}
+   if (starts_with(line, "--shallow ")) {
+   unsigned char sha1[20];
+   if (get_sha1_hex(line + 10, sha1))
+   die("not an SHA-1 '%s'", line + 10);
+   register_shallow(sha1);
+   /* XXX: set shallow.c:is_shallow = 1 ? */
+   continue;
+   }
die("not a rev '%s'", line);
}
if (handle_revision_arg(line, &revs, flags, 
REVARG_CANNOT_BE_FILENAME))
diff --git a/upload-pack.c b/upload-pack.c
index 0c44f6b..a5c50e4 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -70,6 +70,14 @@ static ssize_t send_client_data(int fd, const char *data, 
ssize_t sz)
return sz;
 }
 
+static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
+{
+   FILE *fp = cb_data;
+   if (graft->nr_parent == -1)
+   fprintf(fp, "--shallow %s\n", sha1_to_hex(graft->sha1));
+   return 0;
+}
+
 static void create_pack_file(void)
 {
struct child_process pack_objects;
@@ -81,12 +89,10 @@ static void create_pack_file(void)
const char *argv[12];
int i, arg = 0;
FILE *pipe_fd;
-   char *shallow_file = NULL;
 
if (shallow_nr) {
-   shallow_file = setup_temporary_shallow(NULL);
argv[arg++] = "--shallow-file";
-   argv[arg++] = shallow_file;
+   argv[arg++] = "";
}
argv[arg++] = "pack-objects";
argv[arg++] = "--revs";
@@ -114,6 +120,9 @@ static void create_pack_file(void)
 
pipe_fd = xfdopen(pack_objects.in, "w");
 
+   if (shallow_nr)
+   for_each_commit_graft(write_one_shallow, pipe_fd);
+
for (i = 0; i < want_obj.nr; i++)
fprintf(pipe_fd, "%s\n",
sha1_to_hex(want_obj.objects[i].item->sha1));
@@ -242,12 +251,6 @@ static void create_pack_file(void)
error("git upload-pack: git-pack-objects died with error.");
goto fail;
}
-   if (shallow_file) {
-   if (*shallow_file)
-   unlink(shallow_file);
-   free(shallow_file);
-   }
-
/* flush the data */
if (0 <= buffered) {
data[0] = buffered;
-- 8< --
-- 
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://vg

Re: [PATCH v4 27/27] count-objects: report unused files in $GIT_DIR/repos/...

[Duy Nguyen]

On Wed, Mar 5, 2014 at 11:25 AM, Eric Sunshine  wrote:
>>  static void update_common_dir(struct strbuf *buf, int git_dir_len)
>>  {
>> +   /*
>> +* Remember to report_linked_checkout_garbage()
>> +* builtin/count-objects.c
>> +*/
>
> I couldn't figure out why this comment was telling me to remember to
> report "linked checkout garbage" until I realized that you omitted the
> word "update" (as in "remember to update"). It might be clearer to say
> something along these lines:
>
> Keep synchronized with related list in
> builtin/count-objects.c:report_linked_checkout_garbage().
>

I have a tendency to accidentally important words :)

> Is it not possible or just too much of a hassle to maintain this list
> in just one place, as in a header which is included by these two
> files? The exceptions, such as 'log' and 'gc.pid', could be marked by
> a special character in the entry ("!gc.pid", for example) or any such
> scheme.

It might work. Let me try.
-- 
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

Loan Application

[Loans]

Loan Application at a low rate of 0.5% send your Name,Amount,Phone and country 
to standar...@56788.com

Note: $5,000.00 USD minimum and $100,000,000 Maximum.
--
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: Bad git log behavior with multiple glob path arguments

[Duy Nguyen]

On Wed, Mar 5, 2014 at 12:56 PM, Jeremy Nickurak  wrote:
> git log seems to understand globs in the last path argument, and the
> last path argument only. I didn't see anything in the git log man page
> expressly saying this was to be expected, but it does seem like it
> ought to work for all the arguments or none of them.

What version did you use? We have a fix in the same area,
e4ddb05 (tree_entry_interesting: match against all pathspecs -
2014-01-25), and it's in v1.8.5.5 and v1.9.0

> Here's a little shell script I ended up using to convince myself I
> wasn't going crazy. I'd expect the same output for all of the git log
> test, since they all specify (either with globs or not) all the files
> added to the repository. Alternatively, if globs aren't expected to
> work, I'd at least expect all the glob tests to return nothing.
>
> Note that glob matching doesn't seem to occur unless '--' is included.

do you mean "git log" does not run at all and complains about
disambiguation, or it runs but nothing is filtered?

> I'm not exactly clear on why that is.
>
> #!/bin/sh
>
> TESTREPO="$(pwd)/bad-glob-test-repo"
>
> rm  -rf "$TESTREPO"
>
> echo "Running tests in $TESTREPO"
> mkdir "$TESTREPO"
> cd "$TESTREPO"
> mkdir subdira
> mkdir subdirb
> mkdir subdirc
>
> git init
> echo a > subdira/file.txt
> echo b > subdirb/file.txt
> echo c > subdirc/file.txt
>
> git add subdira/file.txt
> git commit -m 'a'
>
> git add subdirb/file.txt
> git commit -m 'b'
>
> git add subdirc/file.txt
> git commit -m 'c'
>
> echo Glob Test 1: git log --oneline -- 'subdira/*.txt' 'subdirb/*.txt'
> 'subdirc/*.txt'
> git log --oneline -- 'subdira/*.txt' 'subdirb/*.txt' 'subdirc/*.txt'
>
> echo Glob Test 2: git log --oneline -- 'subdira/*.txt' 'subdirc/*.txt'
> 'subdirb/*.txt'
> git log --oneline -- 'subdira/*.txt' 'subdirc/*.txt' 'subdirb/*.txt'
>
> echo Glob Test 3: git log --oneline -- 'subdirb/*.txt' 'subdira/*.txt'
> 'subdirc/*.txt'
> git log --oneline -- 'subdirb/*.txt' 'subdira/*.txt' 'subdirc/*.txt'
>
> echo Glob Test 4: git log --oneline -- 'subdirb/*.txt' 'subdirc/*.txt'
> 'subdira/*.txt'
> git log --oneline -- 'subdirb/*.txt' 'subdirc/*.txt' 'subdira/*.txt'
>
> echo Glob Test 5: git log --oneline -- 'subdirc/*.txt' 'subdira/*.txt'
> 'subdirb/*.txt'
> git log --oneline -- 'subdirc/*.txt' 'subdira/*.txt' 'subdirb/*.txt'
>
> echo Glob Test 6: git log --oneline -- 'subdirc/*.txt' 'subdirb/*.txt'
> 'subdira/*.txt'
> git log --oneline -- 'subdirc/*.txt' 'subdirb/*.txt' 'subdira/*.txt'
>
> echo Explicit Test 1: git log --oneline -- 'subdira/file.txt'
> 'subdirb/file.txt' 'subdirc/file.txt'
> git log --oneline -- 'subdira/file.txt' 'subdirb/file.txt' 'subdirc/file.txt'
>
> echo Explicit Test 2: git log --oneline -- 'subdira/file.txt'
> 'subdirc/file.txt' 'subdirb/file.txt'
> git log --oneline -- 'subdira/file.txt' 'subdirc/file.txt' 'subdirb/file.txt'
>
> echo Explicit Test 3: git log --oneline -- 'subdirb/file.txt'
> 'subdira/file.txt' 'subdirc/file.txt'
> git log --oneline -- 'subdirb/file.txt' 'subdira/file.txt' 'subdirc/file.txt'
>
> echo Explicit Test 4: git log --oneline -- 'subdirb/file.txt'
> 'subdirc/file.txt' 'subdira/file.txt'
> git log --oneline -- 'subdirb/file.txt' 'subdirc/file.txt' 'subdira/file.txt'
>
> echo Explicit Test 5: git log --oneline -- 'subdirc/file.txt'
> 'subdira/file.txt' 'subdirb/file.txt'
> git log --oneline -- 'subdirc/file.txt' 'subdira/file.txt' 'subdirb/file.txt'
>
> echo Explicit Test 6: git log --oneline -- 'subdirc/file.txt'
> 'subdirb/file.txt' 'subdira/file.txt'
> git log --oneline -- 'subdirc/file.txt' 'subdirb/file.txt' 'subdira/file.txt'
>
> --
> Jeremy Nickurak -= Email/XMPP: -= jer...@nickurak.ca =-
> --
> 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



-- 
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

git push first asks for credentials, then checks the branch exists

[Dmitry]

Hi,

Here's my usecase. I have created a BranchWithVeryLongName and I want to have 
it pushed to origin. So I use this command with version 1.8.1.2:

git push origin BranchMistypedLongName

(note that I mistyped the branch name). The following happens:
1. git asks me for username and password
2. it authenticates on the origin server
3. it bails out with "error: sfc refspec BranchMistypedLongName does not match 
any"


Can't git perhaps check that the branch exists before it asks me for 
credentials and just say there's no such branch?

Could you please fix this?

Thank you.
Dmitry.
--
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 17/19] Portable alloca for Git

[Kirill Smelkov]

On Fri, Feb 28, 2014 at 06:19:58PM +0100, Erik Faye-Lund wrote:
> On Fri, Feb 28, 2014 at 6:00 PM, Kirill Smelkov  wrote:
> > On Fri, Feb 28, 2014 at 02:50:04PM +0100, Erik Faye-Lund wrote:
> >> On Fri, Feb 28, 2014 at 2:44 PM, Erik Faye-Lund  
> >> wrote:
> >> > On Mon, Feb 24, 2014 at 5:21 PM, Kirill Smelkov  wrote:
> >> >> diff --git a/Makefile b/Makefile
> >> >> index dddaf4f..0334806 100644
> >> >> --- a/Makefile
> >> >> +++ b/Makefile
> >> >> @@ -316,6 +321,7 @@ endif
> >> >>  ifeq ($(uname_S),Windows)
> >> >> GIT_VERSION := $(GIT_VERSION).MSVC
> >> >> pathsep = ;
> >> >> +   HAVE_ALLOCA_H = YesPlease
> >> >> NO_PREAD = YesPlease
> >> >> NEEDS_CRYPTO_WITH_SSL = YesPlease
> >> >> NO_LIBGEN_H = YesPlease
> >> >
> >> > In MSVC, alloca is defined in in malloc.h, not alloca.h:
> >> >
> >> > http://msdn.microsoft.com/en-us/library/wb1s57t5.aspx
> >> >
> >> > In fact, it has no alloca.h at all. But we don't have malloca.h in
> >> > mingw either, so creating a compat/win32/alloca.h that includes
> >> > malloc.h is probably sufficient.
> >>
> >> "But we don't have alloca.h in mingw either", sorry.
> >
> > Don't we have that for MSVC already in
> >
> > compat/vcbuild/include/alloca.h
> >
> > and
> >
> > ifeq ($(uname_S),Windows)
> > ...
> > BASIC_CFLAGS = ... -Icompat/vcbuild/include ...
> >
> >
> > in config.mak.uname ?
> 
> Ah, of course. Thanks for setting me straight!
> 
> > And as I've not touched MINGW part in config.mak.uname the patch stays
> > valid as it is :) and we can incrementally update what platforms have
> > working alloca with follow-up patches.
> >
> > In fact that would be maybe preferred, for maintainers to enable alloca
> > with knowledge and testing, as one person can't have them all at hand.
> 
> Yeah, you're probably right.

Erik, the patch has been merged into pu today. Would you please
follow-up with tested MINGW change?

Thanks beforehand,
Kirill
--
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] t7800: add a difftool test for .git-files

[David Aguilar]

From: Junio C Hamano 

Signed-off-by: David Aguilar 
---
This is a replacement patch for the current tip of da/difftool.

 t/t7800-difftool.sh | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 2418528..986b78e 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -434,4 +434,18 @@ test_expect_success PERL 'difftool --no-symlinks detects 
conflict ' '
)
 '
 
+test_expect_success PERL 'difftool properly honors gitlink and core.worktree' '
+   git submodule add ./. submod/ule &&
+   (
+   cd submod/ule &&
+   test_config diff.tool checktrees &&
+   test_config difftool.checktrees.cmd '\''
+   test -d "$LOCAL" && test -d "$REMOTE" && echo good
+   '\'' &&
+   echo good>expect &&
+   git difftool --tool=checktrees --dir-diff HEAD~ >actual &&
+   test_cmp expect actual
+   )
+'
+
 test_done
-- 
1.8.5.5.2.g42fdfc9

--
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

[no subject]

[Stephanie Bennett]

I have a good news for you, for details contact me through
lottery...@yahoo.com
Sir Edmond Newton

_
This communication is intended for the use of the recipient to which it is 
addressed, and may contain confidential, personal, and or privileged 
information. Please contact us immediately if you are not the intended 
recipient of this communication, and do not copy, distribute, or take action 
relying on it. Any communication received in error, or subsequent reply, should 
be deleted or destroyed.
--
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] gitk: replace SHA1 entry field on keyboard paste

[Ilya Bobyr]

On 3/4/2014 10:15 AM, Junio C Hamano wrote:

From: Ilya Bobyr 
Date: Thu, 27 Feb 2014 22:51:37 -0800

We already replace old SHA with the clipboard content for the mouse
paste event.  It seems reasonable to do the same when pasting from
keyboard.

Signed-off-by: Ilya Bobyr 
---

  * Paul?  I do not use <> on my keyboard, so I am not in the
position to say that this patch is correct (or not).  I am just
forwarding it in case you think gitk users will find it useful.


I should have included a cover letter with additional details on what is 
this.
I am still a bit new to the process, so I did not realize it would be 
useful until now.


<> is the "clipboard paste" shortcut.  Ctrl+V on WIndows and Ubuntu.

SHA fields in gitk is a text field that accepts focus.  My expectation 
is that I can replace SHA already in there with one I have in the clipboard.
The problem is that I was not able to find an easy way.  Not immediately 
at least.

Here is what I observed:
1. If I just click in there and press Ctrl+V, SHA from the clipboard is 
inserted in the spot that I clicked.  Most cases in the middle of the 
SHA symbols already in there.

 Fixing the mess afterwards in just impossible.
2. If I double click "old" SHA in order to select and delete it before 
the paste, selected text is automatically copied into the clipboard (at 
least on Cygwin).
While the SHA field is now empty to accept "new" SHA, the clipboard 
content is lost and I have to go copy new SHA into the clipboard again.
3. I can click once to give SHA field keyboard focus, delete whatever is 
there using Delete and/or Backspace and then paste new SHA.
While this works it is not very convenient.  I also keeped pressing 
Ctrl+W to "delete a word" that was closing gitk window completely %)


Now, it turned out that if I use mouse to do the paste, not the 
keyboard, it does do what actually makes sense: removes old SHA and 
pastes the new one.
As SHA's are atomic, there seems to be no value in been able to insert 
one in the middle of another.


The bug, as I see it, is that it happens only if mouse paste is used.  
That would be the "<>" in the line above the added one.


When keyboard paste is used old SHA is not cleared.

I guess one would say that inserting with the mouse is actually easier, 
but been a Windows user it was unusual to me to use mouse paste in the GUI.

I have never even thought to do that and was struggling for quite a while.

The patch makes both mouse and keyboard paste work the same.
I think that could help someone else who is not used to mouse paste in 
the GUI text boxes as well.



The original patch was done against my tree, so I hand tweaked it
to apply to your tree.

Thanks.


  gitk |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitk b/gitk
index 90764e8..2f58bcf 100755
--- a/gitk
+++ b/gitk
@@ -2585,6 +2585,7 @@ proc makewindow {} {
  bind $fstring  {dofind 1 1}
  bind $sha1entry  {gotocommit; break}
  bind $sha1entry <> clearsha1
+bind $sha1entry <> clearsha1
  bind $cflist <1> {sel_flist %W %x %y; break}
  bind $cflist  {sel_flist %W %x %y; break}
  bind $cflist  {treeclick %W %x %y}


--
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: difftool sends malformed path to exernal tool on Windows

[David Aguilar]

On Mon, Mar 03, 2014 at 04:24:15PM -0700, Paul Lotz wrote:
> David,
> 
> OK, I did as you suggested, and the results were revealing.
> 
> First, I replaced "echo" with "cat".  Result: The contents of both files 
> appeared in the Git Bash Window.
> 
> Then I tried calling LVCompare from the Git Bash and Windows Command Prompt 
> windows with variations on the paths.
> 
> Here are the most relevant results:
> First from the Windows Command Prompt:
> 1) This command works:
> C:\LSST_TS\SystemSW\M2AADT>"C:\Program Files (x86)\National 
> Instruments\Shared\L
> abVIEW Compare\LVCompare.exe" 
> C:\Users\Paul\AppData\Local\Temp\Typedefs_TestStat
> us_Before.ctl C:\LSST_TS\SystemSW\M2AADT\Typedefs\TestStatus.ctl
> [General note:
> I saved a copy of the temp file and replaced the hex string with the string 
> 'Before' to make the file stick around.  The paths are otherwise the same.]

This is aligns with: 
http://zone.ni.com/reference/en-XX/help/371361H-01/lvhowto/configlvcomp_thirdparty/

"lvcompare.exe  ..."

The key thing is the mention of absolute paths.

What is happening is that lvcompare.exe (or likely it's a
Windows thing) changes its current directory to its installation
directory under Progra~1.

That means the relative paths passed in by difftool won't be found.

The way to fix it is to redirect your difftool config to a script
that makes all paths absolute.  This script can then call the real
lvcompare.exe.

You just need to tweak the lvcompare part in your .gitconfig
to look like this:

[difftool "lvcompare"]
cmd = ~/bin/lvcompare.sh \"$LOCAL\" \"$REMOTE\"


... and install an executable lvcompare.sh shell script in in
your $HOME/bin.  Something like this:

#!/bin/sh

abspath () {
(
cd "$(dirname "$1")" &&
printf "%s/%s" "$(pwd)" "$(basename "$1")"
)
}

lvcompare="C:\\Program Files (x86)\National Instruments\\Shared\\LabVIEW 
Compare\\LVCompare.exe"
local=$(abspath "$1")
remote=$(abspath "$2")
exec "$lvcompare" "$local" "$remote"

> 2) C:\LSST_TS\SystemSW\M2AADT>"C:\Program Files (x86)\National 
> Instruments\Shared\L
> abVIEW Compare\LVCompare.exe" 
> C:\Users\Paul\AppData\Local\Temp\Typedefs_TestStat
> us_Before.ctl Typedefs\TestStatus.ctl
> 
> Result: Error message with reference to C:\Program Files (x86)\National 
> Instruments\Shared\L
> abVIEW Compare\supportVIs\_prolvcmp.llb\Typedefs\TestStatus.ctl
> 
> Observation: The second path has to be the full path, not the relative path 
> we get back using "echo".

Yes, that's what it looks like.
-- 
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: New directory lost by git am

[Chris Packham]

On 05/03/14 16:22, Phillip Susi wrote:
> On 03/04/2014 10:08 PM, Chris Packham wrote:
>> Could you provide a few more details such as your git version (git 
>> --version) and an example of the failure. I've tried to reproduce
>> the problem based on the description provided but everything seems
>> to work as expected for me.
> 
> Version 1.8.3.2.
> 
>> git --version git version 1.9.0 mkdir test && cd test && git init 
>> echo "hello world" >a.txt git add a.txt git commit -m"Initial
>> commit" git checkout -b temp mkdir b echo "lorem ipsum" >b/b.txt 
>> git add b/b.txt git commit -m"Add b/b.txt" ls -R .: a.txt  b
>>
>> ./b: b.txt git checkout master git format-patch temp -1 --stdout |
>> git am ls -R .: a.txt  b
>>
>> ./b: b.txt
>>
> 
> You are reapplying the patch while it is already applied.

My example is creating a commit on the "temp" branch then applying it to
the "master" branch using git am.

> Do a reset
> HEAD~1 --hard, and git clean -x -f -d before git am.  I didn't notice
> the missing file myself for some time because it is left in the
> working tree, just not added to the index and included in the commit.
> 

Regardless of reproducing the issue a quick glance at the Release notes
for 1.8.3.3 the following sticks out:

Fixes since v1.8.3.2


 * "git apply" parsed patches that add new files, generated by programs
   other than Git, incorrectly.  This is an old breakage in v1.7.11.

Does that sound like your problem? If you can I'd suggest updating,
ideally to the recent 1.9.0 release but if you're feeling conservative
try 1.8.3.4.

--
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