Re: [PATCH v6 02/11] trailer: process trailers from stdin and arguments

2014-03-07 Thread Dan Carpenter
On Fri, Mar 07, 2014 at 07:19:15AM +0100, Christian Couder wrote:
 On Wed, Mar 5, 2014 at 11:52 PM, Junio C Hamano gits...@pobox.com wrote:
 
  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...
 
 Yeah, I don't know why, but these days I find it very hard to review
 style issues in my own code without being distracted.
 And by the way is there a good script to check them?

Many of these would have been caught with kernel.org's checkpatch.pl
script.

http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl

regards,
dan carpenter

--
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: [PATH] branch.c:install_branch_config():Simplify code generating verbose message.

2014-03-07 Thread Eric Sunshine
Thanks for the submission. Some comments below to give you a taste of
the Git review process...

On Thu, Mar 6, 2014 at 2:58 AM, Paweł Wawruch pa...@aleg.pl wrote:
 From adfcfa0a334378a6242347efc0d614fa193610db Mon Sep 17 00:00:00 2001
 From: =?UTF-8?q?Pawe=C5=82=20Wawruch?= pa...@aleg.pl
 Date: Thu, 6 Mar 2014 00:05:00 +0100

Drop these lines. They are extracted automatically by git am from
the headers of your mail message.

 Subject: [PATCH] branch.c:install_branch_config(): Simplify the long chain of
  if statements. Threre was a long chain of if statements. The code can be more
  clear. The chain is replaced with table of strings. New approach is more
  compact.

The Subject: also is extracted automatically from the email, so it
should not be pasted here. Instead, when you compose your commit
message, write a very concise summary of the patch as the very first
line, then a blank line, then the meat of the description using as
many lines as needed. Wrap the lines to about 65-70 characters. git
format-patch automatically uses that concise first line as the
Subject: of the mail message. This is all explained in section (2)
Describe your changes well of Documentation/SubmittingPatches.

Write in imperative tone. Rather than There was a long..., say
There is a long Rather than The chain is replaced..., say
Replace the chain

You misspelled There as Threre.

Your patch sign-off is missing. It should be placed right here above
the --- line. See Documentation/SubmittingPatches.

 ---

The Git GSoC page on which you found this microproject suggests
mentioning that this is a GSoC submission so that reviewers don't
overlook it. Either do so by adding [GSoC] to the subject or by
mentioning it right here as patch commentary after the --- line.

  branch.c | 38 --
  1 file changed, 16 insertions(+), 22 deletions(-)

 diff --git a/branch.c b/branch.c
 index 723a36b..ebc2172 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -77,29 +77,23 @@ void install_branch_config(int flag, const char *local,
 const char *origin, cons
 strbuf_release(key);

Your patch is whitespace damaged, and thus not easily applied. This
probably resulted from pasting it into your email client. Using git
send-email avoids this problem.

 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);
 +   const char *messages[8] ;

Drop the space before the semicolon.

On this project, we avoid intermixing declarations with code, so move
the declaration of messages[] to the top of the block.

 +   messages[0] = _(Branch %s set up to track remote branch %s 
 from %s by
 rebasing.);
 +   messages[1] = _(Branch %s set up to track remote branch %s 
 from %s.);
 +   messages[2] = _(Branch %s set up to track local branch %s by
 rebasing.);
 +   messages[3] = _(Branch %s set up to track local branch %s.);
 +   messages[4] = _(Branch %s set up to track remote ref %s by 
 rebasing.);
 +   messages[5] = _(Branch %s set up to track remote ref %s.);
 +   messages[6] = _(Branch %s set up to track local ref %s by 
 rebasing.);
 +   messages[7] = _(Branch %s set up to track local ref %s.);

Rather than hardcoding the array size and then assigning the elements
individually, perhaps you can construct the array with an initializer.
Use N_(...) rather than _(...).

const char *messages[] = {
N_(...),
...
};

 +   const char *name = remote_is_branch ? remote : shortname;
 +   int message_number = rebasing + 2 * (origin != NULL) + 4 *
 remote_is_branch;

Move the 

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

2014-03-07 Thread Eric Sunshine
On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak karthik@gmail.com 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 karthik@gmail.com
 ---
 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] == ' ')
 +   sig = gpg_sig + 1;

Other than the poor variable name 'buf_date' and the review comments
by Junio and Tanay that this particular change is of questionable
value and perhaps should be dropped, this version of the patch looks
reasonable.

Thanks.

 if (sig) {
 strbuf_add(signature, sig, next - sig);
 saw_signature = 1;
 @@ -1193,10 +1195,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


git merge --date --author

2014-03-07 Thread Yann Droneaud
Hi,

I was trying to rebuild some history and found myself trying to use some
options with git merge which are available in git commit:

 git merge \
 --date 2013-12-31 23:59:59 + \
 --author Happy New Year happy.new-year@gregorian.calendar \
 current-year

But unfortunately, unlike git commit, git merge doesn't support either
--date or --author.

I can work around with environment variables:

 GIT_AUTHOR_DATE=2013-12-31 23:59:59 + \
 GIT_AUTHOR_NAME=Happy New Year \
 GIT_AUTHOR_EMAIL=happy.new-year@gregorian.calendar  \
 git merge current-year

But I'd like to know if there's a specific reason for git merge to not
support --date and --author ?

Regards.

-- 
Yann Droneaud
OPTEYA


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


Re: git merge --date --author

2014-03-07 Thread Michael Haggerty
On 03/07/2014 11:30 AM, Yann Droneaud wrote:
 I was trying to rebuild some history and found myself trying to use some
 options with git merge which are available in git commit:
 
  git merge \
  --date 2013-12-31 23:59:59 + \
  --author Happy New Year happy.new-year@gregorian.calendar \
  current-year
 
 But unfortunately, unlike git commit, git merge doesn't support either
 --date or --author.
 
 I can work around with environment variables:
 
  GIT_AUTHOR_DATE=2013-12-31 23:59:59 + \
  GIT_AUTHOR_NAME=Happy New Year \
  GIT_AUTHOR_EMAIL=happy.new-year@gregorian.calendar  \
  git merge current-year

You can also use

git merge --no-commit [...]
git commit --date ... --author ...

 But I'd like to know if there's a specific reason for git merge to not
 support --date and --author ?

None that I can think of, except that nobody has implemented it.  Don't
forget that the merge invocation might not lead immediately to a commit
(if there is a conflict, or if --no-commit is used) so the metadata that
is passed to its command line would have to be tucked away safely
somewhere until the merge commit is finally ready to be created.

Michael

-- 
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: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()

2014-03-07 Thread karthik nayak
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?
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 v6 02/11] trailer: process trailers from stdin and arguments

2014-03-07 Thread Christian Couder
On Fri, Mar 7, 2014 at 9:08 AM, Dan Carpenter dan.carpen...@oracle.com wrote:
 On Fri, Mar 07, 2014 at 07:19:15AM +0100, Christian Couder wrote:

 Yeah, I don't know why, but these days I find it very hard to review
 style issues in my own code without being distracted.
 And by the way is there a good script to check them?

 Many of these would have been caught with kernel.org's checkpatch.pl
 script.

 http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/scripts/checkpatch.pl

Yeah, I already thought about using it in the past, but I was not sure
about its status regarding the git source code because it's use is not
suggested in CodingGuidelines, and the discussions about adding it
there went nowhere:

http://thread.gmane.org/gmane.comp.version-control.git/223698/focus=224653

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


Re: git merge --date --author

2014-03-07 Thread Andreas Schwab
Yann Droneaud ydrone...@opteya.com writes:

 But I'd like to know if there's a specific reason for git merge to not
 support --date and --author ?

It's rather unusual that a merge is performed on behalf of a different
author.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
And now for something completely different.
--
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

2014-03-07 Thread Paul Lotz
David,

I investigated further and found that \$LOCAL\ \$REMOTE\ return the remote 
and local files (reversed).  (One can easily see this in my 2/28 e-mail.)  
Reversing these (\$REMOTE\ \$LOCAL\) does indeed reverse the output.  It is 
easy to work around this issue, but how can this be?

Paul

-Original Message-
From: Paul Lotz [mailto:pl...@lsst.org] 
Sent: Wednesday, March 5, 2014 3:28 PM
To: 'David Aguilar'
Cc: 'Git Mailing List'
Subject: RE: difftool sends malformed path to exernal tool on Windows

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\M2AADTC:\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 absolute path to VI 1 ...

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\M2AADTC:\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


Confirm Receipt

2014-03-07 Thread Mrs.Supini Thrunkul

Hello,

I am Mrs.Supini Thrunkul from Tai Yau Bank Hong Kong,I need your cooperation  
to transfer $47.3 million US Dollars to any trusted account within your  
control.

Contact me for more details.  

Mrs.Supini Thrunkul
Tel: +85 2580 848 65
--
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] show_ident_date: fix always-false conditional

2014-03-07 Thread Jeff King
On Thu, Mar 06, 2014 at 08:35:24PM -0500, Eric Sunshine wrote:

 1dca155fe3fa (log: handle integer overflow in timestamps, 2014-02-24)
 assigns the result of strtol() to an 'int' and then checks it against
 LONG_MIN and LONG_MAX, indicating underflow or overflow, even though
 'int' may not be large enough to represent those values.
 
 On Mac, the compiler complains:
 
 warning: comparison of constant 9223372036854775807 with
   expression of type 'int' is always false
   [-Wtautological-constant-out-of-range-compare]
   if (tz == LONG_MAX || tz == LONG_MIN)
 
 Similarly for the LONG_MIN case. Fix this.

Yeah, this is definitely a potential bug. When I added the overflow
check, I blindly assumed that the existing code was at least using a
sufficiently large type to store the result of strtol, but it's not.

I don't think your fix catches all overflow, though:

 + else if (ident-tz_begin  ident-tz_end) {
 + errno = 0;
 + tz = strtol(ident-tz_begin, NULL, 10);
 + if (errno)

Errno will trigger if we overflowed a long, but then we assign the
result into an int, possibly truncating the result.

 Alternately, the result of strtol() could be assigned temporarily to a
 'long', compared against LONG_MIN and LONG_MAX, and then assigned to the
 'int' tz variable.

That catches overflow from strtol, but we'd then truncate when we pass
it as an int to show_date.

I think we want this instead:

-- 8 --
Subject: show_ident_date: fix tz range check

Commit 1dca155fe3fa (log: handle integer overflow in
timestamps, 2014-02-24) tried to catch integer overflow
coming from strtol() on the timezone field by comparing against
LONG_MIN/LONG_MAX. However, the intermediate tz variable
is an int, which means it can never be LONG_MAX on LP64
systems; we would truncate the output from strtol before the
comparison.

Clang's -Wtautological-constant-out-of-range-compare notices
this and rightly complains.

Let's instead store the result of strtol in a long, and then
compare it against INT_MIN/INT_MAX. This will catch overflow
from strtol, and also overflow when we pass the result as an
int to show_date.

Reported-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Jeff King p...@peff.net
---
 pretty.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pretty.c b/pretty.c
index 3b811ed..6e266dd 100644
--- a/pretty.c
+++ b/pretty.c
@@ -397,7 +397,7 @@ static const char *show_ident_date(const struct ident_split 
*ident,
   enum date_mode mode)
 {
unsigned long date = 0;
-   int tz = 0;
+   long tz = 0;
 
if (ident-date_begin  ident-date_end)
date = strtoul(ident-date_begin, NULL, 10);
@@ -406,7 +406,7 @@ static const char *show_ident_date(const struct ident_split 
*ident,
else {
if (ident-tz_begin  ident-tz_end)
tz = strtol(ident-tz_begin, NULL, 10);
-   if (tz == LONG_MAX || tz == LONG_MIN)
+   if (tz = INT_MAX || tz = INT_MIN)
tz = 0;
}
return show_date(date, tz, mode);
-- 
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: [RFC/PATCH 4/4] replace: add --edit option

2014-03-07 Thread Jeff King
On Thu, Mar 06, 2014 at 08:57:58PM -0500, Eric Sunshine wrote:

  +   if (strbuf_read(result, cmd.out, 41)  0)
  +   die_errno(unable to read from mktree);
  +   close(cmd.out);
  +
  +   if (finish_command(cmd))
  +   die(mktree reported failure);
  +   if (get_sha1_hex(result.buf, sha1)  0)
  +   die(mktree did not return an object name);
 
 strbuf_release(result);

Thanks for catching. I'll include it in any re-roll.

-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

2014-03-07 Thread Jeff King
On Fri, Mar 07, 2014 at 08:08:37AM +0100, Christian Couder wrote:

  Be it graft or replace, I do not think we want to invite people to
  use these mechansims too lightly to locally rewrite their history
  willy-nilly without fixing their mistakes at the object layer with
  commit --amend, rebase, bfg, etc. in the longer term.  So in
  that sense, adding a command to make it easy is not something I am
  enthusiastic about.
 
  On the other hand, if the user does need to use graft or replace
  (perhaps to prepare for casting the fixed history in stone with
  filter-branch), it would be good to help them avoid making mistakes
  while doing so and tool support may be a way to do so.
 
  So, ... I am of two minds.
 
 Maybe if we add a new command (or maybe a script) with a name long and
 cryptic-looking enough like git create-replacement-object it will
 scare casual users from touching it, while power users will be happy
 to benefit from it.

I do not think the features we are talking about are significantly more
dangerous than git replace is in the first place. If we want to make
people aware of the dangers, perhaps git-replace.1 is the right place to
do it.

-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] show_ident_date: fix always-false conditional

2014-03-07 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 12:15 PM, Jeff King p...@peff.net wrote:
 On Thu, Mar 06, 2014 at 08:35:24PM -0500, Eric Sunshine wrote:

 1dca155fe3fa (log: handle integer overflow in timestamps, 2014-02-24)
 assigns the result of strtol() to an 'int' and then checks it against
 LONG_MIN and LONG_MAX, indicating underflow or overflow, even though
 'int' may not be large enough to represent those values.

 On Mac, the compiler complains:

 warning: comparison of constant 9223372036854775807 with
   expression of type 'int' is always false
   [-Wtautological-constant-out-of-range-compare]
   if (tz == LONG_MAX || tz == LONG_MIN)

 Similarly for the LONG_MIN case. Fix this.

 Yeah, this is definitely a potential bug. When I added the overflow
 check, I blindly assumed that the existing code was at least using a
 sufficiently large type to store the result of strtol, but it's not.

 I don't think your fix catches all overflow, though:

 + else if (ident-tz_begin  ident-tz_end) {
 + errno = 0;
 + tz = strtol(ident-tz_begin, NULL, 10);
 + if (errno)

 Errno will trigger if we overflowed a long, but then we assign the
 result into an int, possibly truncating the result.

 Alternately, the result of strtol() could be assigned temporarily to a
 'long', compared against LONG_MIN and LONG_MAX, and then assigned to the
 'int' tz variable.

 That catches overflow from strtol, but we'd then truncate when we pass
 it as an int to show_date.

 I think we want this instead:

Makes sense.

Acked-by: Eric Sunshine sunsh...@sunshineco.com

 -- 8 --
 Subject: show_ident_date: fix tz range check

 Commit 1dca155fe3fa (log: handle integer overflow in
 timestamps, 2014-02-24) tried to catch integer overflow
 coming from strtol() on the timezone field by comparing against
 LONG_MIN/LONG_MAX. However, the intermediate tz variable
 is an int, which means it can never be LONG_MAX on LP64
 systems; we would truncate the output from strtol before the
 comparison.

 Clang's -Wtautological-constant-out-of-range-compare notices
 this and rightly complains.

 Let's instead store the result of strtol in a long, and then
 compare it against INT_MIN/INT_MAX. This will catch overflow
 from strtol, and also overflow when we pass the result as an
 int to show_date.

 Reported-by: Eric Sunshine sunsh...@sunshineco.com
 Signed-off-by: Jeff King p...@peff.net
 ---
  pretty.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/pretty.c b/pretty.c
 index 3b811ed..6e266dd 100644
 --- a/pretty.c
 +++ b/pretty.c
 @@ -397,7 +397,7 @@ static const char *show_ident_date(const struct 
 ident_split *ident,
enum date_mode mode)
  {
 unsigned long date = 0;
 -   int tz = 0;
 +   long tz = 0;

 if (ident-date_begin  ident-date_end)
 date = strtoul(ident-date_begin, NULL, 10);
 @@ -406,7 +406,7 @@ static const char *show_ident_date(const struct 
 ident_split *ident,
 else {
 if (ident-tz_begin  ident-tz_end)
 tz = strtol(ident-tz_begin, NULL, 10);
 -   if (tz == LONG_MAX || tz == LONG_MIN)
 +   if (tz = INT_MAX || tz = INT_MIN)
 tz = 0;
 }
 return show_date(date, tz, mode);
 --
 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 v3] upload-pack: send shallow info over stdin to pack-objects

2014-03-07 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Fri, Mar 7, 2014 at 1:37 AM, Junio C Hamano gits...@pobox.com wrote:
 I like what I see in this patch, but I wonder if we can essentially
 revert that temporary shallow file patch and replace it with the
 same (or a similar) mechanism uniformly?

 Using --shallow-file is uniform.  The only downside is temporary file.
 Without it you'll need to think of a way (probably different way for
 each command) to feed shallow info to.

Yes, that is what I meant to say by the we need a way to tell hooks
in some cases below; we are in agreement.

 On the receive-pack side, the comment at the bottom of
 preprare_shallow_update() makes it clear that, if we wanted to use
 hooks, we cannot avoid having the proposed new shallow-file in a
 temporary file, which is unfortunate.  Do we have a similar issue on
 hooks on the upload-pack side?

 No. I don't think we have hooks on upload-pack.

The question was not only about do we happen to work OK with the
current code? but about are we closing the door for the future?

If we ever need to add hooks to upload-pack, with the updated
approach, its operation will not be affected by the temporary
shallow file tailored for this specific customer.  Which I think is
a good thing in general.

But at the same time, it means that its operation cannot be
customized for the specific customer, taking into account what they
lack (which can be gleaned by looking at the temporary shallow
information).  I do think that it is not a big downside, but that is
merely my knee-jerk reaction.

 Nothing for Documentation/ anywhere?

 Heh git-pack-objects.txt never described stdin format. At least I
 searched for --not in it and found none. So I gladly accepted the
 situation and skipped doc update :D

To pile new technical debt on top of existing ones is to make things
worse, which we would rather not to see.
--
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] upload-pack: send shallow info over stdin to pack-objects

2014-03-07 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, Mar 6, 2014 at 3:49 PM, Nguyễn Thái Ngọc Duy pclo...@gmail.com 
 wrote:
 diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh
 index 3ae9092..a980574 100755
 --- a/t/t5537-fetch-shallow.sh
 +++ b/t/t5537-fetch-shallow.sh
 @@ -173,4 +173,17 @@ EOF
 )
  '

 +test_expect_success POSIXPERM,SANITY 'shallow fetch from a read-only repo' '
 +   cp -R .git read-only.git 
 +   find read-only.git -print | xargs chmod -w 
 +   test_when_finished find read-only.git -type d -print | xargs chmod 
 +w 
 +   git clone --no-local --depth=2 read-only.git from-read-only 
 +   git --git-dir=from-read-only/.git log --format=%s actual 
 +   cat expect EOF 
 +add-1-back
 +4
 +EOF
 +   test_cmp expect actual
 +'
 +
  test_done

 It's a separate issue, but maybe we should add a similar test case for
 non-shallow clone from a read-only repo too. Are there any other
 operations that should work well on read-only repos?

Any read-only operation ;-)?

On the object-transfer front, that would mean fetching from,
archive-r from, and perhaps creating bundle from.

Locally, log, grep, blame, gitk (but only the browsing part), etc.

If a read-write borrower borrows from a read-only location via the
objects/info/alternates mechanism, anying operation to the borrower
should also work without modifying the borrowee.

--
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 long for timezone in pretty.c:show_ident_date()

2014-03-07 Thread Brian Gernhardt
The value is parsed with strtol and compared against LONG_MIN and
LONG_MAX, which doesn't make much sense for an int.

Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com
---
 pretty.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pretty.c b/pretty.c
index 3b811ed..29ebc4f 100644
--- a/pretty.c
+++ b/pretty.c
@@ -397,7 +397,7 @@ static const char *show_ident_date(const struct ident_split 
*ident,
   enum date_mode mode)
 {
unsigned long date = 0;
-   int tz = 0;
+   long tz = 0;
 
if (ident-date_begin  ident-date_end)
date = strtoul(ident-date_begin, NULL, 10);
-- 
1.9.0.281.gfc51f0a.dirty

--
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: Microproject idea: new OPT_* macros for PARSE_OPT_NONEG

2014-03-07 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 I don't know the scope of these microprojects, but yes I think it'll
 take a few hours for this. By the way, a bit more thought on the idea:
 instead of making OPT_BOOL_NONEG() that sets NONEG, we could make
 OPT_BOOL_FLAGS(..., NONEG), which is more flexible.

What does a boolean that can never be set to false achieve, by the
way?  If you have

[alias]
foo = bar --frotz

then you may want to be able to say git foo --no-frotz to
countermand the frotz option, and by marking the boolean frotz
option to be a NONEG, you can forbid such a usage.  That is the only
use case I can think of, and that particular use case does not
sound like a valid one.

Looking at git grep -B3 OPT_NONEG output, it seems that NONEG is
associated mostly with OPTION_CALLBACK and OPTION_SET_INT in the
existing code.

Perhaps OPT_SET_INT should default to not just OPT_NOARG but also
OPT_NONEG?

I have a suspition that most users of other OPT_SET_TYPE
short-hands may also want them to default to NONEG (and the rare
ones that want to allow --no-value-of-this-type=Heh for some
reason to use the fully spelled form).  IIRC NONEG is relatively a
new addition, and many existing OPT_STRING() may predate it.

So I am not sure if doubling the number of OPT_type macros as your
micro suggestion proposes is the right solution to the problem.
--
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] Use long for timezone in pretty.c:show_ident_date()

2014-03-07 Thread Eric Sunshine
On Fri, Mar 7, 2014 at 1:47 PM, Brian Gernhardt
br...@gernhardtsoftware.com wrote:
 The value is parsed with strtol and compared against LONG_MIN and
 LONG_MAX, which doesn't make much sense for an int.

 Signed-off-by: Brian Gernhardt br...@gernhardtsoftware.com

Thanks. Find a more complete fix here [1].

[1]: http://thread.gmane.org/gmane.comp.version-control.git/243582/focus=243617

 ---
  pretty.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/pretty.c b/pretty.c
 index 3b811ed..29ebc4f 100644
 --- a/pretty.c
 +++ b/pretty.c
 @@ -397,7 +397,7 @@ static const char *show_ident_date(const struct 
 ident_split *ident,
enum date_mode mode)
  {
 unsigned long date = 0;
 -   int tz = 0;
 +   long tz = 0;

 if (ident-date_begin  ident-date_end)
 date = strtoul(ident-date_begin, NULL, 10);
 --
 1.9.0.281.gfc51f0a.dirty

 --
 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: git merge --date --author

2014-03-07 Thread Junio C Hamano
Andreas Schwab sch...@linux-m68k.org writes:

 Yann Droneaud ydrone...@opteya.com writes:

 But I'd like to know if there's a specific reason for git merge to not
 support --date and --author ?

 It's rather unusual that a merge is performed on behalf of a different
 author.

Yes.  Michael's Nobody bothered to implement it is also correct
but the reason why nobody bothered to most likely is due to why
would you want to lie?.

If the use case is to rebuild history, you would need to be able to
also lie about the committer, so

 git merge \
 --date 2013-12-31 23:59:59 + \
 --author Happy New Year happy.new-year@gregorian.calendar \
 current-year

in such a history-rebuild script would not be sufficient.  The
script can set necessary environment variables to lie about both
author and commiter, though, of course.



--
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] tag: grok --with as synonym to --contains

2014-03-07 Thread Junio C Hamano
Just like git branch can be told to list the branches that has the
named commit by git branch --with commit, teach the same
short-hand to git tag, so that git tag --with commit shows the
releases with the named commit.

Signed-off-by: Junio C Hamano gits...@pobox.com
---

 * After umpteenth time I got an optparse error I finally decided
   that this may be worth consideration.

   Just like git branch, the synonym is not advertised in the
   documentation nor git cmd -h output.  We _might_ want to expose
   both at the same time, but that is not in the scope of this patch.

 builtin/tag.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/builtin/tag.c b/builtin/tag.c
index af3af3f..cb7bb2b 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -471,6 +471,12 @@ int cmd_tag(int argc, const char **argv, const char 
*prefix)
parse_opt_with_commit, (intptr_t)HEAD,
},
{
+   OPTION_CALLBACK, 0, with, with_commit, N_(commit),
+   N_(print only tags that contain the commit),
+   PARSE_OPT_HIDDEN | PARSE_OPT_LASTARG_DEFAULT,
+   parse_opt_with_commit, (intptr_t)HEAD,
+   },
+   {
OPTION_CALLBACK, 0, points-at, NULL, N_(object),
N_(print only tags of the object), 0, 
parse_opt_points_at
},
--
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-07 Thread Jeff King
On Thu, Mar 06, 2014 at 10:47:43PM +0100, Julian Brost wrote:

 I've noticed some behavior of git that might lead to some security
 issues if the user is not aware of this.
 
 Assume we have an evil user on a system, let's call him eve. He
 prepares a repository where he allows other user to push changes to.
 If he now adds a post-receive hook, git will happly execute it as
 whatever user pushes to this repository:

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

 Something similiar might happen if eve adds some alias to the config
 file in the repository and grants any other user read access to the
 repository. These aliases will be executed when some other user is
 running any git command in this repository. Even though git does not
 allow defining aliases for existing commands, you might mistype
 something, so adding an alias for lg instead of log might succeed:

Much easier is to define an external diff or textconv tool; then the
victim does not even have to typo.

 I'd suggest taking a similar approach as Mercurial [1], i.e. ignoring
 configuration files and hooks owned by another user unless the owner
 is explicitly trusted

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.

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

-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 v7 03/11] trailer: read and process config information

2014-03-07 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes:

 This patch implements reading the configuration
 to get trailer information, and then processing
 it and storing it in a doubly linked list.

Read and process the ..., perhaps?

 The config information is stored in the list
 whose first item is pointed to by:

 static struct trailer_item *first_conf_item;

If feels somewhat strange if a doubly linked list has only the first
pointer without the last pointer, unless the previous field of the
first one by convention points at the last one, forming a cycle
(which I think is a reasonable thing to do, as a desire for a quick
access to the top and the bottom is often a reason to use doubly
linked list---I didn't check if you implement it that way, though).

 +static int set_where(struct conf_info *item, const char *value)
 +{
 + if (!strcasecmp(after, value))
 + item-where = WHERE_AFTER;
 + else if (!strcasecmp(before, value))
 + item-where = WHERE_BEFORE;
 + else
 + return 1;

Please follow the usual convention of returning a negative value
upon error, unless there is a compelling reason not to do so.

Do we really want to do strcasecmp here?  Are some values case
sensitive and we only allow after, AfTer, AFTER, etc. as case
insensitive keywords?  If all values are expected to be case
insensitive, wouldn't it be better to downcase in the caller (just
like we do in the config parser) and use strcmp() against lower case
constants?

All of the comments applies equally to other functions.

 +enum trailer_info_type { TRAILER_KEY, TRAILER_COMMAND, TRAILER_WHERE,
 +  TRAILER_IF_EXISTS, TRAILER_IF_MISSING };
 +
 +static int set_name_and_type(const char *conf_key, const char *suffix,
 +  enum trailer_info_type type,
 +  char **pname, enum trailer_info_type *ptype)
 +{
 + int ret = ends_with(conf_key, suffix);
 + if (ret) {
 + *pname = xstrndup(conf_key, strlen(conf_key) - strlen(suffix));
 + *ptype = type;
 + }
 + return ret;
 +}

This implementation, together with the caller that makes many calls
to it, looks overly inefficient (both runtime- and reviewtime-wise),
doesn't it?

How about having the caller upfront find the last dot before
checking the name-and-type to avoid calling ends_with() so many
times unnecessarily?

Also, wouldn't it be better to make the caller more table-driven,
i.e.

static struct {
const char *name;
enum trailer_kinfo_type type;
} trailer_config_items[] = {
{ key, TRAILER_KEY },
...
};

in the caller's scope, and then

const char *trailer_item, *variable_name;

trailer_item = skip_prefix(conf_key, trailer.);
if (!trailer_item)
return 0;

variable_name = strrchr(trailer_item, '.');
if (!variable_name) {
... trailer_item is a two-level variable name.
... Handle it in whatever way.
return 0;
}

variable_name++;
for (i = 0; i  ARRAY_SIZE(trailer_config_items); i++) {
if (strcmp(trailer_config_items[i].name, variable_name))
continue;
name = xstrdup(...);
type = trailer_config_items[i].type;
goto found;
}

/* Unknown trailer.item.variable_name */
return 0;

found:
... do whatever depending on the type ...

or something?

 +static struct trailer_item *get_conf_item(const char *name)
 +{
 + struct trailer_item *item;
 + struct trailer_item *previous;
 +
 + /* Look up item with same name */
 + for (previous = NULL, item = first_conf_item;
 +  item;
 +  previous = item, item = item-next) {
 + if (!strcasecmp(item-conf.name, name))
 + return item;
 + }
 +
 + /* Item does not already exists, create it */
 + item = xcalloc(sizeof(struct trailer_item), 1);
 + item-conf.name = xstrdup(name);
 +
 + if (!previous)
 + first_conf_item = item;
 + else {
 + previous-next = item;
 + item-previous = previous;
 + }
 +
 + return item;
 +}
 +
 +static int git_trailer_config(const char *conf_key, const char *value, void 
 *cb)
 +{
 + if (starts_with(conf_key, trailer.)) {
 + const char *orig_conf_key = conf_key;
 + struct trailer_item *item;
 + struct conf_info *conf;
 + char *name;
 + enum trailer_info_type type;
 +
 + conf_key += 8;
 + if (!set_name_and_type(conf_key, .key, TRAILER_KEY, name, 
 type) 
 + !set_name_and_type(conf_key, .command, TRAILER_COMMAND, 
 name, type) 
 + !set_name_and_type(conf_key, .where, TRAILER_WHERE, 
 name, type) 
 + 

Re: [PATCH/RFC] Documentation: Say that submodule clones use a separate gitdirs.

2014-03-07 Thread Andrew Keller
On Mar 7, 2014, at 2:53 AM, Henri GEIST geist.he...@laposte.net wrote:

 Adding a note in the submodule documentation signaling that the
 automatically cloned missing submodules are cloned with a separate
 gitdir. And where it is put.
 
 Signed-off-by: Henri GEIST geist.he...@laposte.net
 ---
 Documentation/git-submodule.txt |5 +
 1 file changed, 5 insertions(+)
 
 diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
 index 21cb59a..ea837fd 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 @@ -64,6 +64,11 @@ using the 'status' subcommand and get a detailed overview 
 of the
 difference between the index and checkouts using the 'summary'
 subcommand.
 
 +*NOTE*: when submodule add or submodule update commands clone a missing
 +submodule, the option --separate-git-dir is passed to the clone command
 +and the gitdir of the submodule is placed outside of its working
 +directory in the .git/module of the current repository.
 +

The modules directory is 'modules'.  And, the '.git' folder is not always 
called '.git' -- in a submodule, for example, the directory name is the name of 
the module.

Also, this file contains mostly high-level documentation, and this addition 
feels technical in nature.  Is there a location for more technical 
documentation?  Or, perhaps it can be reworded to sound less technical?

 
 COMMANDS
 
 -- 
 1.7.9.3.369.gd715.dirty
 
 
 --

 - 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: [PATCH v7 00/11] Add interpret-trailers builtin

2014-03-07 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes:

 * many style fixes

This round is readable ;-)  Thanks.

 * clearer and nicer setup tests

Those long lines that use printf with many embedded \n were harder
to read and also looked harder to maintain if we ever wanted to
change them.  Splicing a string with \n in the middle of a long
single line is far harder than adding an independent line, I would
think.  For example:

... 
printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n 
expected 
...

is easier to read and maintain if written like so (with using HT
properly---our MUAs may damage it and turn the indentation into
spaces):

... 
sed -e s/ Z$/ / expect -\EOF 
Fixes: Z
Acked-by= Z
Reviewed-by: Z
Signed-off-by: Z
EOF
...

--
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 v7 03/11] trailer: read and process config information

2014-03-07 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes:

 Christian Couder chrisc...@tuxfamily.org writes:

 This patch implements reading the configuration
 to get trailer information, and then processing
 it and storing it in a doubly linked list.

 Read and process the ..., perhaps?

 The config information is stored in the list
 whose first item is pointed to by:

 static struct trailer_item *first_conf_item;

 If feels somewhat strange if a doubly linked list has only the first
 pointer without the last pointer, unless the previous field of the
 first one by convention points at the last one, forming a cycle
 (which I think is a reasonable thing to do, as a desire for a quick
 access to the top and the bottom is often a reason to use doubly
 linked list---I didn't check if you implement it that way, though).

Can't say I agree here.  Basically all my doubly-linked lists are not
for traversing data forwards and backwards but for making it possible to
delete list members that have not been reached by list traversal but
rather by orthogonal data access methods.  Consequently, my back links
usually don't point to the previous list member (which would require
special-casing the first element) but rather to its referring
forward-pointing link (which for the first list element means a pointer
to the list head).

Having a last pointer is an orthogonal concept: you need it if you
want to append to a list's end without actually looking at its members.
The presence of such a last pointer actually makes it quite more ugly to
delete the last member of a doubly-linked list reached by some other
means as you then need to have some way of adjusting the tail pointer
accordingly.

-- 
David Kastrup
--
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 v7 03/11] trailer: read and process config information

2014-03-07 Thread Junio C Hamano
David Kastrup d...@gnu.org writes:

 Junio C Hamano gits...@pobox.com writes:

 Christian Couder chrisc...@tuxfamily.org writes:

 The config information is stored in the list
 whose first item is pointed to by:

 static struct trailer_item *first_conf_item;

 If feels somewhat strange ...

 Can't say I agree here.  Basically all my doubly-linked lists are not
 for traversing data forwards and backwards ...
 Having a last pointer is an orthogonal concept ...

Yeah, that is where somewhat strange, not wrong, comes from ;-)


--
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-07 Thread Junio C Hamano
Andrew Keller and...@kellerfarm.com writes:

 diff --git a/Documentation/git-submodule.txt 
 b/Documentation/git-submodule.txt
 index 21cb59a..ea837fd 100644
 --- a/Documentation/git-submodule.txt
 +++ b/Documentation/git-submodule.txt
 ...
 Also, this file contains mostly high-level documentation, and this
 addition feels technical in nature.  Is there a location for more
 technical documentation?  Or, perhaps it can be reworded to sound
 less technical?

I tend to agree that the new paragraph looked somewhat out of place
and goes into a too low-level detail of the implementation.

The repository-layout documentation may be a better place for
readers to learn what lives where inside $GIT_DIR.

--
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-07 Thread Henri GEIST
Le vendredi 07 mars 2014 à 16:42 -0500, Andrew Keller a écrit :
 On Mar 7, 2014, at 2:53 AM, Henri GEIST geist.he...@laposte.net wrote:
 
  Adding a note in the submodule documentation signaling that the
  automatically cloned missing submodules are cloned with a separate
  gitdir. And where it is put.
  
  Signed-off-by: Henri GEIST geist.he...@laposte.net
  ---
  Documentation/git-submodule.txt |5 +
  1 file changed, 5 insertions(+)
  
  diff --git a/Documentation/git-submodule.txt 
  b/Documentation/git-submodule.txt
  index 21cb59a..ea837fd 100644
  --- a/Documentation/git-submodule.txt
  +++ b/Documentation/git-submodule.txt
  @@ -64,6 +64,11 @@ using the 'status' subcommand and get a detailed 
  overview of the
  difference between the index and checkouts using the 'summary'
  subcommand.
  
  +*NOTE*: when submodule add or submodule update commands clone a missing
  +submodule, the option --separate-git-dir is passed to the clone command
  +and the gitdir of the submodule is placed outside of its working
  +directory in the .git/module of the current repository.
  +
 
 The modules directory is 'modules'.  And, the '.git' folder is not always 
 called '.git' -- in a submodule, for example, the directory name is the name 
 of the module.
 
 Also, this file contains mostly high-level documentation, and this addition 
 feels technical in nature.  Is there a location for more technical 
 documentation?  Or, perhaps it can be reworded to sound less technical?

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 have done it and as the doc say nothing about that it take me some time to
understand what happened. And why this repository behave differently than 
others.

May be I can rephrase like this :

*NOTE*: when 'submodule add' or 'submodule update' commands clone a missing
submodule, the $GIT_DIR of the submodule containing the metadata is placed
outside of its working directory in the $GIT_DIR/modules of the current
repository. Those submodule can not directly be copied or move.
You need to use 'git clone'.


 
  
  COMMANDS
  
  -- 
  1.7.9.3.369.gd715.dirty
  
  
  --
 
  - Andrew Keller
 




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


Re: [PATCH] submodule : Add --no-separate-git-dir option to add and update command.

2014-03-07 Thread Jens Lehmann
Am 06.03.2014 23:20, schrieb Henri GEIST:
 Le jeudi 06 mars 2014 à 21:51 +0100, Jens Lehmann a écrit :
 Am 06.03.2014 21:15, schrieb Henri GEIST:
 Le jeudi 06 mars 2014 à 20:48 +0100, Jens Lehmann a écrit :
 Am 06.03.2014 02:25, schrieb Henri GEIST:
 Wow, that shouldn't even work (as everything inside submodule
 shouldn't be part of the superproject but must be contained in
 the submodule itself). Do the git submodule script and other
 git commands like git status work for you in such setups?

 As I stated above it is the purpose of the other patch that I have not 
 already send
 to implement this behavior. And that is why it work.

 Ok.

 Everything including 'git submodule' and 'git status' work perfectly.
 The intent of this patch is only to permit this for gitlinks. Not for 
 regular files.

 But I still believe that this shouldn't be permitted at all,
 no matter if files or submodules are concerned. The pitfalls
 files face in such a scenario are pretty much the same for
 submodules too.
 
 May be you have a good argument for this belief ?

Sure, I stated it further down:

 The problem you're creating with your future patch
 is related to the work tree, not the GIT_DIR: subsubmodule
 could also be added to and tracked by submodule (as that is
 completely unaware of subsubmodule already being tracked by
 the superproject). Then you would end up in real trouble, as
 superproject and submodule could have differing SHA-1s
 recorded for subsubmodule. Don't go there, for just the same
 reasons we do not allow that for files.

 As for the difference between submodules and regular files
 the only difference is in the meaning.
 Technically directory are just a special kind of file.
 But there day to day use is drastically different of
 the use of files which are not directories.
 I am not against enabling it for files as well.
 I am just unable to imagine a case where it make sens.

It doesn't make sense for both files and submodules.

 A possible solution when someone try to do it is to issue a warning.
 We are not able to see any good reason to do this are sure (y/n) ?

No, the only possible solution I see is not to allow that at
all.

 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 ?

 It should be placed in .git/modules/submodule/modules/subsubmodule
 of the superproject (assuming the subsubmodule is part of the first
 level submodule). But in your example that would live in
 .git/modules/submodule/subsubmodule (but as mentioned above, I do
 not consider this a valid setup because then two repositories would
 be responsible for the data inside subsubmodule, which will lead to
 lots of trouble).

 That is why a had proposed an option '--no-separate-git-dir'
 for 'git submodule add|update' then no repository is responsible for the 
 data
 in subsubmodule except subsubmodule itself.

 As I mentioned in my other email, it doesn't matter at all for
 the setup you're describing if the git directory lives under
 .git/modules of the superproject or in a .git directory in the
 submodule. The problem you're creating with your future patch
 is related to the work tree, not the GIT_DIR: subsubmodule
 could also be added to and tracked by submodule (as that is
 completely unaware of subsubmodule already being tracked by
 the superproject). Then you would end up in real trouble, as
 superproject and submodule could have differing SHA-1s
 recorded for subsubmodule. Don't go there, for just the same
 reasons we do not allow that for files.
 
 In fact it mater.
 Because multiples checkout of superproject and submodules in versions
 where subsubmodule is present and not.
 subsubmodule could have been clone one time by submodule and one time
 by superproject.

And each would have a different .git directory. Where is the
problem with that? Size? Different refs?

 And then if they are cloned with --separate-gitdir subsubmodule can
 have two gitdirs in superproject/modules/submodule/subsubmodule and
 in superproject/submodule/modules/subsubmodule.
 Only one is active at a given time but they are two and not synchronized.

But the synchronization is done via the superproject, no?

 What is the use case you are trying to solve and why can that
 not be handled by adding subsubmodule inside submodule?
 
 The problem is access rights.
 
 Imagine you have 2 people Pierre and Paul.
 Each with different access write on the server.
 Pierre has full access on every things.
 Paul has full access on superproject and subsubmodule but no read/write
 access to submodule only execution on the directory.

Ok, I think I'm slowly beginning to understand your setup.

 I want all user to get every things they are allowed to have with the
 command 'git submodule update --init --recursive'.
 Then as Paul can not clone submodule he can not get 

Re: [PATCH/RFC] Documentation: Say that submodule clones use a separate gitdirs.

2014-03-07 Thread Junio C Hamano
Henri GEIST geist.he...@laposte.net 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.


--
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] upload-pack: send shallow info over stdin to pack-objects

2014-03-07 Thread Duy Nguyen
On Sat, Mar 8, 2014 at 1:27 AM, Junio C Hamano gits...@pobox.com wrote:
 On the receive-pack side, the comment at the bottom of
 preprare_shallow_update() makes it clear that, if we wanted to use
 hooks, we cannot avoid having the proposed new shallow-file in a
 temporary file, which is unfortunate.  Do we have a similar issue on
 hooks on the upload-pack side?

 No. I don't think we have hooks on upload-pack.

 The question was not only about do we happen to work OK with the
 current code? but about are we closing the door for the future?

 If we ever need to add hooks to upload-pack, with the updated
 approach, its operation will not be affected by the temporary
 shallow file tailored for this specific customer.  Which I think is
 a good thing in general.

 But at the same time, it means that its operation cannot be
 customized for the specific customer, taking into account what they
 lack (which can be gleaned by looking at the temporary shallow
 information).  I do think that it is not a big downside, but that is
 merely my knee-jerk reaction.

When upload-pack learns about hooks, I think we'll need to go back
with --shallow-file, perhaps we a secure temporary place to write in.
I don't see another way out. Not really sure why upload-pack needs
customization though. The only case I can think of is to prevent most
users from fetching certain objects, but that does not sound
realistic..

 Nothing for Documentation/ anywhere?

 Heh git-pack-objects.txt never described stdin format. At least I
 searched for --not in it and found none. So I gladly accepted the
 situation and skipped doc update :D

 To pile new technical debt on top of existing ones is to make things
 worse, which we would rather not to see.

Of course.. So what should we do with this? Go with this no temp
file approach and deal with hooks when they come, or prepare now and
introduce a secure temp. area?
-- 
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/RFC] Documentation: Say that submodule clones use a separate gitdirs.

2014-03-07 Thread Henri GEIST
Le vendredi 07 mars 2014 à 15:37 -0800, Junio C Hamano a écrit :
 Henri GEIST geist.he...@laposte.net 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.




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


Re: [PATCH v4 18/27] setup.c: support multi-checkout repo setup

2014-03-07 Thread Duy Nguyen
On Thu, Mar 6, 2014 at 2:42 AM, Junio C Hamano gits...@pobox.com wrote:
 Nguyễn Thái Ngọc Duy  pclo...@gmail.com 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,

I tend to think so, .git/commondir is just a convenient way to set
$GIT_COMMON_DIR. $GIT_COMMON_DIR is the key. config.txt correctly
states so, but the commit message is misleading.

  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?

It should work exactly like how normal worktree does, so if $GIT_DIR
is set and $GIT_WORK_TREE is not, $PWD is the worktree root.
-- 
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


[PATCH v5 00/28] Support multiple checkouts

2014-03-07 Thread Nguyễn Thái Ngọc Duy
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
-- 8 --
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 438b213..c635b8d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1184,9 +1184,10 @@ gc.pruneexpire::
unreachable objects immediately.
 
 gc.prunereposexpire::
-   When 'git gc' is run, it will call 'prune --repos --expire 
3.months.ago'.
-   Override the grace period with this config variable.  The value
-   now may be used to disable this  grace period and always prune
+   When 'git gc' is run, it will call
+   'prune --repos --expire 3.months.ago'.
+   Override the grace period with this config variable. The value
+   now may be used to disable the grace period and always prune
$GIT_DIR/repos immediately.
 
 gc.reflogexpire::
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 8e6ad32..8a9cac8 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -238,7 +238,9 @@ print a message to stderr and exit with nonzero status.
 --git-path path::
Resolve $GIT_DIR/path and takes other path relocation
variables such as $GIT_OBJECT_DIRECTORY,
-   $GIT_INDEX_FILE... into account.
+   $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.
 
 --show-cdup::
When the command is invoked from a subdirectory, show the
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 2dc6901..dff6c47 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -47,7 +47,7 @@ use with dumb transports but otherwise is OK as long as
 `objects/info/alternates` points at the object stores it
 borrows from.
 +
-This directory is ignored $GIT_COMMON_DIR is set and
+This directory is ignored if $GIT_COMMON_DIR is set and
 $GIT_COMMON_DIR/objects will be used instead.
 
 objects/[0-9a-f][0-9a-f]::
@@ -95,7 +95,7 @@ refs::
References are stored in subdirectories of this
directory.  The 'git prune' command knows to preserve
objects reachable from refs found in this directory and
-   its subdirectories. This directory is ignored $GIT_COMMON_DIR
+   its subdirectories. This directory is ignored if $GIT_COMMON_DIR
is set and $GIT_COMMON_DIR/refs will be used instead.
 
 refs/heads/`name`::
@@ -118,7 +118,7 @@ refs/replace/`obj-sha1`::
 packed-refs::
records the same information as refs/heads/, refs/tags/,
and friends record in a more efficient way.  See
-   linkgit:git-pack-refs[1]. This file is ignored $GIT_COMMON_DIR
+   linkgit:git-pack-refs[1]. This file is ignored if $GIT_COMMON_DIR
is set and $GIT_COMMON_DIR/packed-refs will be used instead.
 
 HEAD::
@@ -151,7 +151,7 @@ branches::
'repository' argument.  See the REMOTES section in
linkgit:git-fetch[1] for details.  This mechanism is legacy
and not likely to be found in modern repositories. This
-   directory is ignored $GIT_COMMON_DIR is set and
+   directory is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/branches will be used instead.
 
 
@@ -162,7 +162,7 @@ hooks::
default.  To enable, the `.sample` suffix has to be
removed from the filename by renaming.
Read linkgit:githooks[5] for more details about
-   each hook. This directory is ignored $GIT_COMMON_DIR is set
+   each hook. This directory is ignored if $GIT_COMMON_DIR is set
and $GIT_COMMON_DIR/hooks will be used instead.
 
 
@@ -172,7 +172,7 @@ index::
 
 info::
Additional information about the repository is recorded
-   in this directory. This directory is ignored $GIT_COMMON_DIR
+   in this directory. This directory is ignored if $GIT_COMMON_DIR
is set and $GIT_COMMON_DIR/index will be used instead.
 
 info/refs::
@@ -210,7 +210,7 @@ remotes::
'git pull' and 'git push' commands.  See the REMOTES section
in linkgit:git-fetch[1] for details.  This mechanism is legacy
and not likely to be found in modern repositories. This
-   directory is ignored $GIT_COMMON_DIR is set and
+   directory is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/remotes will be used instead.
 
 logs::
@@ -230,7 +230,7 @@ shallow::
This is 

[PATCH v5 01/28] path.c: make get_pathname() return strbuf instead of static buffer

2014-03-07 Thread Nguyễn Thái Ngọc Duy
We've been avoiding PATH_MAX whenever possible. This patch makes
get_pathname() return a strbuf and updates the callers to take
advantage of this. The code is simplified as we no longer need to
worry about buffer overflow.

vsnpath() behavior is changed slightly: previously it always clears
the buffer before writing, now it just appends. Fortunately this is a
static function and all of its callers prepare the buffer properly:
git_path() gets the buffer from get_pathname() which resets the
buffer, the remaining call sites start with STRBUF_INIT'd buffer.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 120 -
 1 file changed, 51 insertions(+), 69 deletions(-)

diff --git a/path.c b/path.c
index 24594c4..5346700 100644
--- a/path.c
+++ b/path.c
@@ -16,11 +16,15 @@ static int get_st_mode_bits(const char *path, int *mode)
 
 static char bad_path[] = /bad-path/;
 
-static char *get_pathname(void)
+static struct strbuf *get_pathname(void)
 {
-   static char pathname_array[4][PATH_MAX];
+   static struct strbuf pathname_array[4] = {
+   STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+   };
static int index;
-   return pathname_array[3  ++index];
+   struct strbuf *sb = pathname_array[3  ++index];
+   strbuf_reset(sb);
+   return sb;
 }
 
 static char *cleanup_path(char *path)
@@ -34,6 +38,13 @@ static char *cleanup_path(char *path)
return path;
 }
 
+static void strbuf_cleanup_path(struct strbuf *sb)
+{
+   char *path = cleanup_path(sb-buf);
+   if (path  sb-buf)
+   strbuf_remove(sb, 0, path - sb-buf);
+}
+
 char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 {
va_list args;
@@ -49,85 +60,70 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
 }
 
-static char *vsnpath(char *buf, size_t n, const char *fmt, va_list args)
+static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
 {
const char *git_dir = get_git_dir();
-   size_t len;
-
-   len = strlen(git_dir);
-   if (n  len + 1)
-   goto bad;
-   memcpy(buf, git_dir, len);
-   if (len  !is_dir_sep(git_dir[len-1]))
-   buf[len++] = '/';
-   len += vsnprintf(buf + len, n - len, fmt, args);
-   if (len = n)
-   goto bad;
-   return cleanup_path(buf);
-bad:
-   strlcpy(buf, bad_path, n);
-   return buf;
+   strbuf_addstr(buf, git_dir);
+   if (buf-len  !is_dir_sep(buf-buf[buf-len - 1]))
+   strbuf_addch(buf, '/');
+   strbuf_vaddf(buf, fmt, args);
+   strbuf_cleanup_path(buf);
 }
 
 char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 {
-   char *ret;
+   struct strbuf sb = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   ret = vsnpath(buf, n, fmt, args);
+   vsnpath(sb, fmt, args);
va_end(args);
-   return ret;
+   if (sb.len = n)
+   strlcpy(buf, bad_path, n);
+   else
+   memcpy(buf, sb.buf, sb.len + 1);
+   strbuf_release(sb);
+   return buf;
 }
 
 char *git_pathdup(const char *fmt, ...)
 {
-   char path[PATH_MAX], *ret;
+   struct strbuf path = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   ret = vsnpath(path, sizeof(path), fmt, args);
+   vsnpath(path, fmt, args);
va_end(args);
-   return xstrdup(ret);
+   return strbuf_detach(path, NULL);
 }
 
 char *mkpathdup(const char *fmt, ...)
 {
-   char *path;
struct strbuf sb = STRBUF_INIT;
va_list args;
-
va_start(args, fmt);
strbuf_vaddf(sb, fmt, args);
va_end(args);
-   path = xstrdup(cleanup_path(sb.buf));
-
-   strbuf_release(sb);
-   return path;
+   strbuf_cleanup_path(sb);
+   return strbuf_detach(sb, NULL);
 }
 
 char *mkpath(const char *fmt, ...)
 {
va_list args;
-   unsigned len;
-   char *pathname = get_pathname();
-
+   struct strbuf *pathname = get_pathname();
va_start(args, fmt);
-   len = vsnprintf(pathname, PATH_MAX, fmt, args);
+   strbuf_vaddf(pathname, fmt, args);
va_end(args);
-   if (len = PATH_MAX)
-   return bad_path;
-   return cleanup_path(pathname);
+   return cleanup_path(pathname-buf);
 }
 
 char *git_path(const char *fmt, ...)
 {
-   char *pathname = get_pathname();
+   struct strbuf *pathname = get_pathname();
va_list args;
-   char *ret;
-
va_start(args, fmt);
-   ret = vsnpath(pathname, PATH_MAX, fmt, args);
+   vsnpath(pathname, fmt, args);
va_end(args);
-   return ret;
+   return pathname-buf;
 }
 
 void home_config_paths(char **global, char **xdg, char *file)
@@ -158,41 +154,27 @@ void home_config_paths(char **global, char **xdg, char 
*file)
 
 char *git_path_submodule(const char *path, const char *fmt, ...)
 {
-  

[PATCH v5 04/28] path.c: rename vsnpath() to do_git_path()

2014-03-07 Thread Nguyễn Thái Ngọc Duy
The name vsnpath() gives an impression that this is general path
handling function. It's not. This is the underlying implementation of
git_path(), git_pathdup() and strbuf_git_path() which will prefix
$GIT_DIR in the result string.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/path.c b/path.c
index 417df76..987fdec 100644
--- a/path.c
+++ b/path.c
@@ -60,7 +60,7 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
 }
 
-static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
+static void do_git_path(struct strbuf *buf, const char *fmt, va_list args)
 {
const char *git_dir = get_git_dir();
strbuf_addstr(buf, git_dir);
@@ -74,7 +74,7 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
 {
va_list args;
va_start(args, fmt);
-   vsnpath(sb, fmt, args);
+   do_git_path(sb, fmt, args);
va_end(args);
 }
 
@@ -83,7 +83,7 @@ char *git_pathdup(const char *fmt, ...)
struct strbuf path = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   vsnpath(path, fmt, args);
+   do_git_path(path, fmt, args);
va_end(args);
return strbuf_detach(path, NULL);
 }
@@ -114,7 +114,7 @@ const char *git_path(const char *fmt, ...)
struct strbuf *pathname = get_pathname();
va_list args;
va_start(args, fmt);
-   vsnpath(pathname, fmt, args);
+   do_git_path(pathname, fmt, args);
va_end(args);
return pathname-buf;
 }
-- 
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


[PATCH v5 08/28] reflog: avoid constructing .lock path with git_path

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Among pathnames in $GIT_DIR, e.g. index or packed-refs, we want to
automatically and silently map some of them to the $GIT_DIR of the
repository we are borrowing from via $GIT_COMMON_DIR mechanism.  When
we formulate the pathname for its lockfile, we want it to be in the
same location as its final destination.  index is not shared and
needs to remain in the borrowing repository, while packed-refs is
shared and needs to go to the borrowed repository.

git_path() could be taught about the .lock suffix and map
index.lock and packed-refs.lock the same way their basenames are
mapped, but instead the caller can help by asking where the basename
(e.g. index) is mapped to git_path() and then appending .lock
after the mapping is done.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/reflog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 852cff6..ccf2cf6 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -372,7 +372,7 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
if (!file_exists(log_file))
goto finish;
if (!cmd-dry_run) {
-   newlog_path = git_pathdup(logs/%s.lock, ref);
+   newlog_path = mkpathdup(%s.lock, log_file);
cb.newlog = fopen(newlog_path, w);
}
 
-- 
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


[PATCH v5 11/28] Add new environment variable $GIT_COMMON_DIR

2014-03-07 Thread Nguyễn Thái Ngọc Duy
This variable is intended to support multiple working directories
attached to a repository. Such a repository may have a main working
directory, created by either git init or git clone and one or more
linked working directories. These working directories and the main
repository share the same repository directory.

In linked working directories, $GIT_COMMON_DIR must be defined to point
to the real repository directory and $GIT_DIR points to an unused
subdirectory inside $GIT_COMMON_DIR. File locations inside the
repository are reorganized from the linked worktree view point:

 - worktree-specific such as HEAD, logs/HEAD, index, other top-level
   refs and unrecognized files are from $GIT_DIR.

 - the rest like objects, refs, info, hooks, packed-refs, shallow...
   are from $GIT_COMMON_DIR

Scripts are supposed to retrieve paths in $GIT_DIR with git rev-parse
--git-path, which will take care of $GIT_DIR vs $GIT_COMMON_DIR
business.

The redirection is done by git_path(), git_pathdup() and
strbuf_git_path(). The selected list of paths goes to $GIT_COMMON_DIR,
not the other way around in case a developer adds a new
worktree-specific file and it's accidentally promoted to be shared
across repositories (this includes unknown files added by third party
commands)

The list of known files that belong to $GIT_DIR are:

ADD_EDIT.patch BISECT_ANCESTORS_OK BISECT_EXPECTED_REV BISECT_LOG
BISECT_NAMES CHERRY_PICK_HEAD COMMIT_MSG FETCH_HEAD HEAD MERGE_HEAD
MERGE_MODE MERGE_RR NOTES_EDITMSG NOTES_MERGE_WORKTREE ORIG_HEAD
REVERT_HEAD SQUASH_MSG TAG_EDITMSG fast_import_crash_* logs/HEAD
next-index-* rebase-apply rebase-merge rsync-refs-* sequencer/*
shallow_*

Path mapping is NOT done for git_path_submodule(). Multi-checkouts are
not supported as submodules.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git.txt  |  8 +++
 Documentation/gitrepository-layout.txt | 42 ++
 cache.h|  4 +++-
 environment.c  | 19 +++
 path.c | 34 +++
 t/t0060-path-utils.sh  | 15 
 6 files changed, 108 insertions(+), 14 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 02bbc08..b094b1f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -773,6 +773,14 @@ Git so take care if using Cogito etc.
an explicit repository directory set via 'GIT_DIR' or on the
command line.
 
+'GIT_COMMON_DIR'::
+   If this variable is set to a path, non-worktree files that are
+   normally in $GIT_DIR will be taken from this path
+   instead. Worktree-specific files such as HEAD or index are
+   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] for
+   details. This variable has lower precedence than other path
+   variables such as GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY...
+
 Git Commits
 ~~~
 'GIT_AUTHOR_NAME'::
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index aa03882..556da09 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -46,6 +46,9 @@ of incomplete object store is not suitable to be published for
 use with dumb transports but otherwise is OK as long as
 `objects/info/alternates` points at the object stores it
 borrows from.
++
+This directory is ignored if $GIT_COMMON_DIR is set and
+$GIT_COMMON_DIR/objects will be used instead.
 
 objects/[0-9a-f][0-9a-f]::
A newly created object is stored in its own file.
@@ -92,7 +95,8 @@ refs::
References are stored in subdirectories of this
directory.  The 'git prune' command knows to preserve
objects reachable from refs found in this directory and
-   its subdirectories.
+   its subdirectories. This directory is ignored if $GIT_COMMON_DIR
+   is set and $GIT_COMMON_DIR/refs will be used instead.
 
 refs/heads/`name`::
records tip-of-the-tree commit objects of branch `name`
@@ -114,7 +118,8 @@ refs/replace/`obj-sha1`::
 packed-refs::
records the same information as refs/heads/, refs/tags/,
and friends record in a more efficient way.  See
-   linkgit:git-pack-refs[1].
+   linkgit:git-pack-refs[1]. This file is ignored if $GIT_COMMON_DIR
+   is set and $GIT_COMMON_DIR/packed-refs will be used instead.
 
 HEAD::
A symref (see glossary) to the `refs/heads/` namespace
@@ -133,6 +138,11 @@ being a symref to point at the current branch.  Such a 
state
 is often called 'detached HEAD.'  See linkgit:git-checkout[1]
 for details.
 
+config::
+   Repository specific configuration file. This file is ignored
+   $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/config will be
+   used instead.
+
 branches::
A slightly deprecated way to store shorthands to be used
to specify a URL to 'git fetch', 'git pull' and 

[PATCH v5 12/28] git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objects

2014-03-07 Thread Nguyễn Thái Ngọc Duy
If $GIT_COMMON_DIR is set, $GIT_OBJECT_DIRECTORY should be
$GIT_COMMON_DIR/objects, not $GIT_DIR/objects. Just let rev-parse
--git-path handle it.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-sh-setup.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index fffa3c7..475ca43 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -343,7 +343,7 @@ then
echo 2 Unable to determine absolute path of git directory
exit 1
}
-   : ${GIT_OBJECT_DIRECTORY=$GIT_DIR/objects}
+   : ${GIT_OBJECT_DIRECTORY=$(git rev-parse --git-path objects)}
 fi
 
 peel_committish () {
-- 
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


[PATCH v5 10/28] commit: use SEQ_DIR instead of hardcoding sequencer

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/commit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/commit.c b/builtin/commit.c
index 3767478..ee3ac10 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -155,7 +155,7 @@ static void determine_whence(struct wt_status *s)
whence = FROM_MERGE;
else if (file_exists(git_path(CHERRY_PICK_HEAD))) {
whence = FROM_CHERRY_PICK;
-   if (file_exists(git_path(sequencer)))
+   if (file_exists(git_path(SEQ_DIR)))
sequencer_in_use = 1;
}
else
-- 
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


[PATCH v5 03/28] Convert git_snpath() to strbuf_git_path()

2014-03-07 Thread Nguyễn Thái Ngọc Duy
In the previous patch, git_snpath() is modified to allocate a new
strbuf buffer because vsnpath() needs that. But that makes it awkward
because git_snpath() receives a pre-allocated buffer from outside and
has to copy data back. Rename it to strbuf_git_path() and make it
receive strbuf directly.

The conversion from git_snpath() to git_path() in
update_refs_for_switch() is safe because that function does not keep
any pointer to the round-robin buffer pool allocated by
get_pathname().

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 25 +
 cache.h|  4 +--
 path.c | 11 ++--
 refs.c | 78 +-
 refs.h |  2 +-
 5 files changed, 65 insertions(+), 55 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 64c2aca..efb5e2f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -585,18 +585,21 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (opts-new_orphan_branch) {
if (opts-new_branch_log  !log_all_ref_updates) {
int temp;
-   char log_file[PATH_MAX];
-   const char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
+   struct strbuf log_file = STRBUF_INIT;
+   int ret;
+   const char *ref_name;
 
+   ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
temp = log_all_ref_updates;
log_all_ref_updates = 1;
-   if (log_ref_setup(ref_name, log_file, 
sizeof(log_file))) {
+   ret = log_ref_setup(ref_name, log_file);
+   log_all_ref_updates = temp;
+   strbuf_release(log_file);
+   if (ret) {
fprintf(stderr, _(Can not do reflog 
for '%s'\n),
opts-new_orphan_branch);
-   log_all_ref_updates = temp;
return;
}
-   log_all_ref_updates = temp;
}
}
else
@@ -651,14 +654,10 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
new-name);
}
}
-   if (old-path  old-name) {
-   char log_file[PATH_MAX], ref_file[PATH_MAX];
-
-   git_snpath(log_file, sizeof(log_file), logs/%s, 
old-path);
-   git_snpath(ref_file, sizeof(ref_file), %s, old-path);
-   if (!file_exists(ref_file)  file_exists(log_file))
-   remove_path(log_file);
-   }
+   if (old-path  old-name 
+   !file_exists(git_path(%s, old-path)) 
+file_exists(git_path(logs/%s, old-path)))
+   remove_path(git_path(logs/%s, old-path));
}
remove_branch_state();
strbuf_release(msg);
diff --git a/cache.h b/cache.h
index a344a5f..0fae730 100644
--- a/cache.h
+++ b/cache.h
@@ -646,8 +646,8 @@ extern int check_repository_format(void);
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
__attribute__((format (printf, 3, 4)));
-extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
-   __attribute__((format (printf, 3, 4)));
+extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
+   __attribute__((format (printf, 2, 3)));
 extern char *git_pathdup(const char *fmt, ...)
__attribute__((format (printf, 1, 2)));
 extern char *mkpathdup(const char *fmt, ...)
diff --git a/path.c b/path.c
index 36d461e..417df76 100644
--- a/path.c
+++ b/path.c
@@ -70,19 +70,12 @@ static void vsnpath(struct strbuf *buf, const char *fmt, 
va_list args)
strbuf_cleanup_path(buf);
 }
 
-char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
 {
-   struct strbuf sb = STRBUF_INIT;
va_list args;
va_start(args, fmt);
-   vsnpath(sb, fmt, args);
+   vsnpath(sb, fmt, args);
va_end(args);
-   if (sb.len = n)
-   strlcpy(buf, bad_path, n);
-   else
-   memcpy(buf, sb.buf, sb.len + 1);
-   strbuf_release(sb);
-   return buf;
 }
 
 char *git_pathdup(const char *fmt, ...)
diff --git a/refs.c b/refs.c
index f846f2f..c5613b0 100644
--- a/refs.c
+++ b/refs.c
@@ -1325,10 +1325,12 @@ static const char *handle_missing_loose_ref(const char 
*refname,
 
 const char 

[PATCH v5 06/28] Make git_path() aware of file relocation in $GIT_DIR

2014-03-07 Thread Nguyễn Thái Ngọc Duy
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 pclo...@gmail.com
---
 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 path is a gitfile then the resolved path
to the real repository is printed.
 
+--git-path path::
+   Resolve $GIT_DIR/path 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.
+
 --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);
diff --git a/cache.h b/cache.h
index 0fae730..0f5f44f 100644
--- a/cache.h
+++ b/cache.h
@@ -585,6 +585,7 @@ extern int fsync_object_files;
 extern int core_preload_index;
 extern int core_apply_sparse_checkout;
 extern int precomposed_unicode;
+extern int git_db_env, git_index_env, git_graft_env;
 
 /*
  * The character that begins a commented line in user-editable file
diff --git a/environment.c b/environment.c
index 4a3437d..f513479 100644
--- a/environment.c
+++ b/environment.c
@@ -82,6 +82,7 @@ static size_t namespace_len;
 
 static const char *git_dir;
 static char *git_object_dir, *git_index_file, *git_graft_file;
+int git_db_env, git_index_env, git_graft_env;
 
 /*
  * Repository-local GIT_* environment variables; see cache.h for details.
@@ -137,15 +138,19 @@ static void setup_git_env(void)
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
sprintf(git_object_dir, %s/objects, git_dir);
-   }
+   } else
+   git_db_env = 1;
git_index_file = getenv(INDEX_ENVIRONMENT);
if (!git_index_file) {
git_index_file = xmalloc(strlen(git_dir) + 7);
sprintf(git_index_file, %s/index, git_dir);
-   }
+   } else
+   git_index_env = 1;
git_graft_file = getenv(GRAFT_ENVIRONMENT);
if (!git_graft_file)
git_graft_file = git_pathdup(info/grafts);
+   else
+   git_graft_env = 1;
if (getenv(NO_REPLACE_OBJECTS_ENVIRONMENT))
read_replace_refs = 0;
namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
diff --git a/path.c b/path.c
index 1c0f160..1069ae0 100644
--- a/path.c
+++ b/path.c
@@ -60,13 +60,58 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
return cleanup_path(buf);
 }
 
+static int dir_prefix(const char *buf, const char *dir)
+{
+   int len = strlen(dir);
+   return !strncmp(buf, dir, len) 
+   (is_dir_sep(buf[len]) || buf[len] == '\0');
+}
+
+/* $buf =~ m|$dir/+$file| but without regex */
+static int is_dir_file(const char *buf, const char *dir, const char *file)
+{
+   int len = strlen(dir);
+   if (strncmp(buf, 

[PATCH v5 07/28] *.sh: respect $GIT_INDEX_FILE

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-pull.sh  | 2 +-
 git-stash.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 0a5aa2c..2a90262 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -218,7 +218,7 @@ test true = $rebase  {
if ! git rev-parse -q --verify HEAD /dev/null
then
# On an unborn branch
-   if test -f $GIT_DIR/index
+   if test -f $(git rev-parse --git-path index)
then
die $(gettext updating an unborn branch with changes 
added to the index)
fi
diff --git a/git-stash.sh b/git-stash.sh
index f0a94ab..0de9a6c 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -20,7 +20,7 @@ require_work_tree
 cd_to_toplevel
 
 TMP=$GIT_DIR/.git-stash.$$
-TMPindex=${GIT_INDEX_FILE-$GIT_DIR/index}.stash.$$
+TMPindex=${GIT_INDEX_FILE-$(git rev-parse --git-path index)}.stash.$$
 trap 'rm -f $TMP-* $TMPindex' 0
 
 ref_stash=refs/stash
-- 
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


[PATCH v5 09/28] fast-import: use git_path() for accessing .git dir instead of get_git_dir()

2014-03-07 Thread Nguyễn Thái Ngọc Duy
This allows git_path() to redirect info/fast-import to another place
if needed

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 fast-import.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index a9f328d..28e7a63 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -3125,12 +3125,9 @@ static void parse_progress(void)
 
 static char* make_fast_import_path(const char *path)
 {
-   struct strbuf abs_path = STRBUF_INIT;
-
if (!relative_marks_paths || is_absolute_path(path))
return xstrdup(path);
-   strbuf_addf(abs_path, %s/info/fast-import/%s, get_git_dir(), path);
-   return strbuf_detach(abs_path, NULL);
+   return xstrdup(git_path(info/fast-import/%s, path));
 }
 
 static void option_import_marks(const char *marks,
-- 
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


[PATCH v5 02/28] path.c: make get_pathname() call sites return const char *

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Before the previous commit, get_pathname returns an array of PATH_MAX
length. Even if git_path() and similar functions does not use the
whole array, git_path() caller can, in theory.

After the commit, get_pathname() may return a buffer that has just
enough room for the returned string and git_path() caller should never
write beyond that.

Make git_path(), mkpath() and git_path_submodule() return a const
buffer to make sure callers do not write in it at all.

This could have been part of the previous commit, but the const
conversion is too much distraction from the core changes in path.c.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 2 +-
 builtin/clone.c| 9 +
 builtin/fetch.c| 5 +++--
 builtin/fsck.c | 4 ++--
 builtin/receive-pack.c | 2 +-
 builtin/remote.c   | 2 +-
 builtin/repack.c   | 8 +---
 cache.h| 6 +++---
 fast-import.c  | 2 +-
 notes-merge.c  | 6 +++---
 path.c | 6 +++---
 refs.c | 8 
 run-command.c  | 4 ++--
 run-command.h  | 2 +-
 sha1_file.c| 2 +-
 15 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5df3837..64c2aca 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -586,7 +586,7 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (opts-new_branch_log  !log_all_ref_updates) {
int temp;
char log_file[PATH_MAX];
-   char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
+   const char *ref_name = mkpath(refs/heads/%s, 
opts-new_orphan_branch);
 
temp = log_all_ref_updates;
log_all_ref_updates = 1;
diff --git a/builtin/clone.c b/builtin/clone.c
index 43e772c..9339371 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -289,16 +289,17 @@ static void copy_alternates(struct strbuf *src, struct 
strbuf *dst,
struct strbuf line = STRBUF_INIT;
 
while (strbuf_getline(line, in, '\n') != EOF) {
-   char *abs_path, abs_buf[PATH_MAX];
+   char *abs_path;
if (!line.len || line.buf[0] == '#')
continue;
if (is_absolute_path(line.buf)) {
add_to_alternates_file(line.buf);
continue;
}
-   abs_path = mkpath(%s/objects/%s, src_repo, line.buf);
-   normalize_path_copy(abs_buf, abs_path);
-   add_to_alternates_file(abs_buf);
+   abs_path = mkpathdup(%s/objects/%s, src_repo, line.buf);
+   normalize_path_copy(abs_path, abs_path);
+   add_to_alternates_file(abs_path);
+   free(abs_path);
}
strbuf_release(line);
fclose(in);
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 025bc3e..04b51ca 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -544,7 +544,8 @@ static int store_updated_refs(const char *raw_url, const 
char *remote_name,
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
-   char *url, *filename = dry_run ? /dev/null : git_path(FETCH_HEAD);
+   char *url;
+   const char *filename = dry_run ? /dev/null : git_path(FETCH_HEAD);
int want_status;
 
fp = fopen(filename, a);
@@ -778,7 +779,7 @@ static void check_not_current_branch(struct ref *ref_map)
 
 static int truncate_fetch_head(void)
 {
-   char *filename = git_path(FETCH_HEAD);
+   const char *filename = git_path(FETCH_HEAD);
FILE *fp = fopen(filename, w);
 
if (!fp)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1affdd5..643c980 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -225,12 +225,12 @@ static void check_unreachable_object(struct object *obj)
printf(dangling %s %s\n, typename(obj-type),
   sha1_to_hex(obj-sha1));
if (write_lost_and_found) {
-   char *filename = git_path(lost-found/%s/%s,
+   const char *filename = git_path(lost-found/%s/%s,
obj-type == OBJ_COMMIT ? commit : other,
sha1_to_hex(obj-sha1));
FILE *f;
 
-   if (safe_create_leading_directories(filename)) {
+   if (safe_create_leading_directories_const(filename)) {
error(Could not create lost-found);
return;
}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 85bba35..4d27acb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -599,7 +599,7 @@ static void 

[PATCH v5 05/28] path.c: group git_path(), git_pathdup() and strbuf_git_path() together

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 path.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/path.c b/path.c
index 987fdec..1c0f160 100644
--- a/path.c
+++ b/path.c
@@ -78,6 +78,16 @@ void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
va_end(args);
 }
 
+const char *git_path(const char *fmt, ...)
+{
+   struct strbuf *pathname = get_pathname();
+   va_list args;
+   va_start(args, fmt);
+   do_git_path(pathname, fmt, args);
+   va_end(args);
+   return pathname-buf;
+}
+
 char *git_pathdup(const char *fmt, ...)
 {
struct strbuf path = STRBUF_INIT;
@@ -109,16 +119,6 @@ const char *mkpath(const char *fmt, ...)
return cleanup_path(pathname-buf);
 }
 
-const char *git_path(const char *fmt, ...)
-{
-   struct strbuf *pathname = get_pathname();
-   va_list args;
-   va_start(args, fmt);
-   do_git_path(pathname, fmt, args);
-   va_end(args);
-   return pathname-buf;
-}
-
 void home_config_paths(char **global, char **xdg, char *file)
 {
char *xdg_home = getenv(XDG_CONFIG_HOME);
-- 
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


[PATCH v5 19/28] setup.c: support multi-checkout repo setup

2014-03-07 Thread Nguyễn Thái Ngọc Duy
The repo setup procedure is updated to detect $GIT_DIR/commondir and
set $GIT_COMMON_DIR properly.

The core.worktree is ignored when $GIT_COMMON_DIR is set. This is
because the config file is shared in multi-checkout setup, but
checkout directories _are_ different. Making core.worktree effective
in all checkouts mean it's back to a single checkout.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt|  2 ++
 Documentation/git-rev-parse.txt |  3 ++
 builtin/rev-parse.c |  4 +++
 cache.h |  1 +
 environment.c   |  8 ++---
 setup.c | 33 +-
 t/t1501-worktree.sh | 76 +
 t/t1510-repo-setup.sh   |  1 +
 trace.c |  1 +
 9 files changed, 115 insertions(+), 14 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..313d4b3 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -381,6 +381,8 @@ false), while all other repositories are assumed to be bare 
(bare
 
 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.
This can be overridden by the GIT_WORK_TREE environment
variable and the '--work-tree' command line option.
The value can be an absolute path or relative to the path to
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 46020d9..8a9cac8 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -215,6 +215,9 @@ If `$GIT_DIR` is not defined and the current directory
 is not detected to lie in a Git repository or work tree
 print a message to stderr and exit with nonzero status.
 
+--git-common-dir::
+   Show `$GIT_COMMON_DIR` if defined, else `$GIT_DIR`.
+
 --is-inside-git-dir::
When the current working directory is below the repository
directory print true, otherwise false.
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index e50bc65..c7057ce 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -744,6 +744,10 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
printf(%s%s.git\n, cwd, len  cwd[len-1] != 
'/' ? / : );
continue;
}
+   if (!strcmp(arg, --git-common-dir)) {
+   puts(get_git_common_dir());
+   continue;
+   }
if (!strcmp(arg, --resolve-git-dir)) {
const char *gitdir = resolve_gitdir(argv[i+1]);
if (!gitdir)
diff --git a/cache.h b/cache.h
index 9182427..992e241 100644
--- a/cache.h
+++ b/cache.h
@@ -407,6 +407,7 @@ extern char *get_object_directory(void);
 extern char *get_index_file(void);
 extern char *get_graft_file(void);
 extern int set_git_dir(const char *path);
+extern int get_common_dir(struct strbuf *sb, const char *gitdir);
 extern const char *get_git_namespace(void);
 extern const char *strip_namespace(const char *namespaced_ref);
 extern const char *get_git_work_tree(void);
diff --git a/environment.c b/environment.c
index c998120..0999fc1 100644
--- a/environment.c
+++ b/environment.c
@@ -126,6 +126,7 @@ static char *expand_namespace(const char *raw_namespace)
 
 static void setup_git_env(void)
 {
+   struct strbuf sb = STRBUF_INIT;
const char *gitfile;
const char *shallow_file;
 
@@ -134,12 +135,9 @@ static void setup_git_env(void)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
gitfile = read_gitfile(git_dir);
git_dir = xstrdup(gitfile ? gitfile : git_dir);
-   git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
-   if (git_common_dir) {
+   if (get_common_dir(sb, git_dir))
git_common_dir_env = 1;
-   git_common_dir = xstrdup(git_common_dir);
-   } else
-   git_common_dir = git_dir;
+   git_common_dir = strbuf_detach(sb, NULL);
git_object_dir = getenv(DB_ENVIRONMENT);
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_common_dir) + 9);
diff --git a/setup.c b/setup.c
index 42849f3..40ce191 100644
--- a/setup.c
+++ b/setup.c
@@ -170,14 +170,15 @@ void verify_non_filename(const char *prefix, const char 
*arg)
'git command [revision...] -- [file...]', arg);
 }
 
-static void get_common_dir(struct strbuf *sb, const char *gitdir)
+int get_common_dir(struct strbuf *sb, const char *gitdir)
 {
struct strbuf data = STRBUF_INIT;
struct strbuf path = STRBUF_INIT;
const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+   int ret = 0;
if (git_common_dir) {

[PATCH v5 13/28] *.sh: avoid hardcoding $GIT_DIR/hooks/...

2014-03-07 Thread Nguyễn Thái Ngọc Duy
If $GIT_COMMON_DIR is set, it should be $GIT_COMMON_DIR/hooks/, not
$GIT_DIR/hooks/. Just let rev-parse --git-path handle it.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-am.sh  | 22 +++---
 git-rebase--interactive.sh |  6 +++---
 git-rebase--merge.sh   |  6 ++
 git-rebase.sh  |  4 ++--
 templates/hooks--applypatch-msg.sample |  4 ++--
 templates/hooks--pre-applypatch.sample |  4 ++--
 6 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index bbea430..10babd2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -803,10 +803,10 @@ To restore the original branch and stop patching run 
\\$cmdline --abort\.
continue
fi
 
-   if test -x $GIT_DIR/hooks/applypatch-msg
+   hook=$(git rev-parse --git-path hooks/applypatch-msg)
+   if test -x $hook
then
-   $GIT_DIR/hooks/applypatch-msg $dotest/final-commit ||
-   stop_here $this
+   $hook $dotest/final-commit || stop_here $this
fi
 
if test -f $dotest/final-commit
@@ -880,9 +880,10 @@ did you forget to use 'git add'?
stop_here_user_resolve $this
fi
 
-   if test -x $GIT_DIR/hooks/pre-applypatch
+   hook=$(git rev-parse --git-path hooks/pre-applypatch)
+   if test -x $hook
then
-   $GIT_DIR/hooks/pre-applypatch || stop_here $this
+   $hook || stop_here $this
fi
 
tree=$(git write-tree) 
@@ -908,18 +909,17 @@ did you forget to use 'git add'?
echo $(cat $dotest/original-commit) $commit  
$dotest/rewritten
fi
 
-   if test -x $GIT_DIR/hooks/post-applypatch
-   then
-   $GIT_DIR/hooks/post-applypatch
-   fi
+   hook=$(git rev-parse --git-path hooks/post-applypatch)
+   test -x $hook  $hook
 
go_next
 done
 
 if test -s $dotest/rewritten; then
 git notes copy --for-rewrite=rebase  $dotest/rewritten
-if test -x $GIT_DIR/hooks/post-rewrite; then
-   $GIT_DIR/hooks/post-rewrite rebase  $dotest/rewritten
+hook=$(git rev-parse --git-path hooks/post-rewrite)
+if test -x $hook; then
+   $hook rebase  $dotest/rewritten
 fi
 fi
 
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43c19e0..dd84f9d 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -632,9 +632,9 @@ do_next () {
git notes copy --for-rewrite=rebase  $rewritten_list ||
true # we don't care if this copying failed
} 
-   if test -x $GIT_DIR/hooks/post-rewrite 
-   test -s $rewritten_list; then
-   $GIT_DIR/hooks/post-rewrite rebase  $rewritten_list
+   hook=$(git rev-parse --git-path hooks/post-rewrite)
+   if test -x $hook  test -s $rewritten_list; then
+   $hook rebase  $rewritten_list
true # we don't care if this hook failed
fi 
warn Successfully rebased and updated $head_name.
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index e7d96de..93d7bcf 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -93,10 +93,8 @@ finish_rb_merge () {
if test -s $state_dir/rewritten
then
git notes copy --for-rewrite=rebase $state_dir/rewritten
-   if test -x $GIT_DIR/hooks/post-rewrite
-   then
-   $GIT_DIR/hooks/post-rewrite rebase 
$state_dir/rewritten
-   fi
+   hook=$(git rev-parse --git-path hooks/post-rewrite)
+   test -x $hook  $hook rebase $state_dir/rewritten
fi
say All done.
 }
diff --git a/git-rebase.sh b/git-rebase.sh
index 8a3efa2..b90a6d9 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -195,9 +195,9 @@ run_specific_rebase () {
 
 run_pre_rebase_hook () {
if test -z $ok_to_skip_pre_rebase 
-  test -x $GIT_DIR/hooks/pre-rebase
+  test -x $(git rev-parse --git-path hooks/pre-rebase)
then
-   $GIT_DIR/hooks/pre-rebase ${1+$@} ||
+   $(git rev-parse --git-path hooks/pre-rebase) ${1+$@} ||
die $(gettext The pre-rebase hook refused to rebase.)
fi
 }
diff --git a/templates/hooks--applypatch-msg.sample 
b/templates/hooks--applypatch-msg.sample
index 8b2a2fe..a5d7b84 100755
--- a/templates/hooks--applypatch-msg.sample
+++ b/templates/hooks--applypatch-msg.sample
@@ -10,6 +10,6 @@
 # To enable this hook, rename this file to applypatch-msg.
 
 . git-sh-setup
-test -x $GIT_DIR/hooks/commit-msg 
-   exec $GIT_DIR/hooks/commit-msg ${1+$@}
+commitmsg=$(git rev-parse --git-path hooks/commit-msg)
+test -x $commitmsg  exec $commitmsg ${1+$@}
 :
diff --git a/templates/hooks--pre-applypatch.sample 
b/templates/hooks--pre-applypatch.sample
index b1f187c..4142082 100755
--- a/templates/hooks--pre-applypatch.sample
+++ 

[PATCH v5 15/28] setup.c: convert is_git_directory() to use strbuf

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 37 +
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/setup.c b/setup.c
index 6c3f85f..4994437 100644
--- a/setup.c
+++ b/setup.c
@@ -184,31 +184,36 @@ void verify_non_filename(const char *prefix, const char 
*arg)
  */
 int is_git_directory(const char *suspect)
 {
-   char path[PATH_MAX];
-   size_t len = strlen(suspect);
+   struct strbuf path = STRBUF_INIT;
+   int ret = 0;
+   size_t len;
 
-   if (PATH_MAX = len + strlen(/objects))
-   die(Too long path: %.*s, 60, suspect);
-   strcpy(path, suspect);
+   strbuf_addstr(path, suspect);
+   len = path.len;
if (getenv(DB_ENVIRONMENT)) {
if (access(getenv(DB_ENVIRONMENT), X_OK))
-   return 0;
+   goto done;
}
else {
-   strcpy(path + len, /objects);
-   if (access(path, X_OK))
-   return 0;
+   strbuf_addstr(path, /objects);
+   if (access(path.buf, X_OK))
+   goto done;
}
 
-   strcpy(path + len, /refs);
-   if (access(path, X_OK))
-   return 0;
+   strbuf_setlen(path, len);
+   strbuf_addstr(path, /refs);
+   if (access(path.buf, X_OK))
+   goto done;
 
-   strcpy(path + len, /HEAD);
-   if (validate_headref(path))
-   return 0;
+   strbuf_setlen(path, len);
+   strbuf_addstr(path, /HEAD);
+   if (validate_headref(path.buf))
+   goto done;
 
-   return 1;
+   ret = 1;
+done:
+   strbuf_release(path);
+   return ret;
 }
 
 int is_inside_git_dir(void)
-- 
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


[PATCH v5 17/28] setup.c: convert check_repository_format_gently to use strbuf

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/setup.c b/setup.c
index 7e5b334..5085ab1 100644
--- a/setup.c
+++ b/setup.c
@@ -288,7 +288,9 @@ void setup_work_tree(void)
 
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
 {
-   char repo_config[PATH_MAX+1];
+   struct strbuf sb = STRBUF_INIT;
+   const char *repo_config;
+   int ret = 0;
 
/*
 * git_config() can't be used here because it calls git_pathdup()
@@ -299,7 +301,8 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
 * Use a gentler version of git_config() to check if this repo
 * is a good one.
 */
-   snprintf(repo_config, PATH_MAX, %s/config, gitdir);
+   strbuf_addf(sb, %s/config, gitdir);
+   repo_config = sb.buf;
git_config_early(check_repository_format_version, NULL, repo_config);
if (GIT_REPO_VERSION  repository_format_version) {
if (!nongit_ok)
@@ -309,9 +312,10 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
GIT_REPO_VERSION, repository_format_version);
warning(Please upgrade Git);
*nongit_ok = -1;
-   return -1;
+   ret = -1;
}
-   return 0;
+   strbuf_release(sb);
+   return ret;
 }
 
 /*
-- 
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


[PATCH v5 18/28] setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 setup.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/setup.c b/setup.c
index 5085ab1..42849f3 100644
--- a/setup.c
+++ b/setup.c
@@ -292,6 +292,10 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
const char *repo_config;
int ret = 0;
 
+   get_common_dir(sb, gitdir);
+   strbuf_addstr(sb, /config);
+   repo_config = sb.buf;
+
/*
 * git_config() can't be used here because it calls git_pathdup()
 * to get $GIT_CONFIG/config. That call will make setup_git_env()
@@ -301,8 +305,6 @@ static int check_repository_format_gently(const char 
*gitdir, int *nongit_ok)
 * Use a gentler version of git_config() to check if this repo
 * is a good one.
 */
-   strbuf_addf(sb, %s/config, gitdir);
-   repo_config = sb.buf;
git_config_early(check_repository_format_version, NULL, repo_config);
if (GIT_REPO_VERSION  repository_format_version) {
if (!nongit_ok)
-- 
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


[PATCH v5 20/28] wrapper.c: wrapper to open a file, fprintf then close

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 cache.h   |  2 ++
 wrapper.c | 31 +++
 2 files changed, 33 insertions(+)

diff --git a/cache.h b/cache.h
index 992e241..979ac6e 100644
--- a/cache.h
+++ b/cache.h
@@ -1239,6 +1239,8 @@ static inline ssize_t write_str_in_full(int fd, const 
char *str)
 {
return write_in_full(fd, str, strlen(str));
 }
+__attribute__((format (printf,3,4)))
+extern int write_file(const char *path, int fatal, const char *fmt, ...);
 
 /* pager.c */
 extern void setup_pager(void);
diff --git a/wrapper.c b/wrapper.c
index 0cc5636..5ad628c 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -455,3 +455,34 @@ struct passwd *xgetpwuid_self(void)
errno ? strerror(errno) : _(no such user));
return pw;
 }
+
+int write_file(const char *path, int fatal, const char *fmt, ...)
+{
+   struct strbuf sb = STRBUF_INIT;
+   va_list params;
+   int fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0666);
+   if (fd  0) {
+   if (fatal)
+   die_errno(_(could not open %s for writing), path);
+   return -1;
+   }
+   va_start(params, fmt);
+   strbuf_vaddf(sb, fmt, params);
+   va_end(params);
+   if (write_in_full(fd, sb.buf, sb.len) != sb.len) {
+   int err = errno;
+   close(fd);
+   strbuf_release(sb);
+   errno = err;
+   if (fatal)
+   die_errno(_(could not write to %s), path);
+   return -1;
+   }
+   strbuf_release(sb);
+   if (close(fd)) {
+   if (fatal)
+   die_errno(_(could not close %s), path);
+   return -1;
+   }
+   return 0;
+}
-- 
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


[PATCH v5 14/28] git-stash: avoid hardcoding $GIT_DIR/logs/....

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 git-stash.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 0de9a6c..8093326 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -183,7 +183,7 @@ store_stash () {
fi
 
# Make sure the reflog for stash is kept.
-   : $GIT_DIR/logs/$ref_stash
+   : $(git rev-parse --git-path logs/$ref_stash)
git update-ref -m $stash_msg $ref_stash $w_commit
ret=$?
test $ret != 0  test -z $quiet 
@@ -258,7 +258,7 @@ save_stash () {
say $(gettext No local changes to save)
exit 0
fi
-   test -f $GIT_DIR/logs/$ref_stash ||
+   test -f $(git rev-parse --git-path logs/$ref_stash) ||
clear_stash || die $(gettext Cannot initialize stash)
 
create_stash $stash_msg $untracked
-- 
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


[PATCH v5 16/28] setup.c: detect $GIT_COMMON_DIR in is_git_directory()

2014-03-07 Thread Nguyễn Thái Ngọc Duy
If the file $GIT_DIR/commondir exists, it contains the value of
$GIT_COMMON_DIR.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/gitrepository-layout.txt |  7 ++
 setup.c| 43 +-
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 556da09..c6f3649 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -233,6 +233,13 @@ shallow::
file is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/shallow will be used instead.
 
+commondir::
+   If this file exists, $GIT_COMMON_DIR (see linkgit:git[1]) will
+   be set to the path specified in this file if it is not
+   explicitly set. If the specified path is relative, it is
+   relative to $GIT_DIR. The repository with commondir is
+   incomplete without the repository pointed by commondir.
+
 modules::
Contains the git-repositories of the submodules. This
directory is ignored if $GIT_COMMON_DIR is set and
diff --git a/setup.c b/setup.c
index 4994437..7e5b334 100644
--- a/setup.c
+++ b/setup.c
@@ -170,6 +170,33 @@ void verify_non_filename(const char *prefix, const char 
*arg)
'git command [revision...] -- [file...]', arg);
 }
 
+static void get_common_dir(struct strbuf *sb, const char *gitdir)
+{
+   struct strbuf data = STRBUF_INIT;
+   struct strbuf path = STRBUF_INIT;
+   const char *git_common_dir = getenv(GIT_COMMON_DIR_ENVIRONMENT);
+   if (git_common_dir) {
+   strbuf_addstr(sb, git_common_dir);
+   return;
+   }
+   strbuf_addf(path, %s/commondir, gitdir);
+   if (file_exists(path.buf)) {
+   if (strbuf_read_file(data, path.buf, 0) = 0)
+   die_errno(_(failed to read %s), path.buf);
+   while (data.len  (data.buf[data.len - 1] == '\n' ||
+   data.buf[data.len - 1] == '\r'))
+   data.len--;
+   data.buf[data.len] = '\0';
+   strbuf_reset(path);
+   if (!is_absolute_path(data.buf))
+   strbuf_addf(path, %s/, gitdir);
+   strbuf_addbuf(path, data);
+   strbuf_addstr(sb, real_path(path.buf));
+   } else
+   strbuf_addstr(sb, gitdir);
+   strbuf_release(data);
+   strbuf_release(path);
+}
 
 /*
  * Test if it looks like we're at a git directory.
@@ -188,13 +215,22 @@ int is_git_directory(const char *suspect)
int ret = 0;
size_t len;
 
-   strbuf_addstr(path, suspect);
+   /* Check worktree-related signatures */
+   strbuf_addf(path, %s/HEAD, suspect);
+   if (validate_headref(path.buf))
+   goto done;
+
+   strbuf_reset(path);
+   get_common_dir(path, suspect);
len = path.len;
+
+   /* Check non-worktree-related signatures */
if (getenv(DB_ENVIRONMENT)) {
if (access(getenv(DB_ENVIRONMENT), X_OK))
goto done;
}
else {
+   strbuf_setlen(path, len);
strbuf_addstr(path, /objects);
if (access(path.buf, X_OK))
goto done;
@@ -205,11 +241,6 @@ int is_git_directory(const char *suspect)
if (access(path.buf, X_OK))
goto done;
 
-   strbuf_setlen(path, len);
-   strbuf_addstr(path, /HEAD);
-   if (validate_headref(path.buf))
-   goto done;
-
ret = 1;
 done:
strbuf_release(path);
-- 
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


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

2014-03-07 Thread Nguyễn Thái Ngọc Duy
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 pclo...@gmail.com
---
 builtin/count-objects.c |  4 +++-
 cache.h |  1 +
 path.c  | 29 +++--
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a7f70cb..d3a1620 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -102,8 +102,10 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
-   if (verbose)
+   if (verbose) {
report_garbage = real_report_garbage;
+   report_linked_checkout_garbage();
+   }
memcpy(path, objdir, len);
if (len  objdir[len-1] != '/')
path[len++] = '/';
diff --git a/cache.h b/cache.h
index 979ac6e..a0ff430 100644
--- a/cache.h
+++ b/cache.h
@@ -662,6 +662,7 @@ extern const char *mkpath(const char *fmt, ...) 
__attribute__((format (printf, 1
 extern const char *git_path(const char *fmt, ...) __attribute__((format 
(printf, 1, 2)));
 extern const char *git_path_submodule(const char *path, const char *fmt, ...)
__attribute__((format (printf, 2, 3)));
+extern void report_linked_checkout_garbage(void);
 
 extern char *sha1_file_name(const unsigned char *sha1);
 extern char *sha1_pack_name(const unsigned char *sha1);
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)
 }
 
 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


[PATCH v5 27/28] gc: support prune --repos

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/config.txt |  7 +++
 builtin/gc.c | 17 +
 2 files changed, 24 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 313d4b3..c635b8d 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1183,6 +1183,13 @@ gc.pruneexpire::
now may be used to disable this  grace period and always prune
unreachable objects immediately.
 
+gc.prunereposexpire::
+   When 'git gc' is run, it will call
+   'prune --repos --expire 3.months.ago'.
+   Override the grace period with this config variable. The value
+   now may be used to disable the grace period and always prune
+   $GIT_DIR/repos immediately.
+
 gc.reflogexpire::
 gc.pattern.reflogexpire::
'git reflog expire' removes reflog entries older than
diff --git a/builtin/gc.c b/builtin/gc.c
index 39d9b27..85c3c0c 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -30,11 +30,13 @@ static int aggressive_window = 250;
 static int gc_auto_threshold = 6700;
 static int gc_auto_pack_limit = 50;
 static const char *prune_expire = 2.weeks.ago;
+static const char *prune_repos_expire = 3.months.ago;
 
 static struct argv_array pack_refs_cmd = ARGV_ARRAY_INIT;
 static struct argv_array reflog = ARGV_ARRAY_INIT;
 static struct argv_array repack = ARGV_ARRAY_INIT;
 static struct argv_array prune = ARGV_ARRAY_INIT;
+static struct argv_array prune_repos = ARGV_ARRAY_INIT;
 static struct argv_array rerere = ARGV_ARRAY_INIT;
 
 static char *pidfile;
@@ -81,6 +83,14 @@ static int gc_config(const char *var, const char *value, 
void *cb)
}
return git_config_string(prune_expire, var, value);
}
+   if (!strcmp(var, gc.prunereposexpire)) {
+   if (value  strcmp(value, now)) {
+   unsigned long now = approxidate(now);
+   if (approxidate(value) = now)
+   return error(_(Invalid %s: '%s'), var, value);
+   }
+   return git_config_string(prune_repos_expire, var, value);
+   }
return git_default_config(var, value, cb);
 }
 
@@ -274,6 +284,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_pushl(reflog, reflog, expire, --all, NULL);
argv_array_pushl(repack, repack, -d, -l, NULL);
argv_array_pushl(prune, prune, --expire, NULL);
+   argv_array_pushl(prune_repos, prune, --repos, --expire, NULL);
argv_array_pushl(rerere, rerere, gc, NULL);
 
git_config(gc_config, NULL);
@@ -334,6 +345,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
return error(FAILED_RUN, prune.argv[0]);
}
 
+   if (prune_repos_expire) {
+   argv_array_push(prune_repos, prune_repos_expire);
+   if (run_command_v_opt(prune_repos.argv, RUN_GIT_CMD))
+   return error(FAILED_RUN, prune_repos.argv[0]);
+   }
+
if (run_command_v_opt(rerere.argv, RUN_GIT_CMD))
return error(FAILED_RUN, rerere.argv[0]);
 
-- 
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


[PATCH v5 22/28] checkout: support checking out into a new working directory

2014-03-07 Thread Nguyễn Thái Ngọc Duy
git checkout --to sets up a new working directory with a .git file
pointing to $GIT_DIR/repos/id. It then executes git checkout again
on the new worktree with the same arguments except --to is taken
out. The second checkout execution, which is not contaminated with any
info from the current repository, will actually check out and
everything that normal git checkout does.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-checkout.txt | 34 +
 Documentation/git.txt  |  3 +-
 Documentation/gitrepository-layout.txt |  7 +++
 builtin/checkout.c | 93 +-
 path.c |  2 +-
 t/t2025-checkout-to.sh (new +x)| 48 ++
 6 files changed, 183 insertions(+), 4 deletions(-)
 create mode 100755 t/t2025-checkout-to.sh

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 33ad2ad..fcf73b2 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -225,6 +225,13 @@ This means that you can use `git checkout -p` to 
selectively discard
 edits from your current working tree. See the ``Interactive Mode''
 section of linkgit:git-add[1] to learn how to operate the `--patch` mode.
 
+--to=path::
+   Check out a new branch in a separate working directory at
+   `path`. A new working directory is linked to the current
+   repository, sharing everything except working directory
+   specific files such as HEAD, index... See MULTIPLE CHECKOUT
+   MODE section for more information.
+
 branch::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with refs/heads/, is a valid ref), then that
@@ -388,6 +395,33 @@ $ git reflog -2 HEAD # or
 $ git log -g -2 HEAD
 
 
+MULTIPLE CHECKOUT MODE
+---
+Normally a working directory is attached to repository. When git
+checkout --to is used, a new working directory is attached to the
+current repository. This new working directory is called linked
+checkout as compared to the main checkout prepared by git init or
+git clone. A repository has one main checkout and zero or more
+linked checkouts.
+
+All checkouts share the same repository. Linked checkouts see the
+repository a bit different from the main checkout. When the checkout
+new reads the path $GIT_DIR/HEAD for example, the actual path
+returned could be $GIT_DIR/repos/new/HEAD. This ensures checkouts
+won't step on each other.
+
+Each linked checkout has a private space in $GIT_DIR/repos, usually
+named after the base name of the working directory with a number added
+to make it unique. The linked checkout's $GIT_DIR points to this
+private space while $GIT_COMMON_DIR points to the main checkout's
+$GIT_DIR. These settings are done by git checkout --to.
+
+Because in this mode $GIT_DIR becomes a lightweight virtual file
+system where a path could be rewritten to some place else, accessing
+$GIT_DIR from scripts should use `git rev-parse --git-path` to resolve
+a path instead of using it directly unless the path is known to be
+private to the working directory.
+
 EXAMPLES
 
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index b094b1f..bdb9b0f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -777,7 +777,8 @@ Git so take care if using Cogito etc.
If this variable is set to a path, non-worktree files that are
normally in $GIT_DIR will be taken from this path
instead. Worktree-specific files such as HEAD or index are
-   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] for
+   taken from $GIT_DIR. See linkgit:gitrepository-layout[5] and
+   the section 'MULTIPLE CHECKOUT MODE' in linkgit:checkout[1]
details. This variable has lower precedence than other path
variables such as GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY...
 
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index c6f3649..2206f35 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -245,6 +245,13 @@ modules::
directory is ignored if $GIT_COMMON_DIR is set and
$GIT_COMMON_DIR/modules will be used instead.
 
+repos::
+   Contains worktree specific information of linked
+   checkouts. Each subdirectory contains the worktree-related
+   part of a linked checkout. This directory is ignored
+   $GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be
+   used instead.
+
 SEE ALSO
 
 linkgit:git-init[1],
diff --git a/builtin/checkout.c b/builtin/checkout.c
index efb5e2f..aa4ffd1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -48,6 +48,10 @@ struct checkout_opts {
const char *prefix;
struct pathspec pathspec;
struct tree *source_tree;
+
+   const char *new_worktree;
+   const char **saved_argv;
+   

[PATCH v5 23/28] checkout: clean up half-prepared directories in --to mode

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 49 +++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index aa4ffd1..97fdfcc 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -20,6 +20,7 @@
 #include resolve-undo.h
 #include submodule.h
 #include argv-array.h
+#include sigchain.h
 
 static const char * const checkout_usage[] = {
N_(git checkout [options] branch),
@@ -815,6 +816,35 @@ static int switch_branches(const struct checkout_opts 
*opts,
return ret || writeout_error;
 }
 
+static const char *junk_work_tree;
+static const char *junk_git_dir;
+static int is_junk;
+static pid_t junk_pid;
+
+static void remove_junk(void)
+{
+   struct strbuf sb = STRBUF_INIT;
+   if (!is_junk || getpid() != junk_pid)
+   return;
+   if (junk_git_dir) {
+   strbuf_addstr(sb, junk_git_dir);
+   remove_dir_recursively(sb, 0);
+   strbuf_reset(sb);
+   }
+   if (junk_work_tree) {
+   strbuf_addstr(sb, junk_work_tree);
+   remove_dir_recursively(sb, 0);
+   }
+   strbuf_release(sb);
+}
+
+static void remove_junk_on_signal(int signo)
+{
+   remove_junk();
+   sigchain_pop(signo);
+   raise(signo);
+}
+
 static int prepare_linked_checkout(const struct checkout_opts *opts,
   struct branch_info *new)
 {
@@ -823,7 +853,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
const char *path = opts-new_worktree, *name;
struct stat st;
struct child_process cp;
-   int counter = 0, len;
+   int counter = 0, len, ret;
 
if (!new-commit)
die(_(no branch specified));
@@ -849,13 +879,21 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
strbuf_addf(sb_repo, %d, counter);
}
name = strrchr(sb_repo.buf, '/') + 1;
+
+   junk_pid = getpid();
+   atexit(remove_junk);
+   sigchain_push_common(remove_junk_on_signal);
+
if (mkdir(sb_repo.buf, 0777))
die_errno(_(could not create directory of '%s'), sb_repo.buf);
+   junk_git_dir = sb_repo.buf;
+   is_junk = 1;
 
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;
 
write_file(sb_git.buf, 1, gitdir: %s/repos/%s\n,
   real_path(get_git_common_dir()), name);
@@ -880,7 +918,14 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
memset(cp, 0, sizeof(cp));
cp.git_cmd = 1;
cp.argv = opts-saved_argv;
-   return run_command(cp);
+   ret = run_command(cp);
+   if (!ret)
+   is_junk = 0;
+   strbuf_release(sb);
+   strbuf_release(sb_repo);
+   strbuf_release(sb_git);
+   return ret;
+
 }
 
 static int git_checkout_config(const char *var, const char *value, void *cb)
-- 
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


[PATCH v5 21/28] use new wrapper write_file() for simple file writing

2014-03-07 Thread Nguyễn Thái Ngọc Duy
This fixes common problems in these code about error handling,
forgetting to close the file handle after fprintf() fails, or not
printing out the error string..

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/branch.c  |  4 +---
 builtin/init-db.c |  7 +--
 daemon.c  | 11 +--
 submodule.c   |  9 ++---
 transport.c   |  8 +++-
 5 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index b4d7716..3eebdbc 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -754,7 +754,6 @@ static const char edit_description[] = BRANCH_DESCRIPTION;
 
 static int edit_branch_description(const char *branch_name)
 {
-   FILE *fp;
int status;
struct strbuf buf = STRBUF_INIT;
struct strbuf name = STRBUF_INIT;
@@ -767,8 +766,7 @@ static int edit_branch_description(const char *branch_name)
  %s\n
Lines starting with '%c' will be stripped.\n,
branch_name, comment_line_char);
-   fp = fopen(git_path(edit_description), w);
-   if ((fwrite(buf.buf, 1, buf.len, fp)  buf.len) || fclose(fp)) {
+   if (write_file(git_path(edit_description), 0, %s, buf.buf)) {
strbuf_release(buf);
return error(_(could not write branch description template: 
%s),
 strerror(errno));
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c7c76bb..081e512 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -342,7 +342,6 @@ int set_git_dir_init(const char *git_dir, const char 
*real_git_dir,
 static void separate_git_dir(const char *git_dir)
 {
struct stat st;
-   FILE *fp;
 
if (!stat(git_link, st)) {
const char *src;
@@ -358,11 +357,7 @@ static void separate_git_dir(const char *git_dir)
die_errno(_(unable to move %s to %s), src, git_dir);
}
 
-   fp = fopen(git_link, w);
-   if (!fp)
-   die(_(Could not create git link %s), git_link);
-   fprintf(fp, gitdir: %s\n, git_dir);
-   fclose(fp);
+   write_file(git_link, 1, gitdir: %s\n, git_dir);
 }
 
 int init_db(const char *template_dir, unsigned int flags)
diff --git a/daemon.c b/daemon.c
index 503e039..b880d30 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1122,15 +1122,6 @@ static void daemonize(void)
 }
 #endif
 
-static void store_pid(const char *path)
-{
-   FILE *f = fopen(path, w);
-   if (!f)
-   die_errno(cannot open pid file '%s', path);
-   if (fprintf(f, %PRIuMAX\n, (uintmax_t) getpid())  0 || fclose(f) 
!= 0)
-   die_errno(failed to write pid file '%s', path);
-}
-
 static int serve(struct string_list *listen_addr, int listen_port,
 struct credentials *cred)
 {
@@ -1339,7 +1330,7 @@ int main(int argc, char **argv)
sanitize_stdfds();
 
if (pid_file)
-   store_pid(pid_file);
+   write_file(pid_file, 1, %PRIuMAX\n, (uintmax_t) getpid());
 
/* prepare argv for serving-processes */
cld_argv = xmalloc(sizeof (char *) * (argc + 2));
diff --git a/submodule.c b/submodule.c
index 613857e..fe5748d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1135,16 +1135,11 @@ void connect_work_tree_and_git_dir(const char 
*work_tree, const char *git_dir)
struct strbuf file_name = STRBUF_INIT;
struct strbuf rel_path = STRBUF_INIT;
const char *real_work_tree = xstrdup(real_path(work_tree));
-   FILE *fp;
 
/* Update gitfile */
strbuf_addf(file_name, %s/.git, work_tree);
-   fp = fopen(file_name.buf, w);
-   if (!fp)
-   die(_(Could not create git link %s), file_name.buf);
-   fprintf(fp, gitdir: %s\n, relative_path(git_dir, real_work_tree,
- rel_path));
-   fclose(fp);
+   write_file(file_name.buf, 1, gitdir: %s\n,
+  relative_path(git_dir, real_work_tree, rel_path));
 
/* Update core.worktree setting */
strbuf_reset(file_name);
diff --git a/transport.c b/transport.c
index ca7bb44..2df8a15 100644
--- a/transport.c
+++ b/transport.c
@@ -294,7 +294,6 @@ static int write_one_ref(const char *name, const unsigned 
char *sha1,
 {
struct strbuf *buf = data;
int len = buf-len;
-   FILE *f;
 
/* when called via for_each_ref(), flags is non-zero */
if (flags  !starts_with(name, refs/heads/) 
@@ -303,10 +302,9 @@ static int write_one_ref(const char *name, const unsigned 
char *sha1,
 
strbuf_addstr(buf, name);
if (safe_create_leading_directories(buf-buf) ||
-   !(f = fopen(buf-buf, w)) ||
-   fprintf(f, %s\n, sha1_to_hex(sha1))  0 ||
-   fclose(f))
-   return error(problems writing temporary file %s, buf-buf);
+   write_file(buf-buf, 0, %s\n, sha1_to_hex(sha1)))
+   return 

[PATCH v5 26/28] gc: style change -- no SP before closing bracket

2014-03-07 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/gc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index c19545d..39d9b27 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -260,7 +260,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
OPT__QUIET(quiet, N_(suppress progress reporting)),
{ OPTION_STRING, 0, prune, prune_expire, N_(date),
N_(prune unreferenced objects),
-   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire },
+   PARSE_OPT_OPTARG, NULL, (intptr_t)prune_expire},
OPT_BOOL(0, aggressive, aggressive, N_(be more thorough 
(increased runtime))),
OPT_BOOL(0, auto, auto_gc, N_(enable auto-gc mode)),
OPT_BOOL(0, force, force, N_(force running gc even if there 
may be another gc running)),
@@ -273,7 +273,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
argv_array_pushl(pack_refs_cmd, pack-refs, --all, --prune, NULL);
argv_array_pushl(reflog, reflog, expire, --all, NULL);
argv_array_pushl(repack, repack, -d, -l, NULL);
-   argv_array_pushl(prune, prune, --expire, NULL );
+   argv_array_pushl(prune, prune, --expire, NULL);
argv_array_pushl(rerere, rerere, gc, NULL);
 
git_config(gc_config, NULL);
-- 
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


[PATCH v5 25/28] prune: strategies for linked checkouts

2014-03-07 Thread Nguyễn Thái Ngọc Duy
(alias R=$GIT_COMMON_DIR/repos/id)

 - 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/id is to be pruned.

 - If $R/locked exists, repos/id 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 pclo...@gmail.com
---
 Documentation/git-prune.txt|  3 ++
 Documentation/gitrepository-layout.txt | 19 +
 builtin/checkout.c | 14 +++
 builtin/prune.c| 74 ++
 setup.c| 13 ++
 5 files changed, 123 insertions(+)

diff --git a/Documentation/git-prune.txt b/Documentation/git-prune.txt
index 058ac0d..7babf11 100644
--- a/Documentation/git-prune.txt
+++ b/Documentation/git-prune.txt
@@ -48,6 +48,9 @@ OPTIONS
 --expire time::
Only expire loose objects older than time.
 
+--repos::
+   Prune directories in $GIT_DIR/repos.
+
 head...::
In addition to objects
reachable from any of our references, keep objects
diff --git a/Documentation/gitrepository-layout.txt 
b/Documentation/gitrepository-layout.txt
index 2206f35..dff6c47 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -252,6 +252,25 @@ repos::
$GIT_COMMON_DIR is set and $GIT_COMMON_DIR/repos will be
used instead.
 
+repos/id/gitdir::
+   A text file containing the absolute path back to the .git file
+   that points to here. This is used to check if the linked
+   repository has been manually removed and there is no need to
+   keep this directory any more. mtime of this file should be
+   updated every time the linked repository is accessed.
+
+repos/id/locked::
+   If this file exists, the linked repository may be on a
+   portable device and not available. It does not mean that the
+   linked repository is gone and `repos/id` could be
+   removed. The file's content contains a reason string on why
+   the repository is locked.
+
+repos/id/link::
+   If this file exists, it is a hard link to the linked .git
+   file. It is used to detect if the linked repository is
+   manually removed.
+
 SEE ALSO
 
 linkgit:git-init[1],
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
+* 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);
/*
@@ -913,6 +923,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
 * value would do because this value will be ignored and
 * replaced at the next (real) checkout.
 */
+   strbuf_reset(sb);
strbuf_addf(sb, %s/HEAD, sb_repo.buf);
write_file(sb.buf, 1, %s\n, sha1_to_hex(new-commit-object.sha1));
strbuf_reset(sb);
@@ -931,6 +942,9 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
ret = run_command(cp);
if (!ret)
is_junk = 0;
+   strbuf_reset(sb);
+   strbuf_addf(sb, %s/locked, sb_repo.buf);
+   unlink_or_warn(sb.buf);
strbuf_release(sb);
strbuf_release(sb_repo);
strbuf_release(sb_git);
diff --git a/builtin/prune.c b/builtin/prune.c
index de43b26..733cb3b 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -112,6 +112,70 @@ static void prune_object_dir(const char *path)
}
 }
 
+static const char *prune_repo_dir(const char *id, struct stat *st)
+{
+   char *path;
+   int fd, len;
+   if (file_exists(git_path(repos/%s/locked, id)))
+   return NULL;
+   

[PATCH v5 24/28] checkout: detach if the branch is already checked out elsewhere

2014-03-07 Thread Nguyễn Thái Ngọc Duy
The normal rule is anything outside refs/heads/ is detached. This
increases strictness of the rule a bit more: if the branch is checked
out (either in $GIT_COMMON_DIR/HEAD or any $GIT_DIR/repos/.../HEAD)
then it's detached as well.

A hint is given so the user knows where to go and do something there
if they still want to checkout undetached here.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/checkout.c | 80 ++
 t/t2025-checkout-to.sh | 15 --
 2 files changed, 92 insertions(+), 3 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 97fdfcc..598b43d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -433,6 +433,11 @@ struct branch_info {
const char *name; /* The short name used */
const char *path; /* The full name of a real branch */
struct commit *commit; /* The named commit */
+   /*
+* if not null the branch is detached because it's already
+* checked out in this checkout
+*/
+   char *checkout;
 };
 
 static void setup_branch_path(struct branch_info *branch)
@@ -641,6 +646,11 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (old-path  advice_detached_head)
detach_advice(new-name);
describe_detached_head(_(HEAD is now at), 
new-commit);
+   if (new-checkout  !*new-checkout)
+   fprintf(stderr, _(hint: the main checkout is 
holding this branch\n));
+   else if (new-checkout)
+   fprintf(stderr, _(hint: the linked checkout %s 
is holding this branch\n),
+   new-checkout);
}
} else if (new-path) { /* Switch branches. */
create_symref(HEAD, new-path, msg.buf);
@@ -983,6 +993,73 @@ static const char *unique_tracking_name(const char *name, 
unsigned char *sha1)
return NULL;
 }
 
+static int check_linked_checkout(struct branch_info *new,
+ const char *name, const char *path)
+{
+   struct strbuf sb = STRBUF_INIT;
+   char *start, *end;
+   if (strbuf_read_file(sb, path, 0)  0)
+   return 0;
+   if (!starts_with(sb.buf, ref:)) {
+   strbuf_release(sb);
+   return 0;
+   }
+
+   start = sb.buf + 4;
+   while (isspace(*start))
+   start++;
+   end = start;
+   while (*end  !isspace(*end))
+   end++;
+   if (!strncmp(start, new-path, end - start) 
+   new-path[end - start] == '\0') {
+   strbuf_release(sb);
+   new-path = NULL; /* detach */
+   new-checkout = xstrdup(name); /* reason */
+   return 1;
+   }
+   strbuf_release(sb);
+   return 0;
+}
+
+static void check_linked_checkouts(struct branch_info *new)
+{
+   struct strbuf path = STRBUF_INIT;
+   DIR *dir;
+   struct dirent *d;
+
+   strbuf_addf(path, %s/repos, get_git_common_dir());
+   if ((dir = opendir(path.buf)) == NULL) {
+   strbuf_release(path);
+   return;
+   }
+
+   strbuf_reset(path);
+   strbuf_addf(path, %s/HEAD, get_git_common_dir());
+   /*
+* $GIT_COMMON_DIR/HEAD is practically outside
+* $GIT_DIR so resolve_ref_unsafe() won't work (it
+* uses git_path). Parse the ref ourselves.
+*/
+   if (check_linked_checkout(new, , path.buf)) {
+   strbuf_release(path);
+   closedir(dir);
+   return;
+   }
+
+   while ((d = readdir(dir)) != NULL) {
+   if (!strcmp(d-d_name, .) || !strcmp(d-d_name, ..))
+   continue;
+   strbuf_reset(path);
+   strbuf_addf(path, %s/repos/%s/HEAD,
+   get_git_common_dir(), d-d_name);
+   if (check_linked_checkout(new, d-d_name, path.buf))
+   break;
+   }
+   strbuf_release(path);
+   closedir(dir);
+}
+
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
@@ -1110,6 +1187,9 @@ static int parse_branchname_arg(int argc, const char 
**argv,
else
new-path = NULL; /* not an existing branch */
 
+   if (new-path)
+   check_linked_checkouts(new);
+
new-commit = lookup_commit_reference_gently(rev, 1);
if (!new-commit) {
/* not a commit */
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index 5ec49e2..2d35a9b 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -13,13 +13,14 @@ test_expect_success 'checkout --to not updating paths' '
 '
 
 test_expect_success 'checkout --to a new worktree' '
+   git 

Confirm Receipt

2014-03-07 Thread Mrs.Supini Thrunkul

Hello,

I am Mrs.Supini Thrunkul from Tai Yau Bank Hong Kong,I need your cooperation  
to transfer $47.3 million US Dollars to any trusted account within your  
control.

Contact me for more details.  

Mrs.Supini Thrunkul
Tel: +85 2580 848 65
--
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 01/28] path.c: make get_pathname() return strbuf instead of static buffer

2014-03-07 Thread Torsten Bögershausen
On 2014-03-08 03.47, Nguyễn Thái Ngọc Duy wrote:
 We've been avoiding PATH_MAX whenever possible. This patch makes
 get_pathname() return a strbuf and updates the callers to take
 advantage of this. The code is simplified as we no longer need to
 worry about buffer overflow.
 
 vsnpath() behavior is changed slightly: previously it always clears
Minor question:
Is the function name vsnpath() appropriate any more ?
How about renaming the function into strbuf_vaddpath() ?

 the buffer before writing, now it just appends. Fortunately this is a
 static function and all of its callers prepare the buffer properly:
 git_path() gets the buffer from get_pathname() which resets the
 buffer, the remaining call sites start with STRBUF_INIT'd buffer.
 
 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.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


[GSoC][PATCH v2] use strchrnul() in place of strchr() and strlen()

2014-03-07 Thread Rohit Mani
Avoid scanning strings twice, once with strchr() and then with
strlen(), by using strchrnul().

Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Rohit Mani rohit.m...@outlook.com
---
PATCH v2 Updated commit description
Updated code according to suggestions [1]
[1] http://article.gmane.org/gmane.comp.version-control.git/243550

 archive.c|4 ++--
 cache-tree.c |   15 ++-
 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, 35 insertions(+), 60 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..2130f32 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, '/');
+   namelen = slash - path;
it-entry_count = -1;
-   if (!slash) {
+   if (!*slash) {
int pos;
-   namelen = strlen(path);
pos = subtree_pos(it, path, namelen);
if (0 = pos) {
cache_tree_free(it-down[pos]-cache_tree);
@@ -143,7 +143,6 @@ void cache_tree_invalidate_path(struct cache_tree *it, 
const char *path)
}
return;
}
-   namelen = slash - path;
down = find_subtree(it, path, namelen, 0);
if (down)
cache_tree_invalidate_path(down-cache_tree, slash + 1);
@@ -554,9 +553,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 +561,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)
+   if (*slash)
while (*slash  *slash == '/')
slash++;
-   if (!slash || !*slash)
+   if (!*slash)
return it; /* prefix ended with slashes */
path = slash;
}
diff --git a/diff.c b/diff.c
index e800666..cb6b98b 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) {
int n;
char *end;
if (!isdigit(*++eq))
diff --git a/fast-import.c b/fast-import.c
index 4fd18a3..dbbf6b0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1485,14 +1485,11 @@ 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  !S_ISDIR(mode)  subtree)
die(Non-directories cannot have subtrees);
 
if (!root-tree)
@@ -1501,7 +1498,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) {
if (!S_ISDIR(mode)