[PATCH v2] rev-parse --parseopt: option argument name hints

2014-03-09 Thread Ilya Bobyr
Built-in commands can specify names for option arguments when usage text
is generated for a command.  sh based commands should be able to do the
same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr 
---
 Documentation/git-rev-parse.txt |   11 +--
 builtin/rev-parse.c |   17 -
 t/t1502-rev-parse-parseopt.sh   |   20 
 3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
 
 'git rev-parse --parseopt' input format is fully text based. It has two parts,
 separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.
 The lines after the separator describe the options.
 
 Each line of options has this format:
 
 
-* SP+ help LF
+*? SP+ help LF
 
 
 ``::
@@ -313,6 +313,12 @@ Each line of options has this format:
 
* Use `!` to not make the corresponding negated long option available.
 
+``::
+   ``, if specified, is used as a name of the argument, if the
+   option takes an argument. `` is terminated by the first
+   whitespace. Angle braces are added automatically.  Underscore symbols
+   are replaced with spaces.
+
 The remainder of the line, after stripping the spaces, is used
 as the help associated to the option.
 
@@ -333,6 +339,7 @@ h,helpshow the help
 
 foo   some nifty option --foo
 bar=  some cool option --bar with an argument
+baz=arg   another cool option --baz with an argument named 
 
   An option group Header
 C?option C with an optional argument"
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 45901df..7a58404 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
usage[unb++] = strbuf_detach(&sb, NULL);
}
 
-   /* parse: (|,|)[=?]? SP+  */
+   /* parse: (|,|)[*=?!]*? SP+  */
while (strbuf_getline(&sb, stdin, '\n') != EOF) {
const char *s;
+   const char *argh;
struct option *o;
 
if (!sb.len)
@@ -419,6 +420,20 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
o->value = &parsed;
o->flags = PARSE_OPT_NOARG;
o->callback = &parseopt_dump;
+
+   /* Possible argument name hint */
+   argh = s;
+   while (s > sb.buf && strchr("*=?!", s[-1]) == NULL)
+   --s;
+   if (s != sb.buf && s != argh) {
+   char *a;
+   o->argh = a = xmemdupz(s, argh - s);
+   while (a = strchr(a, '_'))
+   *a = ' ';
+   }
+   if (s == sb.buf)
+   s = argh;
+
while (s > sb.buf && strchr("*=?!", s[-1])) {
switch (*--s) {
case '=':
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 83b1300..bf0db05 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -18,6 +18,17 @@ An option group Header
 -C[...]   option C with an optional argument
 -d, --data[=...]  short and long option with an optional argument
 
+Argument hints
+-b   short option required argument
+--bar2   long option required argument
+-e, --fuz 
+  short and long option required argument
+-s[]short option optional argument
+--long[=]   long option optional argument
+-g, --fluf[=]   short and long option optional argument
+--longest 
+  a very long argument hint
+
 Extras
 --extra1  line above used to cause a segfault but no longer 
does
 
@@ -39,6 +50,15 @@ b,baz a short and long option
 C?option C with an optional argument
 d,data?   short and long option with an optional argument
 
+ Argument hints
+b=arg short option required argument
+bar2=arg  long option required argument
+e,fuz=with_spaces  short and long option required argument
+s?someshort option optional argument
+long?data long option optional argument
+g,fluf?path short and long option optional argument
+longest=a_very_long_argument_hint  a very long argument hint
+
 Extras
 extra1line above used to cause a segfault but no longer does
 EOF
-- 
1.7.9

--
To unsubscribe from this list

Re: [PATCH] rev-parse --parseopt: option argument name hints

2014-03-09 Thread Ilya Bobyr

On 3/4/2014 11:22 AM, Junio C Hamano wrote:

Ilya Bobyr  writes:


Built-in commands can specify names for option arguments, that are shown
when usage text is generated for the command.  sh based commands should
be able to do the same.

Option argument name hint is any text that comes after [*=?!] after the
argument name up to the first whitespace.  Underscores are replaced with
whitespace.  It is unlikely that an underscore would be useful in the
hint text.

Signed-off-by: Ilya Bobyr 
---
  Documentation/git-rev-parse.txt |   11 +--
  builtin/rev-parse.c |   17 -
  t/t1502-rev-parse-parseopt.sh   |   20 
  3 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 0d2cdcd..4cb6e02 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -284,13 +284,13 @@ Input Format
  
  'git rev-parse --parseopt' input format is fully text based. It has two parts,

  separated by a line that contains only `--`. The lines before the separator
-(should be more than one) are used for the usage.
+(could be more than one) are used for the usage.

Good spotting.  I think the original author meant to say there
should be at least one line to serve as the usage string, so
updating it to "should be one or more" may be more accurate, but
"could be more than one" would also work.


Changed to "should be one or more".


  The lines after the separator describe the options.
  
  Each line of options has this format:
  
  

-* SP+ help LF
+*? SP+ help LF
  
  
  ``::

@@ -313,6 +313,12 @@ Each line of options has this format:
  
  	* Use `!` to not make the corresponding negated long option available.
  
+``::

+   ``, if specified, is used as a name of the argument, if the
+   option takes an argument. `` is terminated by the first
+   whitespace. Angle braces are added automatically.  Underscore symbols
+   are replaced with spaces.

I had a hard time understanding this "Angle brackets are added
automatically" one (obviously nobody wants extra angle brackets
added around option arguments given by the user), until I looked at
the addition of the test to realize that this description is only
about how it appears in the help output.  The description needs to
be clarified to avoid confusion.


I've reworded some of the sentences.  I think it is better now.  Let me 
know what you think.



@@ -333,6 +339,7 @@ h,helpshow the help
  
  foo   some nifty option --foo

  bar=  some cool option --bar with an argument
+baz=arg   another cool option --baz with an argument named 

It probably is better not to have " named " at the end here, as
that gives an apparent-but-false contradiction with the "Angle
brackets are added *automatically*" and confuse readers.  At least,
it confused _this_ reader.


I am not sure I understand what is confusing here.  But I removed the " 
named " part.
If there would be an example, I think, it is easy to understand how it 
works.



After the "eval" in the existing example to parse the "$@" argument
list in this part of the documentation, it may be a good idea to say
something like:

The above command, when "$@" is "--help", produces the
following help output:

... sample output here ...

to show the actual output.  That way, we can illustrate how input
"baz?arg description of baz" is turned into "--baz[=]" output
clearly (yes, I am suggesting to use '?' in the new example, not '='
whose usage is already shown in the existing example).


Documentation on the whole argument parsing is quite short, so, I 
though, adding an example just to show how usage is generated would look 
like I am trying to make this feature look important than it is :)


I've added another section that shows usage text generated for the 
example specification.



diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index aaeb611..83a769e 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -395,9 +395,10 @@ static int cmd_parseopt(int argc, const char **argv, const 
char *prefix)
usage[unb++] = strbuf_detach(&sb, NULL);
}
  
-	/* parse: (|,|)[=?]? SP+  */

+   /* parse: (|,|)[*=?!]*? SP+  */
while (strbuf_getline(&sb, stdin, '\n') != EOF) {
const char *s;
+   const char *argh;

Let's spell that variable name out, e.g. arg_hint or something.


I was looking at the surrounding code for some style guidance, but most 
local variables have short names like "s", "o", "onb", "osz", "sb".

There are some that are longer.  So I was quite unsure here.
At the same time the target structure that holds the option description 
calls this string "argh".
Also, this is not really an "arg_hint" but the end of it.  Argument name 
is actually between s and argh, if there is some.

Considering all that, "argh" seemed like an OK name.

I've renamed it t

[PATCH] [GSOC] branch.c: install_branch_config: simplify if chain

2014-03-09 Thread Adam
Simplify if chain in install_branch_config().

Signed-off-by: Adam 
---
 branch.c |   46 +++---
 1 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/branch.c b/branch.c
index 723a36b..b2d59f1 100644
--- a/branch.c
+++ b/branch.c
@@ -77,29 +77,29 @@ void install_branch_config(int flag, const char *local, 
const char *origin, cons
strbuf_release(&key);
 
if (flag & BRANCH_CONFIG_VERBOSE) {
-   if (remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote branch %s 
from %s by rebasing.") :
- _("Branch %s set up to track remote branch %s 
from %s."),
- local, shortname, origin);
-   else if (remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local branch %s 
by rebasing.") :
- _("Branch %s set up to track local branch 
%s."),
- local, shortname);
-   else if (!remote_is_branch && origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track remote ref %s by 
rebasing.") :
- _("Branch %s set up to track remote ref %s."),
- local, remote);
-   else if (!remote_is_branch && !origin)
-   printf_ln(rebasing ?
- _("Branch %s set up to track local ref %s by 
rebasing.") :
- _("Branch %s set up to track local ref %s."),
- local, remote);
-   else
-   die("BUG: impossible combination of %d and %p",
-   remote_is_branch, origin);
+   if (remote_is_branch) {
+   if (origin)
+   printf_ln(rebasing ?
+ _("Branch %s set up to track remote 
branch %s from %s by rebasing.") :
+ _("Branch %s set up to track remote 
branch %s from %s."),
+ local, shortname, origin);
+   else
+   printf_ln(rebasing ?
+ _("Branch %s set up to track local 
branch %s by rebasing.") :
+ _("Branch %s set up to track local 
branch %s."),
+ local, shortname);
+   } else {
+   if (origin)
+   printf_ln(rebasing ?
+ _("Branch %s set up to track remote 
ref %s by rebasing.") :
+ _("Branch %s set up to track remote 
ref %s."),
+ local, remote);
+   else
+   printf_ln(rebasing ?
+ _("Branch %s set up to track local 
ref %s by rebasing.") :
+ _("Branch %s set up to track local 
ref %s."),
+ local, remote);
+   }
}
 }
 
-- 
1.7.1

--
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: howto to run git without a master branch

2014-03-09 Thread Carlos Pereira


There is a "git remote set-head" to manipulate HEAD in a remote 
repository.
Thanks, that is useful (like git symbolic-ref HEAD master-x suggested by 
Kevin, much better than editing the text file)

I agree that this might be viewed as a user experience issue.
But I can not come up with a possible solution.  Can you?
The branch where the user was when he/she pushed the repo to the server? 
it would be his/her responsibility to checkout the proper branch before 
pushing... if that was not good enough than he/she could always use git 
remote set-head... my point is: pointing to a bad branch that exists 
seems better than pointing to a branch that does not exist...


Anyway I agree this is not important, as we can easily change HEAD.

I do not think there should be any issues.

Thank you very much for your answers,

Carlos

--
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] Documentation: Say that submodule clones use a separate gitdirs.

2014-03-09 Thread Andrew Keller
On Mar 7, 2014, at 7:50 PM, Henri GEIST wrote:
> Le vendredi 07 mars 2014 à 15:37 -0800, Junio C Hamano a écrit :
>> Henri GEIST  writes:
>> 
>>> This information is technical in nature but has some importance for general 
>>> users.
>>> As this kind of clone have a separate gitdir, you will have a surprise if 
>>> you
>>> copy past the worktree as the gitdir will not come together.
>> 
>> I am not sure if I understand exactly what you are trying to say.
>> Are you saying that you had a submodule at "sub/dir" in your working
>> tree, and then "mkdir ../another && cp -R sub/dir ../another" did
>> not result in a usable Git working tree in ../another directory?
>> 
>> It is almost like complaining that "mkdir ../newone && cp -R * ../newone/"
>> did not result in a usable git repository in ../newone directory and
>> honestly speaking, that sounds borderline insane, I'd have to say.
>> 
>> Yes, if a user knows what she is doing, she should be able to make
>> something like that work, without running "git clone" (which is
>> probably the way most users would do it).  And yes, it would be good
>> to let the user learn from the documentation enough so that she
>> "knows what she is doing".  But no, I do not think end-user facing
>> documentation for "git-submodule" subcommand is the way to do that.
>> 
>> That is why I suggested repository-layout as potentially a better
>> alternative location.
>> 
>> But perhaps I am mis-reading your rationale.
>> 
>> 
> 
> Let me rephrase my example :
> 
> To give one of my project to someone else I have copied it on a USB key.
> By a simple drag and drop with the mouse.
> And I am quite sure I am not alone doing this way.
> 
> I have done those kind of things lot of time without any problem.
> But that day 'the_project' happened to be a submodule cloned by
> 'git submodule update' then on the USB key the $GIT_DIR of 'the_project'
> was missing.
> 
> If 'man git-submodule' have made me aware of the particularities of submodules
> clone I had write in a terminal:
> 
> git clone the_project /media/usb/the_project
> 
> Or at least I had understand what happened quicker.
> 
> I have nothing against also adding something in repository-layout but I am
> pretty sure normal users never read repository-layout as it is not a command
> they use. And it is not mentioned in most tutorials.

How about something like this:

"The git directory of a submodule lives inside the git directory of the parent 
repository instead of within the working directory."

I'm not sure where to put it, though.

 - Andrew Keller

--
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: howto to run git without a master branch

2014-03-09 Thread Ilya Bobyr

On 3/9/2014 12:54 PM, Carlos Pereira wrote:

On 03/09/2014 07:46 AM, Torsten Bögershausen wrote:
After creating a local repository with these two branches, and a 
server repository with git init --bare, and pushing the two branches:

>  >  git remote add originfoo@bar:~/path/test.git
>  git push origin master-g
>  git push origin master-x
>  >  everything seems fine, but cloning:
>  git clonefoo@bar:~/path/test.git
>  terminates with a warning: remote HEAD refers to nonexistent ref, 
unable to checkout.

This is because Git is trying to be nice:
When you clone, it tries to checkout a branch for you.

What happens when you only have 1 branch, lets say master-x?
If I clone the bare repo here, with only 1 branch, this branch
is automatically checked out (tested on 1.8.5.2)

What happens when you have 2 branches on the server?
Git really can not make a decision which one is the right one to 
check out for
you, so if you have 2 branched like "master" and "develop", it checks 
out the

"master" branch for you.

But if you have "master-x" and "master-g" then Git has no clue, which 
one could

be you favorite one (and neither have I)
The problem is on the server repo, the cloned repo is just a 
consequence. After initializing the server repo and pushing two 
branches master-g and master-x there is no master branch. Therefore 
the HEAD file should not point to a master branch that does not exist: 
ref: refs/heads/master


It could point to master-g (the first branch to be pushed) or master-x 
(the last branch to be pushed), depending of the criterion used by 
git, but pointing to something that does not exist seems weird and is 
the cause of the further complaints when the whole repository is 
cloned...


There is a "git remote set-head" to manipulate HEAD in a remote repository.

The fact that it is not set when you push master-x and master-y could be 
justified to some extent.
I think that this only applies to a case when you are pushing into a 
repository that have no branches.
Consider that you do not have to push only one branch, you may push any 
set of branches you want.  And I do not think that branches in the set 
have any meaningful order.
Also, your local HEAD does not necessary point to the "most reasonable 
branch to checkout when you clone".  It is just your current branch.
Now the questions is, how should the remote repository determine what to 
change HEAD to?


I agree that this might be viewed as a user experience issue.
But I can not come up with a possible solution.  Can you?

I forgot to say that the git version is 1.7.2.5 in both the initial 
repo and the server repo (probably this issue was fixed in newer 
versions?)


As I said, editing directly the HEAD text file on the server, and 
replacing master by master-g (or master-x) seems to solve the problem. 
My question is: is that enough? or shall I expect further issues down 
the road?


I do not think there should be any issues.
Basically if your repository is not using master as the "most reasonable 
branch to checkout when you clone" you need to do "git remote set-head" 
when you are preparing the shared repository.

I guess that in most setups there will be just one shared repository.
--
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: howto to run git without a master branch

2014-03-09 Thread Kevin
HEAD on the remote repo is indeed used to determine what to check out
when cloning. It's quite normal to change it to anything you like. To
change it, you usually use git symbolic-ref HEAD master-x instead of
directly editing that file.

On Sun, Mar 9, 2014 at 8:54 PM, Carlos Pereira
 wrote:
> On 03/09/2014 07:46 AM, Torsten Bögershausen wrote:
>>>
>>> After creating a local repository with these two branches, and a server
>>> repository with git init --bare, and pushing the two branches:
>>> >  >  git remote add originfoo@bar:~/path/test.git
>>>
>>> >  git push origin master-g
>>> >  git push origin master-x
>>> >  >  everything seems fine, but cloning:
>>> >  git clonefoo@bar:~/path/test.git
>>>
>>> >  terminates with a warning: remote HEAD refers to nonexistent ref,
>>> > unable to checkout.
>>>
>>
>> This is because Git is trying to be nice:
>> When you clone, it tries to checkout a branch for you.
>>
>> What happens when you only have 1 branch, lets say master-x?
>> If I clone the bare repo here, with only 1 branch, this branch
>> is automatically checked out (tested on 1.8.5.2)
>>
>> What happens when you have 2 branches on the server?
>> Git really can not make a decision which one is the right one to check out
>> for
>> you, so if you have 2 branched like "master" and "develop", it checks out
>> the
>> "master" branch for you.
>>
>> But if you have "master-x" and "master-g" then Git has no clue, which one
>> could
>> be you favorite one (and neither have I)
>>
>
> The problem is on the server repo, the cloned repo is just a consequence.
> After initializing the server repo and pushing two branches master-g and
> master-x there is no master branch. Therefore the HEAD file should not point
> to a master branch that does not exist: ref: refs/heads/master
>
> It could point to master-g (the first branch to be pushed) or master-x (the
> last branch to be pushed), depending of the criterion used by git, but
> pointing to something that does not exist seems weird and is the cause of
> the further complaints when the whole repository is cloned...
>
> I forgot to say that the git version is 1.7.2.5 in both the initial repo and
> the server repo (probably this issue was fixed in newer versions?)
>
> As I said, editing directly the HEAD text file on the server, and replacing
> master by master-g (or master-x) seems to solve the problem. My question is:
> is that enough? or shall I expect further issues down the road?
>
> Thank you very much,
>
> Carlos
>
>> What does "git branch" say?
>> (I think nothing)
>> What does "git branch -r" say?
>> (I think "origin/master-g" and "origin/master-x")
>>
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: howto to run git without a master branch

2014-03-09 Thread Carlos Pereira

On 03/09/2014 07:46 AM, Torsten Bögershausen wrote:

After creating a local repository with these two branches, and a server 
repository with git init --bare, and pushing the two branches:
>  
>  git remote add originfoo@bar:~/path/test.git

>  git push origin master-g
>  git push origin master-x
>  
>  everything seems fine, but cloning:

>  git clonefoo@bar:~/path/test.git
>  terminates with a warning: remote HEAD refers to nonexistent ref, unable to 
checkout.
 

This is because Git is trying to be nice:
When you clone, it tries to checkout a branch for you.

What happens when you only have 1 branch, lets say master-x?
If I clone the bare repo here, with only 1 branch, this branch
is automatically checked out (tested on 1.8.5.2)

What happens when you have 2 branches on the server?
Git really can not make a decision which one is the right one to check out for
you, so if you have 2 branched like "master" and "develop", it checks out the
"master" branch for you.

But if you have "master-x" and "master-g" then Git has no clue, which one could
be you favorite one (and neither have I)
   
The problem is on the server repo, the cloned repo is just a 
consequence. After initializing the server repo and pushing two branches 
master-g and master-x there is no master branch. Therefore the HEAD file 
should not point to a master branch that does not exist: ref: 
refs/heads/master


It could point to master-g (the first branch to be pushed) or master-x 
(the last branch to be pushed), depending of the criterion used by git, 
but pointing to something that does not exist seems weird and is the 
cause of the further complaints when the whole repository is cloned...


I forgot to say that the git version is 1.7.2.5 in both the initial repo 
and the server repo (probably this issue was fixed in newer versions?)


As I said, editing directly the HEAD text file on the server, and 
replacing master by master-g (or master-x) seems to solve the problem. 
My question is: is that enough? or shall I expect further issues down 
the road?


Thank you very much,

Carlos

What does "git branch" say?
(I think nothing)
What does "git branch -r" say?
(I think "origin/master-g" and "origin/master-x")

   


--
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: Trust issues with hooks and config files

2014-03-09 Thread Julian Brost
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 07.03.2014 22:04, Jeff King wrote:
> Yes, this is a well-known issue. The only safe operation on a
> repository for which somebody else controls hooks and config is to
> fetch from it (upload-pack on the remote repository does not
> respect any dangerous config or hooks).

I'm a little bit surprised that you and some other people I asked see
this as such a low-priority problem as this makes social engineering
attacks on multi-user systems, like they are common at universities,
really easy (this is also how I noticed the problem).

> It has been discussed, but nobody has produced patches. I think
> that nobody is really interested in doing so because:
> 
> 1. It introduces complications into previously-working setups
> (e.g., a daemon running as "nobody" serving repos owned by a "git"
> user needs to mark "git" as trusted).
> 
> 2. In most cases, cross-server boundaries provide sufficient 
> insulation (e.g., I might not push to an evil person's repo, but
> rather to a shared repo whose hooks and config are controlled by
> root on the remote server).
> 
> If you want to work on it, I think it's an interesting area. But
> any development would need to think about the transition plan for
> existing sites that will be broken.

I can understand the problem with backward compatibility but in my
opinion the default behavior should definitely be to ignore untrusted
config files and hooks as it would otherwise only protect users that
are already aware of the issue anyways and manually enable this option.

Are there any plans for some major release in the future that would
allow introducing backward incompatible changes?

I would definitely spend some time working on a patch but so far I
have no idea of git's internals and never looked at the source before.

> At the very least, the current trust model could stand to be
> documented much better (I do not think the rule of "fetching is
> safe, everything else is not" is mentioned anywhere explicitly).

Good point but not enough in my opinion as I haven't read every git
manpage before running it for the first time.
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)

iQIcBAEBCAAGBQJTHKRfAAoJECLcYT6QIdBtGdkQALD24YhS2aHBzi/c6a/1+eOC
xERUshDJvU/3picpTqqwFlqHB6X53vo7uXcCmJU8oaMsGLeLrTyrX77YOzrhlkuK
kERkXGPgyoW1G4enATSURDbjW6kp7SG0lRvGTPNySqDt9FiZYTDP7CGsQRDDl8cL
al50BoNFsx9K/kiPgbDJsenDi/MAclAYlHbHJnEB6aBo06G89zwC2tECFtXewnAD
EbCKPI4tFrUZW/rWxHAEDVs+cI038nMzSNMi7I+HAMG48s+iMfFF69pkdOQjhIsc
3irisLQcKPVNRjSK/dGEKbqkAy9wziNza69tl0EgQn6ewju5NZ4xbAkWQG9LEWfZ
Ux1safkumQsAKiYfn87t5YDXZ3vDYKbChKQv/UlicobVRm0YbhitY2AQAzu+wx1V
mXmG6D91IxfR4B0+AA+3/E8huSD4JJ2laIUwIoYV+y4+ZAlxnT4cNdYiYAH4oEme
wZ9R0wsxVfUm+uFdSBqsgEpv7Bp9PRcREuVXXz0GQH0wQ8zdwOeeNvA3v6ZtodRZ
0q7WOVJpd/3j4fQoWUXFAOZxDIZVorM0dQQvbhXiwOE9UY5Gwpq22lGKHM1t+8EO
lOUCQBXjscfPgyLifdSbnaf11RGgVLERQlpz6hpEGht1J3AqF6tTZPjto+iVV97v
nObYVtMXsAtrKbka2FF+
=ps1M
-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/RFC] rebase: new convenient option to edit/reword/delete a single commit

2014-03-09 Thread Matthieu Moy
Nguyễn Thái Ngọc Duy  writes:

>  Documentation/git-rebase.txt | 11 +++
>  git-rebase--interactive.sh   | 17 ++---
>  git-rebase.sh| 22 +-
>  3 files changed, 46 insertions(+), 4 deletions(-)

This would deserve a few tests ...

>  'git rebase' --continue | --skip | --abort | --edit-todo
> +'git rebase' [--edit | -E | --reword | -R | --delete | -D ] 
[...]
> +-E=::
> +--edit=::
> +-R=::
> +--reword=::
> +-D=::
> +--delete=::

I first thought the two versions were inconsistant, but they're
technically correct since the current patch allows only one instance of
the option. I find it a bit weird to have two different descriptions,
but that may be just me.

> + Configuration variable `rebase.autostash` is
> + ignored.

I first found it really strange, and then understood it's a typo.

s/autostash/autosquash/

;-)

-- 
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: [PATCH v5 28/28] count-objects: report unused files in $GIT_DIR/repos/...

2014-03-09 Thread Duy Nguyen
On Sun, Mar 9, 2014 at 3:21 PM, Eric Sunshine  wrote:
> On Fri, Mar 7, 2014 at 9:48 PM, Nguyễn Thái Ngọc Duy  
> wrote:
>> In linked checkouts, borrowed parts like config is taken from
>> $GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as
>> garbage.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>> diff --git a/path.c b/path.c
>> index ddb5962..5a7dc45 100644
>> --- a/path.c
>> +++ b/path.c
>> @@ -4,6 +4,7 @@
>>  #include "cache.h"
>>  #include "strbuf.h"
>>  #include "string-list.h"
>> +#include "dir.h"
>>
>>  static int get_st_mode_bits(const char *path, int *mode)
>>  {
>> @@ -91,9 +92,9 @@ static void replace_dir(struct strbuf *buf, int len, const 
>> char *newdir)
>>  }
>
> Do you want to add a comment here explaining what the "!" prefix on
> some entries means, or is it sufficiently self-evident to anyone
> looking at the consumers of this array?

I was hoping it was clear from the patch how this "!" was used (or "/"
in the patch that introduces common_list[]). But if any reader thinks
otherwise, I'm happy to add a comment.
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()

2014-03-09 Thread karthik nayak
Hey Eric,

Its been nice learning from you about how to submit patches to git.
was a nice learning curve, now I'm looking into the ideas and will contact the
appropriate mentor soon with a plan.

Thanks
- Karthik

On Sun, Mar 9, 2014 at 1:19 PM, Eric Sunshine  wrote:
> On Fri, Mar 7, 2014 at 5:49 AM, karthik nayak  wrote:
>> Hello Eric,
>> Thanks for your reply, and for that information.  should i patch again or
>> this should do?
>> And what next? Talk to the mentor?
>
> The ultimate authority deciding if a patch is ready is Junio, as it
> would have to be accepted into his tree. Since he already accepted a
> similar patch from a different potential GSoC applicant, it may not
> make sense to refine this one further. What is important is that you
> are now familiar with the review process on this project, and the
> mentors (hopefully) have gained insight into your abilities and how
> you interact with reviewers (which was the goal of these
> microprojects).
>
> Probably best at this point is to consider a proposed project [1] or
> choose your own, and start the task of applying for a GSoC position
> (by whatever means that is done).
>
> [1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Ideas.md
>
>> On Fri, Mar 7, 2014 at 3:04 PM, Eric Sunshine 
>> wrote:
>>>
>>> On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak 
>>> wrote:
>>> > Replace 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. And also manages to do, what the current
>>> > code does in two steps.
>>>
>>> Better. Thanks.
>>>
>>> > Signed-off-by: Karthik Nayak 
>>> > ---
>>> > Hello Eric,
>>> > In this patch, I have:
>>> > 1. Fixed the improper placement of buf_date , initialised to a NULL
>>> > string.
>>> > 2. Fixed whitespace.
>>> > 3. Better naming as per your suggestion.
>>> > 4. Fixed the initilisation before the if statement.
>>> > Thanks for your suggestion.
>>> > ---
>>> >  commit.c | 22 +++---
>>> >  1 file changed, 11 insertions(+), 11 deletions(-)
>>> >
>>> > diff --git a/commit.c b/commit.c
>>> > index 6bf4fe0..4144e00 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_date;
>>> >
>>> > if (!commit->buffer) {
>>> > unsigned long size;
>>> > @@ -565,15 +566,15 @@ static void record_author_date(struct
>>> > author_date_slab *author_date,
>>> > for (buf = commit->buffer ? commit->buffer : buffer;
>>> >  buf;
>>> >  buf = line_end + 1) {
>>> > +   buf_date = skip_prefix(buf, "author ");
>>>
>>> The data after "author " is identification information (name, email),
>>> not date. In fact, this information gets passed to the function
>>> split_ident_line(), so a better name for this variable is 'ident_line'
>>> (but not the misspelling 'indent_line' from one of your earlier
>>> attempts).
>>>
>>> > line_end = strchrnul(buf, '\n');
>>> > -   if (!starts_with(buf, "author ")) {
>>> > +   if (!buf_date) {
>>> > 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_date,
>>> > +line_end - buf_date) ||
>>> > !ident.date_begin || !ident.date_end)
>>> > goto fail_exit; /* malformed "author" line */
>>> > break;
>>> > @@ -1098,6 +1099,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 *gpg_sig;
>>> >
>>> > if (!buffer || type != OBJ_COMMIT)
>>> > goto cleanup;
>>> > @@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,
>>> > next = next ? next + 1 : tail;
>>> > 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 ((gpg_sig = skip_prefix(line, gpg_sig_header))
>>> > + && gpg_sig[0] == ' ')
>>> > +

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

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:48 PM, Nguyễn Thái Ngọc Duy  wrote:
> In linked checkouts, borrowed parts like config is taken from
> $GIT_COMMON_DIR. $GIT_DIR/config is never used. Report them as
> garbage.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/path.c b/path.c
> index ddb5962..5a7dc45 100644
> --- a/path.c
> +++ b/path.c
> @@ -4,6 +4,7 @@
>  #include "cache.h"
>  #include "strbuf.h"
>  #include "string-list.h"
> +#include "dir.h"
>
>  static int get_st_mode_bits(const char *path, int *mode)
>  {
> @@ -91,9 +92,9 @@ static void replace_dir(struct strbuf *buf, int len, const 
> char *newdir)
>  }

Do you want to add a comment here explaining what the "!" prefix on
some entries means, or is it sufficiently self-evident to anyone
looking at the consumers of this array?

>  static const char *common_list[] = {
> -   "/branches", "/hooks", "/info", "/logs", "/lost-found", "/modules",
> +   "/branches", "/hooks", "/info", "!/logs", "/lost-found", "/modules",
> "/objects", "/refs", "/remotes", "/repos", "/rr-cache", "/svn",
> -   "config", "gc.pid", "packed-refs", "shallow",
> +   "config", "!gc.pid", "packed-refs", "shallow",
> NULL
>  };
>
> @@ -107,6 +108,8 @@ static void update_common_dir(struct strbuf *buf, int 
> git_dir_len)
> for (p = common_list; *p; p++) {
> const char *path = *p;
> int is_dir = 0;
> +   if (*path == '!')
> +   path++;
> if (*path == '/') {
> path++;
> is_dir = 1;
> @@ -122,6 +125,28 @@ static void update_common_dir(struct strbuf *buf, int 
> git_dir_len)
> }
>  }
>
> +void report_linked_checkout_garbage(void)
> +{
> +   struct strbuf sb = STRBUF_INIT;
> +   const char **p;
> +   int len;
> +
> +   if (!git_common_dir_env)
> +   return;
> +   strbuf_addf(&sb, "%s/", get_git_dir());
> +   len = sb.len;
> +   for (p = common_list; *p; p++) {
> +   const char *path = *p;
> +   if (*path == '!')
> +   continue;
> +   strbuf_setlen(&sb, len);
> +   strbuf_addstr(&sb, path);
> +   if (file_exists(sb.buf))
> +   report_garbage("unused in linked checkout", sb.buf);
> +   }
> +   strbuf_release(&sb);
> +}
> +
>  static void adjust_git_path(struct strbuf *buf, int git_dir_len)
>  {
> const char *base = buf->buf + git_dir_len;
> --
> 1.9.0.40.gaa8c3ea
--
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 v5 25/28] prune: strategies for linked checkouts

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:48 PM, Nguyễn Thái Ngọc Duy  wrote:
> (alias R=$GIT_COMMON_DIR/repos/)
>
>  - linked checkouts are supposed to keep its location in $R/gitdir up
>to date. The use case is auto fixup after a manual checkout move.
>
>  - linked checkouts are supposed to update mtime of $R/gitdir. If
>$R/gitdir's mtime is older than a limit, and it points to nowhere,
>repos/ is to be pruned.
>
>  - If $R/locked exists, repos/ is not supposed to be pruned. If
>$R/locked exists and $R/gitdir's mtime is older than a really long
>limit, warn about old unused repo.
>
>  - "git checkout --to" is supposed to make a hard link named $R/link
>pointing to the .git file on supported file systems to help detect
>the user manually deleting the checkout. If $R/link exists and its
>link count is greated than 1, the repo is kept.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 598b43d..9dc80f1 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -899,12 +899,22 @@ static int prepare_linked_checkout(const struct 
> checkout_opts *opts,
> junk_git_dir = sb_repo.buf;
> is_junk = 1;
>
> +   /*
> +* lock the incomplete repo so prunt won't delete it, unlock

s/prunt/prune/

> +* after the preparation is over.
> +*/
> +   strbuf_addf(&sb, "%s/locked", sb_repo.buf);
> +   write_file(sb.buf, 1, "initializing\n");
> +
> strbuf_addf(&sb_git, "%s/.git", path);
> if (safe_create_leading_directories_const(sb_git.buf))
> die_errno(_("could not create leading directories of '%s'"),
>   sb_git.buf);
> junk_work_tree = path;
>
> +   strbuf_reset(&sb);
> +   strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
> +   write_file(sb.buf, 1, "%s\n", real_path(sb_git.buf));
> write_file(sb_git.buf, 1, "gitdir: %s/repos/%s\n",
>real_path(get_git_common_dir()), name);
> /*
--
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 v5 06/28] Make git_path() aware of file relocation in $GIT_DIR

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:47 PM, Nguyễn Thái Ngọc Duy  wrote:
> We allow the user to relocate certain paths out of $GIT_DIR via
> environment variables, e.g. GIT_OBJECT_DIRECTORY, GIT_INDEX_FILE and
> GIT_GRAFT_FILE. Callers are not supposed to use git_path() or
> git_pathdup() to get those paths. Instead they must use
> get_object_directory(), get_index_file() and get_graft_file()
> respectively. This is inconvenient and could be missed in review (for
> example, there's git_path("objects/info/alternates") somewhere in
> sha1_file.c).
>
> This patch makes git_path() and git_pathdup() understand those
> environment variables. So if you set GIT_OBJECT_DIRECTORY to /foo/bar,
> git_path("objects/abc") should return /foo/bar/abc. The same is done
> for the two remaining env variables.
>
> "git rev-parse --git-path" is the wrapper for script use.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  Documentation/git-rev-parse.txt |  7 ++
>  builtin/rev-parse.c |  7 ++
>  cache.h |  1 +
>  environment.c   |  9 ++--
>  path.c  | 49 
> +++--
>  t/t0060-path-utils.sh   | 19 
>  6 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 0d2cdcd..46020d9 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -232,6 +232,13 @@ print a message to stderr and exit with nonzero status.
> repository.  If  is a gitfile then the resolved path
> to the real repository is printed.
>
> +--git-path ::
> +   Resolve "$GIT_DIR/" and takes other path relocation
> +   variables such as $GIT_OBJECT_DIRECTORY,
> +   $GIT_INDEX_FILE... into account. For example, if
> +   $GIT_OBJECT_DIRECTORY is set to /foo/bar then "git rev-parse
> +   --git-path objects/abc" returns /tmp/bar/abc.

s/tmp/foo/

> +
>  --show-cdup::
> When the command is invoked from a subdirectory, show the
> path of the top-level directory relative to the current
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index aaeb611..e50bc65 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -518,6 +518,13 @@ int cmd_rev_parse(int argc, const char **argv, const 
> char *prefix)
> for (i = 1; i < argc; i++) {
> const char *arg = argv[i];
>
> +   if (!strcmp(arg, "--git-path")) {
> +   if (!argv[i + 1])
> +   die("--git-path requires an argument");
> +   puts(git_path("%s", argv[i + 1]));
> +   i++;
> +   continue;
> +   }
> if (as_is) {
> if (show_file(arg, output_prefix) && as_is < 2)
> verify_filename(prefix, arg, 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 v5 00/28] Support multiple checkouts

2014-03-09 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 9:47 PM, Nguyễn Thái Ngọc Duy  wrote:
> The diff against v4 is kinda big but it's mostly about converting
> `...` to $(...) and making git_path() and friends return a const
> string.
>
> Another notable change is I no longer attempt to support checkouts on
> portable devices. Torsten pointed out (privately) that my dealing with
> Windows drives was insufficient. And Junio was not so happy with how
> link() was handled either. We can revisit it later.
>
> Many thanks to Eric, who was very patient to go through the series
> carefully and pointed out problems that I overlooked.
>
> v4..v5 diff below for convenience

Thanks for providing an interdiff between the two versions. It
simplified things considerably, especially with the v4 reviews still
relatively fresh in my brain.
--
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