[PATCH v5] lockfile.c: store absolute path

2014-11-02 Thread Michael Haggerty
From: Nguyễn Thái Ngọc Duy pclo...@gmail.com

Locked paths can be saved in a linked list so that if something wrong
happens, *.lock are removed. For relative paths, this works fine if we
keep cwd the same, which is true 99% of time except:

- update-index and read-tree hold the lock on $GIT_DIR/index really
  early, then later on may call setup_work_tree() to move cwd.

- Suppose a lock is being held (e.g. by git add) then somewhere
  down the line, somebody calls real_path (e.g. link_alt_odb_entry),
  which temporarily moves cwd away and back.

During that time when cwd is moved (either permanently or temporarily)
and we decide to die(), attempts to remove relative *.lock will fail,
and the next operation will complain that some files are still locked.

Avoid this case by turning relative paths to absolute before storing
the path in filename field.

Reported-by: Yue Lin Ho yuelinho...@gmail.com
Helped-by: Ramsay Jones ram...@ramsay1.demon.co.uk
Helped-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
Adapted-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
This is a re-roll onto master to resolve conflicts with other changes
that have been merged since v4 [1]:

* The length of path is now available, so use strbuf_add() instead of
  strbuf_addstr().

* LOCK_NODEREF has been renamed to LOCK_NO_DEREF.

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

 lockfile.c| 14 +++---
 t/t2107-update-index-basic.sh | 15 +++
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/lockfile.c b/lockfile.c
index 4f16ee7..9889277 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -128,9 +128,17 @@ static int lock_file(struct lock_file *lk, const char 
*path, int flags)
path);
}
 
-   strbuf_add(lk-filename, path, pathlen);
-   if (!(flags  LOCK_NO_DEREF))
-   resolve_symlink(lk-filename);
+   if (flags  LOCK_NO_DEREF) {
+   strbuf_add_absolute_path(lk-filename, path);
+   } else {
+   struct strbuf resolved_path = STRBUF_INIT;
+
+   strbuf_add(resolved_path, path, pathlen);
+   resolve_symlink(resolved_path);
+   strbuf_add_absolute_path(lk-filename, resolved_path.buf);
+   strbuf_release(resolved_path);
+   }
+
strbuf_addstr(lk-filename, LOCK_SUFFIX);
lk-fd = open(lk-filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
if (lk-fd  0) {
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 1bafb90..dfe02f4 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -65,4 +65,19 @@ test_expect_success '--cacheinfo mode,sha1,path (new 
syntax)' '
test_cmp expect actual
 '
 
+test_expect_success '.lock files cleaned up' '
+   mkdir cleanup 
+   (
+   cd cleanup 
+   mkdir worktree 
+   git init repo 
+   cd repo 
+   git config core.worktree ../../worktree 
+   # --refresh triggers late setup_work_tree,
+   # active_cache_changed is zero, rollback_lock_file fails
+   git update-index --refresh 
+   ! test -f .git/index.lock
+   )
+'
+
 test_done
-- 
2.1.1

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


[gitk PATCH] gitk: Default wrcomcmd to use --pretty=email

2014-11-02 Thread Chris Packham
This makes the Write commit to file context menu option generate a file that
is consumable by 'git am'.

Signed-off-by: Chris Packham judge.pack...@gmail.com
---
Hi Paul,

This is the other side of a git patch I was looking at a while ago[1]. The basic
problem was people were using gitk's Write commit to file functionality to
generate patches for me to apply. It was hard to convince git am to consume 
these.

Eventually after some pointers from Junio I realised that if gitk just used
 --pretty=email then it would generate files that were consumable by git am.
Just adding =email is the minimal change to make things work but it might be a
good idea to add --stat to make it more patch-like (or just switch to using
format-patch).
--
[1] - http://article.gmane.org/gmane.comp.version-control.git/256424
[2] - http://article.gmane.org/gmane.comp.version-control.git/256543

 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 78358a7..462f966 100755
--- a/gitk
+++ b/gitk
@@ -11936,7 +11936,7 @@ if { [info exists ::env(GIT_TRACE)] } {
 }
 
 # defaults...
-set wrcomcmd git diff-tree --stdin -p --pretty
+set wrcomcmd git diff-tree --stdin -p --pretty=email
 
 set gitencoding {}
 catch {
-- 
2.0.4

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


Re: [PATCH] git-svn: use SVN::Ra::get_dir2 when possible

2014-11-02 Thread Hin-Tak Leung
Hmm, I see you are filing the problem against subversion. FWIW,
I am currently using subversion-perl-1.8.10-1.fc20.x86_64 package on fedora 20.
I'll possibly think about filing one under redhat's bugzilla and
let them take it upward too.


On Fri, 31/10/14, Eric Wong normalper...@yhbt.net wrote:

 This avoids the following failure
 with normal get_dir on newer
 versions of SVN (tested with SVN 1.8.8-1ubuntu3.1):
 
   Incorrect parameters given: Could not convert '%ld'
 into a number
 
 get_dir2 also has the potential to be more efficient by
 requesting
 less data.
 
 ref: 1414636504.45506.yahoomailba...@web172304.mail.ir2.yahoo.com
 ref: 1414722617.89476.yahoomailba...@web172305.mail.ir2.yahoo.com
 
 Signed-off-by: Eric Wong normalper...@yhbt.net
 Cc: Hin-Tak Leung ht...@users.sourceforge.net
 ---
   This should fix the vbox clone problem.  SVN
 Perl binding
   breakage (again :).  I shall revert the
 int() changes.
 
    I added those two lines to my git and there is
 no improvement. It
    still won't svn fetch the next revision. I think
 it may be
    important/interesting to find out when or how it
 becomes non-int, so
    I have tar'gz'ed my wont-fetch virtual box .git
 and in the middle of
    uploading here: 
   
    http://sourceforge.net/projects/outmodedbonsai/files/R/
 
    I am also uploading my old R clone also - maybe
 you'd like to see
    why its .git/svn/.caches is so big compared to a
 recent one,
 
   Jakob's changes causes different access patterns, so
 it's expected the
   sizes vary.  I also changed the cherry pick
 cache and removed the
   _rev_list caching entirely, so it should be much
 smaller now.
 
    as well as how and why there were an extra merge
 and two missing
    merges compared to a recent clone?
 
   The different merges are fine, I think, as stated in
     http://mid.gmane.org/20141030230831.ga14...@dcvr.yhbt.net
 
  perl/Git/SVN/Ra.pm | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)
 
 diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
 index 82d6108..1e52709 100644
 --- a/perl/Git/SVN/Ra.pm
 +++ b/perl/Git/SVN/Ra.pm
 @@ -177,7 +177,17 @@ sub get_dir {
          }
      }
      my $pool = SVN::Pool-new;
 -    my ($d, undef, $props) =
 $self-SUPER::get_dir($dir, $r, $pool);
 +    my ($d, undef, $props);
 +
 +    if (defined SVN::Ra::get_dir2) { #
 appeared in SVN 1.4
 +        # n.b. in addition to
 being potentially more efficient,
 +        # this works around
 what appears to be a bug in some
 +        # SVN 1.8 versions
 +        my $kind = 1; #
 SVN_DIRENT_KIND
 +        ($d, undef, $props) =
 $self-get_dir2($dir, $r, $kind, $pool);
 +    } else {
 +        ($d, undef, $props) =
 $self-get_dir($dir, $r, $pool);
 +    }
      my %dirents = map { $_ = { kind
 = $d-{$_}-kind } } keys %$d;
      $pool-clear;
      if ($r != $cache-{r}) {
 -- 
 EW
 
--
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: [L10N] Startup of Git 2.2.0 l10n round 1

2014-11-02 Thread Alexander Shopov
Can you please disambiguate message:
msgid more than one %s

It means that something somewhere was repeated but does not point what
and where. Perhaps users care about that.

It is now used 3 times (trailer.c:552 trailer.c:557
builtin/remote.c:288) but points to different things that were
repeated. It used to mean only that there is a remote' section
repeated.

Kind regards:
al_shopov
--
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: [L10N] Startup of Git 2.2.0 l10n round 1

2014-11-02 Thread Christian Couder
From: Alexander Shopov a...@kambanaria.org

 Can you please disambiguate message:
 msgid more than one %s
 
 It means that something somewhere was repeated but does not point what
 and where. Perhaps users care about that.

If you configure something like:

[trailer stuff]
key = Stuff
key = Other

You will get:

$ echo | LANG=C git interpret-trailers
warning: more than one trailer.stuff.key

Which means that more than value was configured for the
trailer.stuff.key configuration option. And it makes no sense
because only one should be the canonical one.

 It is now used 3 times (trailer.c:552 trailer.c:557
 builtin/remote.c:288) but points to different things that were
 repeated. It used to mean only that there is a remote' section
 repeated.

In builtin/remote.c the warning is also when more than one value is
given for a configuration option.

Feel free to suggest some more explicit warning in both cases if you
want. (Maybe adding  is configured at the end would be enough.) A
patch would be even better.

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: [PATCH] use child_process_init() to initialize struct child_process variables

2014-11-02 Thread Philip Oakley

From: Jeff King p...@peff.net

On Fri, Oct 31, 2014 at 02:48:17PM -0700, Junio C Hamano wrote:


Programs that read a pack data stream unpack-objects were originally
designed to ignore cruft after the pack data stream ends, and
because the bundle file format ends with pack data stream, you
should have been able to append extra information at the end without
breaking older clients.


It's an option, I'd been looking at sneaking the information into the 
refs header section.



Alas, this principle is still true for
unpack-objects, but index-pack broke it fairly early on, and we use
the latter to deal with bundles, so we cannot just tuck extra info
at the end of an existing bundle.  You'd instead need a new option
to create a bundle that cannot be read by existing clients X-.


I think you could use a similar NUL-trick to what we do in the online


I like this 'trick'. I'd not appreciated the use of the null separator
for breaking a line into separate strings that way before (I'd 
understood it, just never appreciated it!).



protocol, and have a ref section like:

 ...sha1... refs/heads/master
 ...sha1... refs/heads/confused-with-master
 ...sha1... HEAD\0symref=refs/heads/master

The current parser reads into a strbuf up to the newline, but we
ignore
everything after the NUL, treating it like a C string. Prior to using
strbufs, we used fgets, which behaves similarly (you could not know
from
fgets that there is extra data after the NUL, but that is OK; we only
want older versions to ignore the data, not do anything useful with
it).



This certainly looks the way to go. The one extra question would be
whether the symref should be included by default when HEAD is present, 
or only if there was possible ambiguity between the other listed refs. 
Previously I'd assumed the latter. The former would appear stronger, as 
long as the symref was within the listed refs, and excluded otherwise.


Philip 


--
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] git-svn: use SVN::Ra::get_dir2 when possible

2014-11-02 Thread Eric Wong
Hin-Tak Leung ht...@users.sourceforge.net wrote:
 Hmm, I see you are filing the problem against subversion. FWIW,
 I am currently using subversion-perl-1.8.10-1.fc20.x86_64 package on fedora 
 20.
 I'll possibly think about filing one under redhat's bugzilla and
 let them take it upward too.

This is another problem with the vbox repository:
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=767530#10

And forwarded upstream:
  http://mid.gmane.org/20141101182722.gb20...@freya.jamessan.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: [L10N] Startup of Git 2.2.0 l10n round 1

2014-11-02 Thread Jiang Xin
2014-11-03 3:06 GMT+08:00 Christian Couder chrisc...@tuxfamily.org:
 From: Alexander Shopov a...@kambanaria.org

 Can you please disambiguate message:
 msgid more than one %s

 It means that something somewhere was repeated but does not point what
 and where. Perhaps users care about that.

 If you configure something like:

 [trailer stuff]
 key = Stuff
 key = Other

 You will get:

 $ echo | LANG=C git interpret-trailers
 warning: more than one trailer.stuff.key

 Which means that more than value was configured for the
 trailer.stuff.key configuration option. And it makes no sense
 because only one should be the canonical one.

 It is now used 3 times (trailer.c:552 trailer.c:557
 builtin/remote.c:288) but points to different things that were
 repeated. It used to mean only that there is a remote' section
 repeated.

 In builtin/remote.c the warning is also when more than one value is
 given for a configuration option.

 Feel free to suggest some more explicit warning in both cases if you
 want. (Maybe adding  is configured at the end would be enough.) A
 patch would be even better.


Yes, Alexander could send a patch for this. It's an I18N thing, not L10n,
so send a patch to the list if you like. @alshopov

 Thanks,
 Christian.

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