[PATCH] Update gitweb.perl to current CGI.pm API

[Roland Mas]

Hi all,

  This simple two-line patch fixes a bug that makes gitweb unusable on
systems where the installed CGI.pm is version 4.04 or later (such as on
Debian unstable).  That version removed the startform method, which had
previously been deprecated in favour of start_form since 2009.

  I don't have any specific tests for that change, but it does help
fixing the testsuite of FusionForge (which includes a test of its Git
plugin, involving gitweb).

  For reference, this is Debian bug #765525 (http://bugs.debian.org/765525).

  (I'm not subscribed to the git@vger mailing-list; please Cc me on
replies.)

  Thanks,

Roland.

From 1b74cfb8568927a307f165e428455789398f6d61 Mon Sep 17 00:00:00 2001
From: Roland Mas lola...@debian.org
Date: Thu, 16 Oct 2014 00:05:25 +0200
Subject: [PATCH] Update gitweb.perl to current CGI.pm API

CGI.pm 4.04 removed the startform method, which had previously been
deprecated in favour of start_form.  Updated gitweb.perl accordingly.

Signed-off-by: Roland Mas lola...@debian.org
---
 gitweb/gitweb.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..ccf7516 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4100,7 +4100,7 @@ sub print_search_form {
if ($use_pathinfo) {
$action .= /.esc_url($project);
}
-   print $cgi-startform(-method = get, -action = $action) .
+   print $cgi-start_form(-method = get, -action = $action) .
  div class=\search\\n .
  (!$use_pathinfo 
  $cgi-input({-name=p, -value=$project, -type=hidden}) . 
\n) .
@@ -5510,7 +5510,7 @@ sub git_project_search_form {
}
 
print div class=\projsearch\\n;
-   print $cgi-startform(-method = 'get', -action = $my_uri) .
+   print $cgi-start_form(-method = 'get', -action = $my_uri) .
  $cgi-hidden(-name = 'a', -value = 'project_list')  . \n;
print $cgi-hidden(-name = 'pf', -value = $project_filter). \n
if (defined $project_filter);
-- 
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

Re: fsck option to remove corrupt objects - why/why not?

[Johan Herland]

On Thu, Oct 16, 2014 at 2:13 AM, Ben Aveling bena@optusnet.com.au wrote:
 On 14/10/2014 19:21, Jeff King wrote:
 On Mon, Oct 13, 2014 at 09:37:27AM +1100, Ben Aveling wrote:
 A question about fsck - is there a reason it doesn't have an option to
 delete bad objects?

 If the objects are reachable, then deleting them would create other big
 problems (i.e., we would be breaking the object graph!).


 The man page for fsck advises:

Any corrupt objects you will have to find in backups or other
archives (i.e., you can just remove them and do an /rsync/ with some
other site in the hopes that somebody else has the object you have
corrupted).


 And that seems sensible to me - the object is corrupt, it is unusable, the
 object graph is already broken, we already have big problems, removing the
 corrupt object(s) doesn't create any new problems, and it allows the
 possibility that the damaged objects can be restored.

 I ask because I have a corrupt repository, and every time I run fsck, it
 reports one corrupt object, then stops. I could write a script to repeatedly
 call fsck and then remove the next corrupt object, but it raises the
 question for me; could it make sense to extend fsck with the option to do to
 the removes?

I am positive to this idea. Yesterday a colleague of mine came to me
with a repo containing a single corrupt object (in a 1.2GB packfile).
We were lucky, since we had a copy of the repo with a good copy of the
same object. However, we were lucky in a couple of other respects, as
well:

I simply copied the packfile containing the good copy into the
corrupted repo, and then ran a git gc, which happened to use the
good copy of the corrupted object and complete successfully (instead
of barfing on the bad copy). The GC then removed the old
(now-obsolete) packfiles, and thus the corruption was gone.

However, exactly _why_ git happened to prefer the good copy in my
copied packfile instead of the bad copy in the existing packfile, I do
not know. I suspect some amount of pure luck was involved. Indeed, I
feared I would have to explode the corrupt pack, then manually replace
the )(now-loose) bad copy with a good copy (from a similarly exploded
pristine pack), and then finally repack everything again. That said,
I'm not at all sure that Git would be able to successfully explode a
pack containing corrupt objects...

I think a better solution would be to tell fsck to remove the corrupt
object(s), as you suggest above, and then copy in the good pack. In
that case, there would be no question that the good copy would be used
in the subsequent GC.

 Or even better, do the removes and then do the necessary
 [r]sync, assuming the user has another repository that has a good copy of
 the bad objects, which in this case I do.

Hmm. I am not sure we want to automate the syncing step. First, git
cannot know _which_ remote is likely to have a good copy of the bad
object. Second, we do not necessarily know what caused the corruption
in the first place, and whether syncing with a remote (which will
create certain amount of write activity on a possibly dying disk
drive) is a good idea at all. Finally, this syncing step will have to
bypass Git's usual reachability analysis (which easily skips fetching
a corrupt blob from otherwise-reachable history), is more involved
than simply calling out to git fetch...


...Johan

-- 
Johan Herland, jo...@herland.net
www.herland.net
--
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 download --- Virus

[risto.makini...@pp.inet.fi]

Hi

I downloaded and started to Install Git.

There is a Virus on you setup.
Program that appears to have trojan-like features or behavior.

Git/bin/pdfinfo.exe

trojan.generic.[variant], gen:trojan.[variant]

Why???

Br.

Risto

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

[Konstantin Khomoutov]

On Thu, 16 Oct 2014 12:35:33 +0300 (EEST)
risto.makini...@pp.inet.fi risto.makini...@pp.inet.fi wrote:

 I downloaded and started to Install Git.
 
 There is a Virus on you setup.
 Program that appears to have trojan-like features or behavior.
 
 Git/bin/pdfinfo.exe
 
 trojan.generic.[variant], gen:trojan.[variant]
 
 Why???

Because your antivirus software applies its (seemingly imperfect)
heuristics and thinks there's a virus while there's none.

To state this in a more blunt way: no there's no any virus in the Git
for Windows installation package.

The other possibility is you obtaining the installation package from
a place other than http://git-scm.com or some malware active on your
computer is changing the packages you're downloading on the fly.  The
latter is highly unlikely though.
--
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: fsck option to remove corrupt objects - why/why not?

[Matthieu Moy]

Ben Aveling bena@optusnet.com.au writes:

 And that seems sensible to me - the object is corrupt, it is unusable,
 the object graph is already broken, we already have big problems,
 removing the corrupt object(s) doesn't create any new problems, and it
 allows the possibility that the damaged objects can be restored.

Removing completely may remove a chance to restore the corrupt object
(rather unlikely, but I can imagine fine binary file surgery to un-break
a broken object file).

But we could move them out of Git's object directory (a bit like
.git/lost-found, we could have .git/corrupt). For unpacked objects, it's
trivial (just mv them in the directory). For packed objects, I don't
know what happens in case they are corrupt. That would solve essentially
any problem that you can solve by removing the file, but it makes the
operation reversible.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: fsck option to remove corrupt objects - why/why not?

[Jeff King]

On Thu, Oct 16, 2014 at 11:04:04AM +0200, Johan Herland wrote:

 I simply copied the packfile containing the good copy into the
 corrupted repo, and then ran a git gc, which happened to use the
 good copy of the corrupted object and complete successfully (instead
 of barfing on the bad copy). The GC then removed the old
 (now-obsolete) packfiles, and thus the corruption was gone.
 
 However, exactly _why_ git happened to prefer the good copy in my
 copied packfile instead of the bad copy in the existing packfile, I do
 not know. I suspect some amount of pure luck was involved.

I'm not sure that it is luck, but more like 8eca0b4 (implement some
resilience against pack corruptions, 2008-06-23) working as intended[1].
Generally, git should be able to warn about corrupted objects and look
in other packs for them (both for regular operations, and for
repacking).

-Peff

[1] That's just one of the many commits dealing with this. Try running
git log --author=Nicolas.Pitre --grep=corrupt for more. :)
--
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: fsck option to remove corrupt objects - why/why not?

[Johan Herland]

On Thu, Oct 16, 2014 at 2:25 PM, Jeff King p...@peff.net wrote:
 On Thu, Oct 16, 2014 at 11:04:04AM +0200, Johan Herland wrote:
 I simply copied the packfile containing the good copy into the
 corrupted repo, and then ran a git gc, which happened to use the
 good copy of the corrupted object and complete successfully (instead
 of barfing on the bad copy). The GC then removed the old
 (now-obsolete) packfiles, and thus the corruption was gone.

 However, exactly _why_ git happened to prefer the good copy in my
 copied packfile instead of the bad copy in the existing packfile, I do
 not know. I suspect some amount of pure luck was involved.

 I'm not sure that it is luck, but more like 8eca0b4 (implement some
 resilience against pack corruptions, 2008-06-23) working as intended[1].
 Generally, git should be able to warn about corrupted objects and look
 in other packs for them (both for regular operations, and for
 repacking).

 -Peff

 [1] That's just one of the many commits dealing with this. Try running
 git log --author=Nicolas.Pitre --grep=corrupt for more. :)

Indeed, from reading the logs, it seems what I assumed was a lucky
strike, was actually carefully designed behavior. With that in mind,
I'm no longer so sure that fsck actually needs an option to remove
corrupt objects. Instead, it's probably better to leave the corrupt
object in place until a good copy can be located and copied into the
repo, at which point Nicolas' brilliant work will make sure a simple
repack takes care of fixing the corruption.

That said, we should consider documenting this strategy for fixing corruptions:
 - Locate the a good copy of the affected objects in another repo
 - Copy relevant pack file or loose object into this repo
 - Run git gc
 - Profit!

...Johan

-- 
Johan Herland, jo...@herland.net
www.herland.net
--
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 describe oddity with GIT_DIR

[Thomas Braun]

Hi,

I've encountered an oddity with git describe.
Consider the following snippet:
-
mkdir test
cd test
git init
echo 1  file
git add file
git commit -m changes
$ git describe --always --dirty
8ad486e
$ cd ..
$ git --git-dir=test/.git describe --always --dirty
8ad486e-dirty
$ GIT_DIR=test/.git git describe --always --dirty
8ad486e-dirty
-

The -dirty suffix appears if invoking git not from the worktree
itself, but should actually never appear.

According to my research this behaviour is there since 9f67d2e8 (Teach
git describe --dirty option, 2009-10-27).

Is that to expected?

Thanks
Thomas
--
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] diff: Handle process substitution

[Gabriel de Perthuis]

For example:

git diff --color-words (echo a b c) (echo a d c)

Changes to struct diff_filespec:
- is_stdin renamed to skip_hashing, which is what it did
- follow_symlinks added, causing diff_populate_filespec to look at
  file contents instead of the readlink value

Paths that are handled specially (using
skip_hashing and follow_symlinks):

/dev/stdin added as an alternative to -
/dev/fd/*
/proc/self/fd/*

The first two are standard ways to refer to file descriptors,
and there is precedence for handling them specially (bash
redirections for example).  The last one is there to support
zsh process substitution, which on Linux uses an OS-specific path.
---
 Documentation/git-diff.txt |  7 +++
 diff-no-index.c|  8 +++-
 diff.c | 26 +-
 diffcore.h |  8 +---
 4 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index bbab35f..f4ca476 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -99,10 +99,17 @@ include::diff-options.txt[]
  path...::
The paths parameters, when given, are used to limit
the diff to the named paths (you can give directory
names and get diff for all files under them).
+   +
+   With --no-index, or with paths outside the worktree,
+   some paths are handled specially: - and /dev/stdin
+   refer to standard input, and /dev/fd/* can be used
+   to refer to existing file descriptors, as in:
+
+   $ git diff --color-words (echo a b c) (echo a d c)
   include::diff-format.txt[]
  EXAMPLES
diff --git a/diff-no-index.c b/diff-no-index.c
index 265709b..586036b 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -70,11 +70,11 @@ static int populate_from_stdin(struct diff_filespec *s)
s-should_munmap = 0;
s-data = strbuf_detach(buf, size);
s-size = size;
s-should_free = 1;
-   s-is_stdin = 1;
+   s-skip_hashing = 1;
return 0;
 }
  static struct diff_filespec *noindex_filespec(const char *name, int mode)
 {
@@ -84,10 +84,16 @@ static struct diff_filespec *noindex_filespec(const
char *name, int mode)
name = /dev/null;
s = alloc_filespec(name);
fill_filespec(s, null_sha1, 0, mode);
if (name == file_from_standard_input)
populate_from_stdin(s);
+   else if (!strcmp(name, /dev/stdin) ||
+!strncmp(name, /dev/fd/, 8) ||
+!strncmp(name, /proc/self/fd/, 14)) {
+   s-skip_hashing = 1;
+   s-follow_symlinks = 1;
+   }
return s;
 }
  static int queue_diff(struct diff_options *o,
  const char *name1, const char *name2)
diff --git a/diff.c b/diff.c
index d7a5c81..c1150d7 100644
--- a/diff.c
+++ b/diff.c
@@ -2711,22 +2711,26 @@ int diff_populate_filespec(struct diff_filespec
*s, unsigned int flags)
reuse_worktree_file(s-path, s-sha1, 0)) {
struct strbuf buf = STRBUF_INIT;
struct stat st;
int fd;
 -  if (lstat(s-path, st)  0) {
+   if (s-follow_symlinks)
+   err = stat(s-path, st);
+   else
+   err = lstat(s-path, st);
+   if (err  0) {
if (errno == ENOENT) {
err_empty:
err = -1;
empty:
s-data = (char *);
s-size = 0;
return err;
}
}
s-size = xsize_t(st.st_size);
-   if (!s-size)
+   if (S_ISREG(st.st_mode)  !s-size)
goto empty;
if (S_ISLNK(st.st_mode)) {
struct strbuf sb = STRBUF_INIT;
if (strbuf_readlink(sb, s-path, s-size))
@@ -2744,13 +2748,25 @@ int diff_populate_filespec(struct diff_filespec
*s, unsigned int flags)
return 0;
}
fd = open(s-path, O_RDONLY);
if (fd  0)
goto err_empty;
-   s-data = xmmap(NULL, s-size, PROT_READ, MAP_PRIVATE, fd, 0);
+   if (S_ISREG(st.st_mode)) {
+   s-data = xmmap(NULL, s-size, PROT_READ, MAP_PRIVATE, 
fd, 0);
+   s-should_munmap = 1;
+   } else {
+   struct strbuf sb = STRBUF_INIT;
+   if (strbuf_read(sb, fd, s-size)  0) {
+   err = error(error while reading from %s: %s,
+s-path, strerror(errno));
+   goto err_empty;
+   }
+   s-data = strbuf_detach(sb, s-size);
+   s-should_munmap = 0;

Re: [PATCH] clone: --dissociate option to mark that reference is only temporary

[Marc Branchaud]

On 14-10-15 05:50 PM, Junio C Hamano wrote:
 Marc Branchaud marcn...@xiplink.com writes:
 
 Yes, but we're cloning gko, not the neighbour.  Doesn't that mean that the
 clone operation won't know about any of the neighbour's refs?
 
 No.  --reference (and a natural implementation of --borrow, I would imagine)
 peeks the refs of the repository we borrow from and that is how
 clone can say I already have objects reachable from these refs, so
 please send me the remainder to the repository it is cloning from.

By know about I meant want to use.  Sorry for being a bit dense about
this; let me try again.

(BTW, it occurs to me that your patch -- if I read it right -- doesn't
fulfill your scenario since it disassociates the clone from all repos,
regardless of whether they are specified with --reference or --borrow.  In
the following I assume a --borrow that only disassociates from the specified
repo and leaves the --reference repo(s) alone.)

Since we're cloning gko's refs, all of the neighbour's unique refs and
objects can be ignored.  Even though paths to the neighbour and the local
pool will be in the clone's alternates file, any refs the clone operation
creates won't need to use any objects from the neighbour.  The clone
operation gives us no way to refer to the neighbour's unique objects.

I just don't see what difference the --borrow option makes.  Consider the two
cases:

With just --reference=/local/pool/linux.git:
1. Set up the alternates file with that path.
2. Copy gko's refs into refs/remotes/origin/.
3. Set up refs/heads/master to refer to gko's HEAD.
4. Checkout refs/heads/master (uses objects from local pool).

With both that --reference and --borrow=../my/neighbour/linux-hack.git:
1. Set up the alternates file with both paths.
2. Copy gko's refs into refs/remotes/origin/.
3. Set up refs/heads/master to refer to gko's HEAD.
4. Checkout refs/heads/master (uses objects from local pool).
5. Disassociate ourselves from the neighbour repo.

In both cases the first four actions have no need of the neighbour repo.  The
second case's fifth action surgically removes the neighbour as an alternate
object store, and we're left with the same clone we got in the first case.
What was the point?

It seems that in order to make something like --borrow useful, git clone
would somehow need to know which of the neighbour's refs you want to *also*
clone, then copy any unique objects from the neighbour before disassociating
from it.

M.

--
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 log with follow author not working

[Robert Dailey]

I have relocated a file into another directory and committed that.
Using the --follow command on the NEW path of the file, I want to find
all commits to that file by a specific author:

$ git log --follow --author david -- new/path/to/file.cpp

When I do this, I get NO results. When I use the OLD path to the file, it works:

$ git log --follow --author david -- OLD/path/to/file.cpp

Also --follow seems to work fine on the NEW path if I do not specify
--author. Is this a bug or am I using this command incorrectly?

Follow up assistance is very much appreciated. Thanks guys.
--
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: fsck option to remove corrupt objects - why/why not?

[Junio C Hamano]

Johan Herland jo...@herland.net writes:

 I simply copied the packfile containing the good copy into the
 corrupted repo, and then ran a git gc, which happened to use the
 good copy of the corrupted object and complete successfully (instead
 of barfing on the bad copy). The GC then removed the old
 (now-obsolete) packfiles, and thus the corruption was gone.

 However, exactly _why_ git happened to prefer the good copy in my
 copied packfile instead of the bad copy in the existing packfile, I do
 not know.

By design ;-)
--
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 describe oddity with GIT_DIR

[Junio C Hamano]

Thomas Braun thomas.br...@virtuell-zuhause.de writes:

 I've encountered an oddity with git describe.
 Consider the following snippet:
 -
 mkdir test
 cd test
 git init
 echo 1  file
 git add file
 git commit -m changes
 $ git describe --always --dirty
 8ad486e
 $ cd ..
 $ git --git-dir=test/.git describe --always --dirty
 8ad486e-dirty
 $ GIT_DIR=test/.git git describe --always --dirty
 8ad486e-dirty
 -

 The -dirty suffix appears if invoking git not from the worktree
 itself, but should actually never appear.

This is not oddity with describe.  You are using --git-dir incorrectly.

When you tell Git where its repository resides with the $GIT_DIR
environment variable or the --git-dir command-line option, unless
you tell it where the top-level of your working tree is, you are
telling that your current working directory is the top-level of your
working tree.

You are asking git describe to describe the state of the HEAD
including the dirtyness of the working tree in various ways.  With
the first invocation, you do not tell Git where things are and let
it correctly figure it out, i.e. you are in 'test' directory and
relative to where you are, .git is the repository and . is the
top of the working tree.  The commit recorded in the .git/HEAD,
i.e. 8ad486e, is used, and its compared with the working tree to
determine dirtiness.  Specifically, the blob object 8ad486e:file is
the same as ./file (that is test/file relative to where you
started with mkdir test above).

With the latter two, you are asking the same question but you go one
level up and then tell Git that the repository is test/.git
(correct) and the top of the working tree is . (a lie).  Again,
test/.git/HEAD records the same commit, but when trying to compare
the contents of its tree, e.g. file at the top-level in the
commit, you do not have file in the working tree.  Git is led to
believe that you removed file, hence your working tree state is
dirty.

Make it a habit to always specify GIT_WORK_TREE when you use
GIT_DIR, unless you know you will always start from the top of the
working tree.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/25] isxdigit: cast input to unsigned char

[Junio C Hamano]

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

 Otherwise, callers must do so or risk triggering warnings
 -Wchar-subscript (and rightfully so; a signed char might
 cause us to use a bogus negative index into the
 hexval_table).

 While we are dropping the now-unnecessary casts from the
 caller in urlmatch.c, we can get rid of similar casts in
 actually parsing the hex by using the hexval() helper, which
 implicitly casts to unsigned (but note that we cannot
 implement isxdigit in terms of hexval(), as it also casts
 its return value to unsigned).

 Signed-off-by: Jeff King p...@peff.net
 ---
 The patch that added more calls to isxdigit later in the series actually
 got reworked. So this is purely a cleanup and can be dropped if need be,
 though I still think it is an improvement.

Yes, thanks.


  git-compat-util.h | 2 +-
  urlmatch.c| 8 
  2 files changed, 5 insertions(+), 5 deletions(-)

 diff --git a/git-compat-util.h b/git-compat-util.h
 index fb41118..44890d5 100644
 --- a/git-compat-util.h
 +++ b/git-compat-util.h
 @@ -677,7 +677,7 @@ extern const unsigned char sane_ctype[256];
  #define iscntrl(x) (sane_istest(x,GIT_CNTRL))
  #define ispunct(x) sane_istest(x, GIT_PUNCT | GIT_REGEX_SPECIAL | \
   GIT_GLOB_SPECIAL | GIT_PATHSPEC_MAGIC)
 -#define isxdigit(x) (hexval_table[x] != -1)
 +#define isxdigit(x) (hexval_table[(unsigned char)(x)] != -1)
  #define tolower(x) sane_case((unsigned char)(x), 0x20)
  #define toupper(x) sane_case((unsigned char)(x), 0)
  #define is_pathspec_magic(x) sane_istest(x,GIT_PATHSPEC_MAGIC)
 diff --git a/urlmatch.c b/urlmatch.c
 index 3d4c54b..618d216 100644
 --- a/urlmatch.c
 +++ b/urlmatch.c
 @@ -43,11 +43,11 @@ static int append_normalized_escapes(struct strbuf *buf,
   from_len--;
   if (ch == '%') {
   if (from_len  2 ||
 - !isxdigit((unsigned char)from[0]) ||
 - !isxdigit((unsigned char)from[1]))
 + !isxdigit(from[0]) ||
 + !isxdigit(from[1]))
   return 0;
 - ch = hexval_table[(unsigned char)*from++]  4;
 - ch |= hexval_table[(unsigned char)*from++];
 + ch = hexval(*from++)  4;
 + ch |= hexval(*from++);
   from_len -= 2;
   was_esc = 1;
   }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 03/25] object_array: factor out slopbuf-freeing logic

[Junio C Hamano]

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

 This is not a lot of code, but it's a logical construct that
 should not need to be repeated (and we are about to add a
 third repetition).

Good, but I have two and a half tangential comments about the
context that appears in this patch ;-)

  void object_array_filter(struct object_array *array,
object_array_each_func_t want, void *cb_data)
  {
 @@ -367,8 +377,7 @@ void object_array_filter(struct object_array *array,
   objects[dst] = objects[src];
   dst++;
   } else {
 - if (objects[src].name != object_array_slopbuf)
 - free(objects[src].name);
 + object_array_release_entry(objects[src]);
   }
   }
   array-nr = dst;
 @@ -400,8 +409,7 @@ void object_array_remove_duplicates(struct object_array 
 *array)
   objects[array-nr] = objects[src];
   array-nr++;
   } else {
 - if (objects[src].name != object_array_slopbuf)
 - free(objects[src].name);
 + object_array_release_entry(objects[src]);
   }
   }
  }

 1.  These two functions both remove elements from a given array
 in-place, the former being in a more generic form that takes a
 caller-specified criterion while the latter uses a hardcoded
 condition to decide what to filter.  aeb4a51e (object_array:
 add function object_array_filter(), 2013-05-25) and later
 1506510c (object_array_remove_duplicates(): rewrite to reduce
 copying, 2013-05-25) should have refactored the latter further
 to implement it in terms of the former, perhaps?

 1.5 I would have expected a function to remove duplicates from an
 array to remove duplicates from the array by comparing the objects
 contained in the array, not entries that may (or may not) point at
 different objects but happens to share the same name; I think this
 function is misnamed.

 2.  We use object_array_remove_duplicates() to de-dup git bundle
 create x master master, which came from b2a6d1c6 (bundle:
 allow the same ref to be given more than once, 2009-01-17),
 which is still the sole caller of the function, and I think
 this is bogus.  Comparing .name would not de-dup git bundle
 create x master refs/heads/master.

I think the right way to fix these two and a half problems is to do
the following:

 - object_array_remove_duplicates() (and contains_name() helper it
   uses) should be removed from object.c;

 - create_bundle() in bundle.c should implement a helper that is
   similar to contains_name() but knows about ref dwimming and use
   it to call object_array_filter() to replace its call to
   object_array_remove_duplicates().

I am not doing this myself, and I do not expect either you or
Michael to do so, either.  I am just writing this down to point out
a low hanging fruit to aspiring new contributors (hint, hint).

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 v2 06/25] reachable: use traverse_commit_list instead of custom walk

[Junio C Hamano]

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

 To find the set of reachable objects, we add a bunch of
 possible sources to our rev_info, call prepare_revision_walk,
 and then launch into a custom walker that handles each
 object top. This is a subset of what traverse_commit_list
 does, so we can just reuse that code (it can also handle
 more complex cases like UNINTERESTING commits and pathspecs,
 but we don't use those features).

 Signed-off-by: Jeff King p...@peff.net
 ---
 I was concerned this would be slower because traverse_commit_list is
 more featureful. To my surprise, it was consistently about 3-4% faster!
 The major difference is that traverse_commit_list will hit all of the
 commits first, and then the trees. For reachability that doesn't matter
 either way, but I suspect the new way has slightly better cache
 locality, leading to the minor speedup.

I am not very surprised, as custom walk hasn't changed much ever
since it was done in ba84a797 (builtin git prune, 2006-07-06),
while the generic traversal code has been worked heavily while it
was still in builtin-rev-list.c and then later moved to
list-objects.c.

  reachable.c | 130 
 
  1 file changed, 17 insertions(+), 113 deletions(-)

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

Re: [PATCH v4] completion: ignore chpwd_functions when cding on zsh

[Øystein Walle]

Brandon Turner bt at brandonturner.net writes:

 
 On Thu, Oct 9, 2014 at 5:11 PM, Junio C Hamano gitster at pobox.com wrote:
  Actually the patch was slightly wrong.  It did not quite matter as
  cd '' is a no-op, but git -C '' cmd is not that lenient (which
  may be something we may want to fix) and breaks t9902 by exposing
  an existing breakage in the callchain.
 
  Here is a replacement.
 
 I did some more testing on this iteration as well - looks good. 
 

Sorry, for the late reply, guys...

I've tested it too and it seems to work just fine.

As for my comments about using $=2 instead of $2: When zsh runs through
this code it does so while emulating ksh. Thus the reliance on splitting
unquoted expansions is not a problem. I was unaware of this until ten
minutes ago. 

Øsse


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

Re: [PATCH v2 21/25] rev-list: add --index-objects option

[Junio C Hamano]

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

 There is currently no easy way to ask the revision traversal
 machinery to include objects reachable from the index (e.g.,
 blobs and trees that have not yet been committed). This
 patch adds an option to do so.

 Signed-off-by: Jeff King p...@peff.net
 ---
 I was tempted to call this just --index, because I could not think of
 what else --index would mean in the context of rev-list. But I also
 worried about weird interactions with other commands that take revision
 arguments. And since this is mostly for internal use anyway, I figured
 the more verbose name is not too bad. I could be convinced otherwise,
 though.

I agree that --index is a bad name as it usually is used in a
particular context: the command can work on various combination of
working tree and the index, and I am asking it to work on both
(e.g. apply --index as opposed to apply --cached).

 diff --git a/Documentation/rev-list-options.txt 
 b/Documentation/rev-list-options.txt
 index 4cf94c6..03ab343 100644
 --- a/Documentation/rev-list-options.txt
 +++ b/Documentation/rev-list-options.txt
 @@ -172,6 +172,13 @@ explicitly.
   Pretend as if all objects mentioned by reflogs are listed on the
   command line as `commit`.
  
 +--index-objects::

This risks index getting misunderstood as a verb, e.g. please
enumerate the objects and assign labels to later refer to them,
doesn't it?

--indexed-objects (short for --show-objects-in-the-index) or
something?

 + Pretend as if all objects used by the index (any blobs, and any
 + trees which are mentioned by the index's cache-tree extension)
 + ad listed on the command line. Note that you probably want to

s/ad/are/, probably?

 + use `--objects`, too, as there are by definition no commits in
 + the index.

For gitlinks/submodules, the index records names of the commit
objects, they are not listed, and that is the right behaviour, but
this description invites some confusion.

--
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 describe oddity with GIT_DIR

[Thomas Braun]

Am 16.10.2014 um 18:57 schrieb Junio C Hamano:
 Thomas Braun thomas.br...@virtuell-zuhause.de writes:
 
 I've encountered an oddity with git describe.
 Consider the following snippet:
 -
 mkdir test
 cd test
 git init
 echo 1  file
 git add file
 git commit -m changes
 $ git describe --always --dirty
 8ad486e
 $ cd ..
 $ git --git-dir=test/.git describe --always --dirty
 8ad486e-dirty
 $ GIT_DIR=test/.git git describe --always --dirty
 8ad486e-dirty
 -

 The -dirty suffix appears if invoking git not from the worktree
 itself, but should actually never appear.
 
 This is not oddity with describe.  You are using --git-dir incorrectly.
 
 When you tell Git where its repository resides with the $GIT_DIR
 environment variable or the --git-dir command-line option, unless
 you tell it where the top-level of your working tree is, you are
 telling that your current working directory is the top-level of your
 working tree.
 
 You are asking git describe to describe the state of the HEAD
 including the dirtyness of the working tree in various ways.  With
 the first invocation, you do not tell Git where things are and let
 it correctly figure it out, i.e. you are in 'test' directory and
 relative to where you are, .git is the repository and . is the
 top of the working tree.  The commit recorded in the .git/HEAD,
 i.e. 8ad486e, is used, and its compared with the working tree to
 determine dirtiness.  Specifically, the blob object 8ad486e:file is
 the same as ./file (that is test/file relative to where you
 started with mkdir test above).
 
 With the latter two, you are asking the same question but you go one
 level up and then tell Git that the repository is test/.git
 (correct) and the top of the working tree is . (a lie).  Again,
 test/.git/HEAD records the same commit, but when trying to compare
 the contents of its tree, e.g. file at the top-level in the
 commit, you do not have file in the working tree.  Git is led to
 believe that you removed file, hence your working tree state is
 dirty.
 
 Make it a habit to always specify GIT_WORK_TREE when you use
 GIT_DIR, unless you know you will always start from the top of the
 working tree.

Thanks a lot Junio for the in-depth explanation.
I'll promise to do more research next time :)

--
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] clone: --dissociate option to mark that reference is only temporary

[Junio C Hamano]

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

 Marc Branchaud marcn...@xiplink.com writes:

 I think things would be more understandable if the option was --dissociate
 repository and was an explicit alternative to --reference:
  [[--reference | --dissociate] repository]

 I'm still not liking the name --dissociate though.  The original suggestion
 of --borrow is better.  Perhaps --library or --local-cache?  I dunno...

 I was not thinking when I originally started the topic with
 --borrow, until I realized that it would not make much sense,
 primarily because we allow multiple references.

 What should this command line do, and how would you implement such a
 behaviour?

 $ git clone \
 --reference=/local/pool/linux.git \
 --borrow=../my/neighbour/linux-hack.git \
 git://git.kernel.org/./linux.git

 With do the usual --reference thing, but then dissociate the result
 from referents option, there is no ambiguity and that is why I did
 not go with the --borrow option suggested in the original thread.

Another approach we could take is to add --borrow and then forbid
mixing --reference and --borrow on the same command line, until
somebody comes up with an implementation to allow us dissociate from
borrowed repositories while still depending on referenced ones, at
which time we can lift that limitation.

But if that future extension is not going to happen, there is not
much difference.  The end result will be either

 - the one that is very clear to the users that they cannot
   selectively dissociate because there is no such option documented
   (i.e. --reference, --dissociate and no --borrow); and

 - the other one that gives a slight hope to the users that the
   combination might work (i.e. with --reference, --borrow and no
   --dissociate) but refuses to do so when it actually is run.

Between the two, the former might actually be easier to the users to
understand, as it keeps their expectation in line with the reality.

So I dunno.

I certainly am *not* going to do the selective dissociation change
myself.  Do we have a volunteer (hint: this probably does not fall
into low-hanging fruit category)?

--
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] core.filemode may need manual action

[Torsten Bögershausen]

core.filemode is set automatically when a repo is created.
But when a repo is exported via CIFS or cygwin is mixed with Git for Windows
core.filemode may better be set manually to false.
Update and improve the documentation.

Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Torsten Bögershausen tbo...@web.de
---

Does this reflect the discussion via email ?
Or is more tweaking needed ?


 Documentation/config.txt | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 4333636..b4fea43 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -204,8 +204,23 @@ advice.*::
 --
 
 core.fileMode::
-   If false, the executable bit differences between the index and
-   the working tree are ignored; useful on broken filesystems like FAT.
+   Tells Git if the executable bit of files in the working tree
+   is to be honored.
+
+   Some filesystems lose the executable bit when a file that is
+   marked as executable is checked out, or checks out an
+   non-executable file with executable bit on.  git init and
+   git clone probe the filesystem to see if it records
+   executable bit correctly when they create a new repository
+   and this variable is automatically set as necessary.
+
+   A repository, however, may be on a filesystem that records
+   the filemode correctly, and this variable is set to 'true'
+   when created, but later may be made accessible from another
+   environment that loses the filemode (e.g. exporting ext4 via
+   CIFS mount, visiting a Cygwin managed repository with
+   MsysGit).  In such a case, it may be necessary to set this
+   variable to 'false'.
See linkgit:git-update-index[1].
 +
 The default is true, except linkgit:git-clone[1] or linkgit:git-init[1]
-- 
2.0.0.GIT

--
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] Update gitweb.perl to current CGI.pm API

[Junio C Hamano]

Looks sensible to me; Jakub, ack?

Roland Mas lola...@debian.org writes:

 Hi all,

   This simple two-line patch fixes a bug that makes gitweb unusable on
 systems where the installed CGI.pm is version 4.04 or later (such as on
 Debian unstable).  That version removed the startform method, which had
 previously been deprecated in favour of start_form since 2009.

   I don't have any specific tests for that change, but it does help
 fixing the testsuite of FusionForge (which includes a test of its Git
 plugin, involving gitweb).

   For reference, this is Debian bug #765525 (http://bugs.debian.org/765525).

   (I'm not subscribed to the git@vger mailing-list; please Cc me on
 replies.)

   Thanks,

 Roland.

 From 1b74cfb8568927a307f165e428455789398f6d61 Mon Sep 17 00:00:00 2001
 From: Roland Mas lola...@debian.org
 Date: Thu, 16 Oct 2014 00:05:25 +0200
 Subject: [PATCH] Update gitweb.perl to current CGI.pm API

 CGI.pm 4.04 removed the startform method, which had previously been
 deprecated in favour of start_form.  Updated gitweb.perl accordingly.

 Signed-off-by: Roland Mas lola...@debian.org
 ---
  gitweb/gitweb.perl | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
 index a9f57d6..ccf7516 100755
 --- a/gitweb/gitweb.perl
 +++ b/gitweb/gitweb.perl
 @@ -4100,7 +4100,7 @@ sub print_search_form {
   if ($use_pathinfo) {
   $action .= /.esc_url($project);
   }
 - print $cgi-startform(-method = get, -action = $action) .
 + print $cgi-start_form(-method = get, -action = $action) .
 div class=\search\\n .
 (!$use_pathinfo 
 $cgi-input({-name=p, -value=$project, -type=hidden}) . 
 \n) .
 @@ -5510,7 +5510,7 @@ sub git_project_search_form {
   }
  
   print div class=\projsearch\\n;
 - print $cgi-startform(-method = 'get', -action = $my_uri) .
 + print $cgi-start_form(-method = 'get', -action = $my_uri) .
 $cgi-hidden(-name = 'a', -value = 'project_list')  . \n;
   print $cgi-hidden(-name = 'pf', -value = $project_filter). \n
   if (defined $project_filter);
--
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] core.filemode may need manual action

[Thomas Braun]

Am 16.10.2014 um 21:29 schrieb Torsten Bögershausen:
 core.filemode is set automatically when a repo is created.
 But when a repo is exported via CIFS or cygwin is mixed with Git for Windows
 core.filemode may better be set manually to false.
 Update and improve the documentation.
 
 Helped-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: Torsten Bögershausen tbo...@web.de
 ---
 
 Does this reflect the discussion via email ?
 Or is more tweaking needed ?
 
 
  Documentation/config.txt | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 4333636..b4fea43 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -204,8 +204,23 @@ advice.*::
  --
  
  core.fileMode::
 - If false, the executable bit differences between the index and
 - the working tree are ignored; useful on broken filesystems like FAT.
 + Tells Git if the executable bit of files in the working tree
 + is to be honored.
 +
 + Some filesystems lose the executable bit when a file that is
 + marked as executable is checked out, or checks out an
 + non-executable file with executable bit on.  git init and
 + git clone probe the filesystem to see if it records
 + executable bit correctly when they create a new repository
 + and this variable is automatically set as necessary.
 +
 + A repository, however, may be on a filesystem that records
 + the filemode correctly, and this variable is set to 'true'
 + when created, but later may be made accessible from another
 + environment that loses the filemode (e.g. exporting ext4 via
 + CIFS mount, visiting a Cygwin managed repository with
 + MsysGit).  In such a case, it may be necessary to set this
 + variable to 'false'.
   See linkgit:git-update-index[1].
  +
  The default is true, except linkgit:git-clone[1] or linkgit:git-init[1]
 

[CC'ing msysgit aka git-for-windows/sdk for input]

I'm not really happy with the term MsysGit here.
Would it be too bold to say
[... ] visiting a Cygwin managed repository with Git for Windows.
?

--
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] core.filemode may need manual action

[Johannes Schindelin]

Hi,

On Thu, 16 Oct 2014, Thomas Braun wrote:

 Am 16.10.2014 um 21:29 schrieb Torsten Bögershausen:
  core.filemode is set automatically when a repo is created.
  But when a repo is exported via CIFS or cygwin is mixed with Git for Windows
  core.filemode may better be set manually to false.
  Update and improve the documentation.
  
  Helped-by: Junio C Hamano gits...@pobox.com
  Signed-off-by: Torsten Bögershausen tbo...@web.de
  ---
  
  Does this reflect the discussion via email ?
  Or is more tweaking needed ?
  
  
   Documentation/config.txt | 19 +--
   1 file changed, 17 insertions(+), 2 deletions(-)
  
  diff --git a/Documentation/config.txt b/Documentation/config.txt
  index 4333636..b4fea43 100644
  --- a/Documentation/config.txt
  +++ b/Documentation/config.txt
  @@ -204,8 +204,23 @@ advice.*::
   --
   
   core.fileMode::
  -   If false, the executable bit differences between the index and
  -   the working tree are ignored; useful on broken filesystems like FAT.
  +   Tells Git if the executable bit of files in the working tree
  +   is to be honored.
  +
  +   Some filesystems lose the executable bit when a file that is
  +   marked as executable is checked out, or checks out an
  +   non-executable file with executable bit on.  git init and
  +   git clone probe the filesystem to see if it records
  +   executable bit correctly when they create a new repository
  +   and this variable is automatically set as necessary.
  +
  +   A repository, however, may be on a filesystem that records
  +   the filemode correctly, and this variable is set to 'true'
  +   when created, but later may be made accessible from another
  +   environment that loses the filemode (e.g. exporting ext4 via
  +   CIFS mount, visiting a Cygwin managed repository with
  +   MsysGit).  In such a case, it may be necessary to set this
  +   variable to 'false'.
  See linkgit:git-update-index[1].
   +
   The default is true, except linkgit:git-clone[1] or linkgit:git-init[1]
  
 
 [CC'ing msysgit aka git-for-windows/sdk for input]
 
 I'm not really happy with the term MsysGit here.
 Would it be too bold to say
 [... ] visiting a Cygwin managed repository with Git for Windows.
 ?

I agree that msysGit is the wrong term. Not only is it about to be
replaced by the Git for Windows SDK, it is *actively* wrong because
msysGit is just the *development environment* to build Git for Windows.
Most users do *not* need msysGit at all.

Ciao,
Dscho

Re: [PATCH] core.filemode may need manual action

[Junio C Hamano]

Torsten Bögershausen tbo...@web.de writes:

 core.filemode is set automatically when a repo is created.
 But when a repo is exported via CIFS or cygwin is mixed with Git for Windows
 core.filemode may better be set manually to false.
 Update and improve the documentation.

 Helped-by: Junio C Hamano gits...@pobox.com
 Signed-off-by: Torsten Bögershausen tbo...@web.de
 ---

 Does this reflect the discussion via email ?
 Or is more tweaking needed ?


  Documentation/config.txt | 19 +--
  1 file changed, 17 insertions(+), 2 deletions(-)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 4333636..b4fea43 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -204,8 +204,23 @@ advice.*::
  --
  
  core.fileMode::
 - If false, the executable bit differences between the index and
 - the working tree are ignored; useful on broken filesystems like FAT.
 + Tells Git if the executable bit of files in the working tree
 + is to be honored.
 +
 + Some filesystems lose the executable bit when a file that is
 + marked as executable is checked out, or checks out an
 + non-executable file with executable bit on.  git init and
 + git clone probe the filesystem to see if it records
 + executable bit correctly when they create a new repository
 + and this variable is automatically set as necessary.
 +
 + A repository, however, may be on a filesystem that records
 + the filemode correctly, and this variable is set to 'true'
 + when created, but later may be made accessible from another
 + environment that loses the filemode (e.g. exporting ext4 via
 + CIFS mount, visiting a Cygwin managed repository with
 + MsysGit).  In such a case, it may be necessary to set this
 + variable to 'false'.
   See linkgit:git-update-index[1].
  +
  The default is true, except linkgit:git-clone[1] or linkgit:git-init[1]

I suspect that the above will not format very well.  Hint: what is
the lone + line before The default is true... doing there?

Aside from Is MsysGit the old name for Git for Windows? raised by
others, I think it may be worthwhile to mention Eclipse, as that is
where the original (by the way, it would have been nice if you left
some pointer to the original discussion when saying the discussion
via email---it took me a while to recall what you are talking
about) from Hilco Wijbenga was about ([$gmane/257689]).  So, perhaps
s/with MsysGit/with Eclipse or Git for Windows/; or something.

Other than that, I do not see anything wrong in there.  Thanks.

As a separate topic, however, we may want to start thinking about
adding a cheat-sheet on platform-specific bits to our documentation.

The alphabetical listing of configuration variables we see here is a
very good way for people to go from variable names to what they do
(e.g. find a variable defined in configuration file of a neighbour
or be instructed to set a variable by project lead and want to learn
why setting the variable to the value is a good idea), but not a
good way to go in the other direction (e.g. have trouble running Git
on a filesystem that mangles filenames and want to find out if there
already is a way to work it around).

The cheat-sheet could be just a list of configuration and
environment variables e.g. Those on Windows may want to check into
these settings.


[Reference]

$gmane/257689:

http://thread.gmane.org/gmane.comp.version-control.git/257558/focus=257689

--
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] Update gitweb.perl to current CGI.pm API

[Junio C Hamano]

Jakub Narębski jna...@gmail.com writes:

 On Thu, Oct 16, 2014 at 9:36 PM, Junio C Hamano gits...@pobox.com wrote:

 Looks sensible to me; Jakub, ack?


 Ack.

 Nb. this code follows back to original gitweb.cgi by Kay Sievers, very
 early in the development (2005)

Thanks.  I realize that Ack sounds as if Yeah, I acknowledge and
admit I was in the wrong earlier, but I didn't mean I think this
is your bug---do you think it is a good fix?  I just meant the
latter half of that sentence.

Here is what I'll queue (note: I've retitled so that an entry in
git shortlog would mean something).

Thanks, both.

-- 8 --
From: Roland Mas lola...@debian.org
Subject: [PATCH] gitweb: use start_form, not startform that was removed in 
CGI.pm 4.04

CGI.pm 4.04 removed the startform method, which had previously been
deprecated in favour of start_form.  Changes file for CGI.pm says:

4.04 2014-09-04
 [ REMOVED / DEPRECATIONS ]
- startform and endform methods removed (previously deprecated,
  you should be using the start_form and end_form methods)

Signed-off-by: Roland Mas lola...@debian.org
Reviewed-by: Jakub Narębski jna...@gmail.com
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 gitweb/gitweb.perl | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a9f57d6..ccf7516 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -4100,7 +4100,7 @@ sub print_search_form {
if ($use_pathinfo) {
$action .= /.esc_url($project);
}
-   print $cgi-startform(-method = get, -action = $action) .
+   print $cgi-start_form(-method = get, -action = $action) .
  div class=\search\\n .
  (!$use_pathinfo 
  $cgi-input({-name=p, -value=$project, -type=hidden}) . 
\n) .
@@ -5510,7 +5510,7 @@ sub git_project_search_form {
}
 
print div class=\projsearch\\n;
-   print $cgi-startform(-method = 'get', -action = $my_uri) .
+   print $cgi-start_form(-method = 'get', -action = $my_uri) .
  $cgi-hidden(-name = 'a', -value = 'project_list')  . \n;
print $cgi-hidden(-name = 'pf', -value = $project_filter). \n
if (defined $project_filter);
-- 
2.1.2-561-gc401a55

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

Re: [PATCH v2 00/11] Consolidate ref parsing code

[Junio C Hamano]

Michael Haggerty mhag...@alum.mit.edu writes:

 This is a rif on Duy's oldish patch series [1]. I started reviewing
 his patch series, but found that some of his patches did multiple
 things, making it harder to review. I started pulling it apart into
 smaller changes to aid my review, and I guess I got carried away :-/

Hmmm...

You are aware of another large change in flight in the same area,
and after having reviewed that series a few times I was hoping that
you would have a better understanding of how ready the other topic
is.  And then I see this series that conflicts with that other
topic.  Is this your way to say that the other topic is not quite
ready?

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

Re: [PATCH 0/4] Multiple worktrees vs. submodules fixes

[Max Kirillov]

On Wed, Oct 15, 2014 at 08:57:20PM +0200, Jens Lehmann wrote:
 Am 15.10.2014 um 00:15 schrieb Max Kirillov:
 I think the logic can be simple: it a submodule is not
 checked-out in the repository checkout --to is called
 from, then it is not checked-out to the new one also. If it
 is, then checkout calls itself recursively in the submodule
 and works like being run in standalone repository.
 
 But when I later decide to populate the submodule in a
 checkout --to work tree, should it automagically also
 use the central storage, creating the modules/name
 directory there if it doesn't exist yet? I think that'd
 make sense to avoid having the work tree layout depend
 on the order commands were ran in. And imagine new
 submodules, they should not be handled differently from
 those already present.

Like place the common directory to
$MAIN_REPO/.git/modules/$SUB/ and worktree-specific part to
$MAIN_REPO/.git/worktrees/$WORKTREE/modules/$SUB, rather
than placing all into the socond one? It would make sense to
make, but then it would be imposible to checkout a diferent
repository into the same submodule in different superproject
checkouts. However stupid is sounds, there could be cases
if, for example, at some moment submodule is being replaced
by another one, and older worktrees should work with older
submodule, while newer uses the newer submodule.

Maybe, there could be some options to tell the command which
populates submodules (which commands that are? submodule update
and other submodule subcommands? or there is something
else?) to use the curent checkout space or the main one. But
I would still leave it depend on what user explicitly calls
and where the initial submodule update is executed.

Also, could you clarify the usage of the /modules/
directory. I did not notice it to affect anything after the
submofule is placed there. Submodule operations use the
submodule repositories directly (through the git link, which
can point anywhere), or in .gitmodules file, or maybe in
.git/config. So there is actually no need to have that
gitdir there. Is it correct?

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

Re: [PATCH v2 0/25] prune-safety

[Junio C Hamano]

Somewhere in this series the object enumeration seems to get
broken.  git clone --no-local of git.git repository died
with

Cloning into 'victim-7'...
remote: Counting objects: 173727, done.
remote: Compressing objects: 100% (43697/43697), done.
remote: Total 173727 (delta 130908), reused 170009 (delta 128151)
Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s,
done.
Resolving deltas: 100% (130908/130908), done.
fatal: did not receive expected object 
a74380c3117994efba501df1707418eb6feb9284
fatal: index-pack failed

where a74380c... is such an object.

If you have a clone of https://github.com/git/git.git

$ git rev-list --parents --objects --all | grep  a74380c3117994

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

Re: [PATCH v2 0/25] prune-safety

[Junio C Hamano]

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

 Somewhere in this series the object enumeration seems to get
 broken.  git clone --no-local of git.git repository died
 with

 Cloning into 'victim-7'...
 remote: Counting objects: 173727, done.
 remote: Compressing objects: 100% (43697/43697), done.
 remote: Total 173727 (delta 130908), reused 170009 (delta 128151)
 Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s,
 done.
 Resolving deltas: 100% (130908/130908), done.
 fatal: did not receive expected object 
 a74380c3117994efba501df1707418eb6feb9284
 fatal: index-pack failed

 where a74380c... is such an object.

Ehh, such is a nonsense.  It is a blob that directly is pointed by
a tag that is in refs/tags/*.

 If you have a clone of https://github.com/git/git.git

 $ git rev-list --parents --objects --all | grep  a74380c3117994

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

Re: [PATCH v2 0/25] prune-safety

[Jeff King]

On Thu, Oct 16, 2014 at 02:07:47PM -0700, Junio C Hamano wrote:

 Somewhere in this series the object enumeration seems to get
 broken.  git clone --no-local of git.git repository died
 with
 
 Cloning into 'victim-7'...
 remote: Counting objects: 173727, done.
 remote: Compressing objects: 100% (43697/43697), done.
 remote: Total 173727 (delta 130908), reused 170009 (delta 128151)
 Receiving objects: 100% (173727/173727), 33.45 MiB | 13.69 MiB/s,
 done.
 Resolving deltas: 100% (130908/130908), done.
 fatal: did not receive expected object 
 a74380c3117994efba501df1707418eb6feb9284
 fatal: index-pack failed
 
 where a74380c... is such an object.
 
 If you have a clone of https://github.com/git/git.git

Hrm. I cannot reproduce the clone failure...

 $ git rev-list --parents --objects --all | grep  a74380c3117994
 
 would be an easy way to reproduce.

But this I can. Digging into 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 v2 0/25] prune-safety

[Jeff King]

On Thu, Oct 16, 2014 at 05:21:12PM -0400, Jeff King wrote:

 Hrm. I cannot reproduce the clone failure...

Oh, because I have bitmaps turned on, and we do not run the list-objects
code at all in that case.

The bug unsurprisingly bisects to traverse_commit_list: support
pending blobs/trees with paths. The problem is that I didn't notice
that handle_commit actually rewrites the object pointer when
unwrapping the tags, and that commit reuses the original pointer from
the entry. So we end up putting two copies of the tag object into the
pending list, rather than the tag and the blob.

The fix is:

diff --git a/revision.c b/revision.c
index 9a0f99a..8030fc8 100644
--- a/revision.c
+++ b/revision.c
@@ -231,12 +231,6 @@ static void add_pending_object_with_mode(struct rev_info 
*revs,
add_pending_object_with_path(revs, obj, name, mode, NULL);
 }
 
-static void add_pending_object_from_entry(struct rev_info *revs,
- struct object_array_entry *e)
-{
-   add_pending_object_with_path(revs, e-item, e-name, e-mode, e-path);
-}
-
 void add_pending_object(struct rev_info *revs,
struct object *obj, const char *name)
 {
@@ -283,6 +277,8 @@ static struct commit *handle_commit(struct rev_info *revs,
 {
struct object *object = entry-item;
const char *name = entry-name;
+   const char *path = entry-path;
+   unsigned int mode = entry-mode;
unsigned long flags = object-flags;
 
/*
@@ -301,6 +297,14 @@ static struct commit *handle_commit(struct rev_info *revs,
die(bad object %s, sha1_to_hex(tag-tagged-sha1));
}
object-flags |= flags;
+   /*
+* We'll handle the tagged object by looping or dropping
+* through to the non-tag handlers below. Do not
+* propagate data from the tag's pending entry.
+*/
+   name = NULL;
+   path = NULL;
+   mode = 0;
}
 
/*
@@ -332,7 +336,7 @@ static struct commit *handle_commit(struct rev_info *revs,
mark_tree_contents_uninteresting(tree);
return NULL;
}
-   add_pending_object_from_entry(revs, entry);
+   add_pending_object_with_path(revs, object, name, mode, path);
return NULL;
}
 
@@ -344,7 +348,7 @@ static struct commit *handle_commit(struct rev_info *revs,
return NULL;
if (flags  UNINTERESTING)
return NULL;
-   add_pending_object_from_entry(revs, entry);
+   add_pending_object_with_path(revs, object, name, mode, path);
return NULL;
}
die(%s is unknown object, name);

We should probably add a test for cloning a tag of an otherwise
unreferenced object, too.

-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

What's cooking in git.git (Oct 2014, #04; Thu, 16)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* bw/trace-no-inline-getnanotime (2014-09-29) 1 commit
  (merged to 'next' on 2014-10-14 at 19facbb)
 + trace.c: do not mark getnanotime() as inline

 No file-scope static variables in an inlined function, please.


* jc/completion-no-chdir (2014-10-09) 1 commit
  (merged to 'next' on 2014-10-14 at 1cf12e1)
 + completion: use git -C $there instead of (cd $there  git ...)


* po/everyday-doc (2014-10-10) 3 commits
  (merged to 'next' on 2014-10-13 at daf1d03)
 + doc: add 'everyday' to 'git help'
 + doc: Makefile regularise OBSOLETE_HTML list building
 + doc: modernise everyday.txt wording and format in man page style

 git help everyday to show the Everyday Git document.

--
[New Topics]

* da/mergetool-temporary-directory (2014-10-16) 2 commits
 - t7610-mergetool: add test cases for mergetool.writeToTemp
 - mergetool: add an option for writing to a temporary directory
 (this branch uses da/mergetool-temporary-filename and da/mergetool-tests; is 
tangled with da/mergetool-tool-help.)

 Allow a temporary directory specified to be used while running git
 mergetool backend.

 Will merge to 'next'.


* da/mergetool-tests (2014-10-16) 4 commits
 - test-lib-functions: adjust style to match CodingGuidelines
 - t7610-mergetool: use test_config to isolate tests
 - t7610-mergetool: add missing  and remove commented-out code
 - t7610-mergetool: use tabs instead of a mix of tabs and spaces
 (this branch is used by da/mergetool-temporary-directory and 
da/mergetool-temporary-filename; is tangled with da/mergetool-tool-help.)

 The clean-up of this test script was long overdue and is a very
 welcome change.

 Will merge to 'next'.


* bc/asciidoctor (2014-10-15) 2 commits
 - Documentation: implement linkgit macro for Asciidoctor
 - Documentation: move some AsciiDoc parameters into variables
 (this branch uses bc/asciidoc.)

 Add machinery to alternatively use AsciiDoctor to format our
 documentation.

 Will merge to 'next'.


* da/mergetool-meld (2014-10-16) 1 commit
 - mergetools/meld: make usage of `--output` configurable and more robust

 Newer versions of 'meld' breaks the auto-detection we use to see if
 they are new enough to support the `--output` option.

 Will merge to 'next'.


* rm/gitweb-start-form (2014-10-16) 1 commit
 - gitweb: use start_form, not startform that was removed in CGI.pm 4.04

 Will merge to 'next'.


* ss/contrib-subtree-contacts (2014-10-15) 2 commits
 - contacts: add a Makefile to generate docs and install
 - subtree: add an install-html target

 Will merge to 'next'.

--
[Stalled]

* je/quiltimport-no-fuzz (2014-09-26) 2 commits
 - git-quiltimport: flip the default not to allow fuzz
 - git-quiltimport.sh: allow declining fuzz with --exact option

 quiltimport drove git apply always with -C1 option to reduce
 context of the patch in order to give more chance to somewhat stale
 patches to apply.  Add an --exact option to disable, and also
 -C$n option to customize this behaviour.  The top patch
 optionally flips the default to --exact.

 Waiting for an Ack.


* eb/no-pthreads (2014-10-13) 2 commits
 - pack-objects: set number of threads before checking and warning
 - index-pack: fix compilation with NO_PTHREADS

 Allow us build with NO_PTHREADS=NoThanks compilation option.  The
 last change (not queued) needs a bit more explanation in its log
 message.


* tr/remerge-diff (2014-09-08) 8 commits
 - log --remerge-diff: show what the conflict resolution changed
 - name-hash: allow dir hashing even when !ignore_case
 - merge-recursive: allow storing conflict hunks in index
 - merge_diff_mode: fold all merge diff variants into an enum
 - combine-diff: do not pass revs-dense_combined_merges redundantly
 - merge-recursive: -Xindex-only to leave worktree unchanged
 - merge-recursive: internal flag to avoid touching the worktree
 - merge-recursive: remove dead conditional in update_stages()

 log -p output learns a new way to let users inspect a merge
 commit by showing the differences between the automerged result
 with conflicts the person who recorded the merge would have seen
 and the final conflict resolution that was recorded in the merge.

 Waiting for a reroll ($gmane/256591).


* hv/submodule-config (2014-06-30) 4 commits
 - do not die on error of parsing fetchrecursesubmodules option
 - use new config API for worktree configurations of submodules
 - extract functions for submodule config set and lookup
 - implement submodule config cache for lookup of submodule names

 Kicked back to 'pu' per request 

Re: [PATCH v2 00/11] Consolidate ref parsing code

[Junio C Hamano]

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

 Michael Haggerty mhag...@alum.mit.edu writes:

 This is a rif on Duy's oldish patch series [1]. I started reviewing
 his patch series, but found that some of his patches did multiple
 things, making it harder to review. I started pulling it apart into
 smaller changes to aid my review, and I guess I got carried away :-/

 Hmmm...

 You are aware of another large change in flight in the same area,
 and after having reviewed that series a few times I was hoping that
 you would have a better understanding of how ready the other topic
 is.  And then I see this series that conflicts with that other
 topic.  Is this your way to say that the other topic is not quite
 ready?

The last one was a rhetorical question and I regret that it came out
a bit harsher than I intended to.  I just wanted to see a bit better
inter-developer coordination, that's all.

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 v2 0/25] prune-safety

[Junio C Hamano]

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

 On Thu, Oct 16, 2014 at 05:21:12PM -0400, Jeff King wrote:

 Hrm. I cannot reproduce the clone failure...

 Oh, because I have bitmaps turned on, and we do not run the list-objects
 code at all in that case.

;-)

 We should probably add a test for cloning a tag of an otherwise
 unreferenced object, too.

Yeah, that too ;-)

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

Re: [PATCH 0/4] Allow building Git with Asciidoctor

[Philip Oakley]

From: brian m. carlson sand...@crustytoothpaste.net
This series is designed to implement the changes necessary to build 
Git

using Asciidoctor instead of AsciiDoc.

[..]

Even with these patches, Asciidoctor warns about everyday.txt and
user-manual.txt.  I'm not sending patches for these right now because
I've seen recent series including those and don't want to cause a
merge conflict.

Does the new version for giteveryday.txt and everyday.txt which 
graduated to master, 1cb3324 (Merge branch 'po/everyday-doc', 
2014-10-16) format OK?


i.e. does 'git help everyday' now correct the Asciidoctor warnings.

I don't have access to Asciidoctor (on MsysGit), but did make sure the 
header underlines were updated.

--
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 v2 00/11] Consolidate ref parsing code

[Michael Haggerty]

On 10/16/2014 10:47 PM, Junio C Hamano wrote:
 Michael Haggerty mhag...@alum.mit.edu writes:
 
 This is a rif on Duy's oldish patch series [1]. I started reviewing
 his patch series, but found that some of his patches did multiple
 things, making it harder to review. I started pulling it apart into
 smaller changes to aid my review, and I guess I got carried away :-/
 
 Hmmm...
 
 You are aware of another large change in flight in the same area,
 and after having reviewed that series a few times I was hoping that
 you would have a better understanding of how ready the other topic
 is.  And then I see this series that conflicts with that other
 topic.  Is this your way to say that the other topic is not quite
 ready?

No, not at all. To be honest, I thought that the changes in this patch
series were localized in an area different than Ronnie and Jonathan's
patches were touching, but stupidly I didn't check for conflicts. Sorry
about that.

There is nothing urgent in this patch series, so I suggest we just put
it back on ice and I will reroll after the dust has settled on
rs/ref-transactions. The conflicts shouldn't be super hard to resolve
(the series don't overlap much *conceptually*), but I'd rather not have
to do it multiple times.

Regarding rs/ref-transaction, I will reply on that thread.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

--
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 v23 0/25] rs/ref-transaction (Use ref transactions, part 3)

[Michael Haggerty]

On 10/15/2014 02:45 AM, Jonathan Nieder wrote:
 This series by Ronnie was last seen on-list at [1].  It includes some
 improvements to the ref-transaction API, improves handling of poorly
 named refs (which should make it easier to tighten the refname format
 checks in the future), and is a stepping stone toward later series
 that use the ref-transaction API more and make it support alternate
 backends (yay!).
 
 The changes since (a merge of 'master' and) v22 are very minor and can
 be seen below[2].  The more important change is that it's rebased
 against current 'master'.
 
 Review comments from Michael and Junio were very helpful in getting
 this in shape.  Thanks much to both.
 
 The series can also be found at
 
   git://repo.or.cz/git/jrn.git tags/rs/ref-transaction
 
 It is based against current 'master' (670a3c1d, 2014-10-14) and
 intended for 'next'.
 
 Thoughts welcome, as always.  Improvements preferred in the form of
 patches on top of the series.

Thanks for the new version. I reviewed the previous version pretty
carefully in Gerrit and was very happy with the overall series, though
there were still some rough spots. It seems to be much more polished now.

Given that I will be traveling for the next two weeks I probably won't
be able to do another thorough review in time. But I just scanned
through patches 01 - 19 and didn't notice any problems. Unfortunately I
have to stop there.

Cheers,
Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu

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

Re: [PATCH v2 00/11] Consolidate ref parsing code

[Duy Nguyen]

On Wed, Oct 15, 2014 at 10:06 PM, Michael Haggerty mhag...@alum.mit.edu wrote:
 As far as I know, Duy isn't actively working on this, so I hope my
 reroll is not unwelcome.

I couldn't be happier that someone does the work for me and I still
benefit from it ;)
-- 
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

(no subject)

[Brokers Home]

--
Are you in a financial distress? do you need financial assistant to 
start up a business or to pay bills? we offer loans to individual's and 
company investors at 3% interest rate.Email us at adeyemisu...@gmail.com


Fill out the details below:

Full name:
Country:
Address:
State:
Zip code:
Amount needed:
Duration:
Tel:
Reason:

Contact us by reply and we give you more response.

Contact Online Manager
Adeyemi Sule
adeyemisu...@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

Re: [PATCH v2 0/25] prune-safety

[Jeff King]

On Thu, Oct 16, 2014 at 03:18:34PM -0700, Junio C Hamano wrote:

  We should probably add a test for cloning a tag of an otherwise
  unreferenced object, too.
 
 Yeah, that too ;-)

Here's that test (after the scissors below). It can be applied totally
separately, though I stuck it in as patch 18.5/25 in the existing
series to confirm that my original series does cause the test to fail,
and that it passes with the patch I posted earlier.

Do you want to just squash the fix I posted earlier (into patch 19, the
traverse_commit_list: ... one)? I'm happy to repost the revised patch,
but my impression of your workflow is that squashing is just as easy
than replacing a patch (i.e., you're running rebase -i either way).

Or I'm happy to re-post the whole series, but it's rather long. :)

 Thanks for a quick diag.

I'm impressed that you found the bug so quickly. :) My biggest fear with
the whole series is that it's disrupting and refactoring some
fundamental parts of the code that might cause regressions. I put a lot
of my faith in the test suite, but that did not work out here (I do
still feel pretty good about the series overall, though I think I'd cook
it longer than usual given the complexity and the areas it touches).

-- 8 --
Subject: t5516: test pushing a tag of an otherwise unreferenced blob

It's not unreasonable to have a tag that points to a blob
that is not part of the normal history. We do this in
git.git to distribute gpg keys. However, we never explicitly
checked in our test suite that this actually works (i.e.,
that pack-objects actually sends the blob because of the tag
mentioning it).

It does in fact work fine, but a recent patch under
discussion broke this, and the test suite didn't notice.
Let's make the test suite more complete.

Signed-off-by: Jeff King p...@peff.net
---
The should have below is belt-and-suspenders. The test actually fails
with my series without the cat-file check, but I'm concerned a bug
that affects pack-objects could also affect the connectivity check on
the receiving side.

 t/t5516-fetch-push.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 67e0ab3..7c8a769 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1277,4 +1277,17 @@ EOF
git push --no-thin --receive-pack=$rcvpck no-thin/.git 
refs/heads/master:refs/heads/foo
 '
 
+test_expect_success 'pushing a tag pushes the tagged object' '
+   rm -rf dst.git 
+   blob=$(echo unreferenced | git hash-object -w --stdin) 
+   git tag -m foo tag-of-blob $blob 
+   git init --bare dst.git 
+   git push dst.git tag-of-blob 
+   # the receiving index-pack should have noticed
+   # any problems, but we double check
+   echo unreferenced expect 
+   git --git-dir=dst.git cat-file blob tag-of-blob actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
2.1.2.596.g7379948

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

Re: [PATCH v2 21/25] rev-list: add --index-objects option

[Jeff King]

On Thu, Oct 16, 2014 at 11:41:35AM -0700, Junio C Hamano wrote:

 I agree that --index is a bad name as it usually is used in a
 particular context: the command can work on various combination of
 working tree and the index, and I am asking it to work on both
 (e.g. apply --index as opposed to apply --cached).

Thanks. I wasn't sure if I was just being paranoid or not, but now there
are at least two of us.

  +--index-objects::
 
 This risks index getting misunderstood as a verb, e.g. please
 enumerate the objects and assign labels to later refer to them,
 doesn't it?
 
 --indexed-objects (short for --show-objects-in-the-index) or
 something?

That sounds reasonable. We could technically do `--indexed` as that is
different from `--index`, but maybe they are still confusingly close.

  +   Pretend as if all objects used by the index (any blobs, and any
  +   trees which are mentioned by the index's cache-tree extension)
  +   ad listed on the command line. Note that you probably want to
 
 s/ad/are/, probably?

Yeah, sorry, vi cruft I think (at least I didn't accidentally insert
C-a C-k ;) ).

  +   use `--objects`, too, as there are by definition no commits in
  +   the index.
 
 For gitlinks/submodules, the index records names of the commit
 objects, they are not listed, and that is the right behaviour, but
 this description invites some confusion.

Good point. How about this:

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 03ab343..3301fde 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -172,12 +172,10 @@ explicitly.
Pretend as if all objects mentioned by reflogs are listed on the
command line as `commit`.
 
---index-objects::
-   Pretend as if all objects used by the index (any blobs, and any
-   trees which are mentioned by the index's cache-tree extension)
-   ad listed on the command line. Note that you probably want to
-   use `--objects`, too, as there are by definition no commits in
-   the index.
+--indexed-objects::
+   Pretend as if all trees and blobs used by the index are listed
+   on the command line.  Note that you probably want to use
+   `--objects`, too.
 
 --ignore-missing::
Upon seeing an invalid object name in the input, pretend as if

I was tempted to not document this at all (nor to add documentation for
--reflog), as I think these are really only going to be used internally.
But it's nice to have documentation even for this internal stuff (if
anything, we should probably be making sure they are just limited to
rev-list plumbing, and not included in the log manpage).

-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

git-bundle rev handling and de-duping

[Jeff King]

[subject tweaked as we have veered quite far off the original, and
 this might get more attention from interested people]

On Thu, Oct 16, 2014 at 10:39:54AM -0700, Junio C Hamano wrote:

  2.  We use object_array_remove_duplicates() to de-dup git bundle
  create x master master, which came from b2a6d1c6 (bundle:
  allow the same ref to be given more than once, 2009-01-17),
  which is still the sole caller of the function, and I think
  this is bogus.  Comparing .name would not de-dup git bundle
  create x master refs/heads/master.

Hmm. We also doesn't respect the UNINTERESTING flag, so it will dedup
foo and ^foo into a single entry. I _think_ this doesn't have any
bad side effects, because they are equivalent (i.e., the flag is carried
on the object, not the pending entry). I would expect this:

  git bundle create ... foo ^foo

to barf (and it does) because the bundle is empty. But with this:

  git bundle create ... another-ref foo ^foo

I would expect the resulting bundle to contain the objects from
another-ref, but still mention foo as an update (we didn't need to
send any objects, since presumably the other side already had them). It
doesn't, but that is not because of the dedup. It is because we generate
the list of refs to send based on the pending list, and we skip all
UNINTERESTING objects (we must, because otherwise ^foo would make an
entry). I.e., it is the same the flag is on the object, not the pending
entry that saved us above now causing its own bug. Obviously the
example above is sort of silly, but if you have:

  # two branches point at the same object
  git branch foo master

  # the other side already has master. Let's send them foo.
  # this will fail because the bundle is empty. That's mildly
  # annoying because we really want to tell them hey, update
  # your foo branch. But at least we get an error.
  git bundle create tmp.bundle foo ^master

  # now the same thing, but we're sending them some other objects.
  # This one succeeds, but silently omits foo from the bundle!
  git bundle create tmp.bundle foo another-ref ^master

I have a feeling that the list needs to be generated from revs.cmdline,
not revs.pending, and the de-duplication needs to happen there (with the
ref resolution that you mention).

I also have the feeling that fast-export had to deal with this exact
same issue. It looks like we use revs.cmdline there. I seem to recall
there was some ongoing work in that area, but I stopped paying close
attention due to some personality conflicts, and I don't know if all of
the issues were ever resolved.

 I think the right way to fix these two and a half problems is to do
 the following:
 
  - object_array_remove_duplicates() (and contains_name() helper it
uses) should be removed from object.c;
 
  - create_bundle() in bundle.c should implement a helper that is
similar to contains_name() but knows about ref dwimming and use
it to call object_array_filter() to replace its call to
object_array_remove_duplicates().

Agreed. The loop in create_bundle right after the de-dup does the
dwimming. Probably it would be simple to just skip duplicates there
using a hashmap or sorted list.

 I am not doing this myself, and I do not expect either you or
 Michael to do so, either.  I am just writing this down to point out
 a low hanging fruit to aspiring new contributors (hint, hint).

I am also not planning on working on it soon, but now we have hopefully
fed plenty of possibilities to anybody who wants to. :)

-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 v2 21/25] rev-list: add --index-objects option

[Jeff King]

On Thu, Oct 16, 2014 at 08:12:31PM -0400, Jeff King wrote:

  --indexed-objects (short for --show-objects-in-the-index) or
  something?
 
 That sounds reasonable. We could technically do `--indexed` as that is
 different from `--index`, but maybe they are still confusingly close.

Here's a re-roll of the final 5 patches of the series with the updated
name (`--indexed-objects`). The name change cascades from patch 22 to
patches 23 and 25 (and I renamed the matching pack-objects option as
well). 24 and 26 are unchanged, but I figured it is easier on you to
replace the whole block of patches at once.

  [22/26]: rev-list: add --indexed-objects option
  [23/26]: reachable: use revision machinery's --indexed-objects code
  [24/26]: pack-objects: use argv_array
  [25/26]: repack: pack objects mentioned by the index
  [26/26]: pack-objects: double-check options before discarding objects
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 23/26] reachable: use revision machinery's --indexed-objects code

[Jeff King]

This does the same thing as our custom code, so let's not
repeat ourselves.

Signed-off-by: Jeff King p...@peff.net
---
 reachable.c | 52 +---
 1 file changed, 1 insertion(+), 51 deletions(-)

diff --git a/reachable.c b/reachable.c
index 0176a88..a647267 100644
--- a/reachable.c
+++ b/reachable.c
@@ -32,56 +32,6 @@ static int add_one_ref(const char *path, const unsigned char 
*sha1, int flag, vo
return 0;
 }
 
-static void add_one_tree(const unsigned char *sha1, struct rev_info *revs)
-{
-   struct tree *tree = lookup_tree(sha1);
-   if (tree)
-   add_pending_object(revs, tree-object, );
-}
-
-static void add_cache_tree(struct cache_tree *it, struct rev_info *revs)
-{
-   int i;
-
-   if (it-entry_count = 0)
-   add_one_tree(it-sha1, revs);
-   for (i = 0; i  it-subtree_nr; i++)
-   add_cache_tree(it-down[i]-cache_tree, revs);
-}
-
-static void add_cache_refs(struct rev_info *revs)
-{
-   int i;
-
-   read_cache();
-   for (i = 0; i  active_nr; i++) {
-   struct blob *blob;
-
-   /*
-* The index can contain blobs and GITLINKs, GITLINKs are hashes
-* that don't actually point to objects in the repository, it's
-* almost guaranteed that they are NOT blobs, so we don't call
-* lookup_blob() on them, to avoid populating the hash table
-* with invalid information
-*/
-   if (S_ISGITLINK(active_cache[i]-ce_mode))
-   continue;
-
-   blob = lookup_blob(active_cache[i]-sha1);
-   if (blob)
-   blob-object.flags |= SEEN;
-
-   /*
-* We could add the blobs to the pending list, but quite
-* frankly, we don't care. Once we've looked them up, and
-* added them as objects, we've really done everything
-* there is to do for a blob
-*/
-   }
-   if (active_cache_tree)
-   add_cache_tree(active_cache_tree, revs);
-}
-
 /*
  * The traversal will have already marked us as SEEN, so we
  * only need to handle any progress reporting here.
@@ -213,7 +163,7 @@ void mark_reachable_objects(struct rev_info *revs, int 
mark_reflog,
revs-tree_objects = 1;
 
/* Add all refs from the index file */
-   add_cache_refs(revs);
+   add_index_objects_to_pending(revs, 0);
 
/* Add all external refs */
for_each_ref(add_one_ref, revs);
-- 
2.1.2.596.g7379948

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

[PATCH v3 22/26] rev-list: add --indexed-objects option

[Jeff King]

There is currently no easy way to ask the revision traversal
machinery to include objects reachable from the index (e.g.,
blobs and trees that have not yet been committed). This
patch adds an option to do so.

Signed-off-by: Jeff King p...@peff.net
---
 Documentation/rev-list-options.txt |  5 
 revision.c | 51 ++
 revision.h |  1 +
 t/t6000-rev-list-misc.sh   | 23 +
 4 files changed, 80 insertions(+)

diff --git a/Documentation/rev-list-options.txt 
b/Documentation/rev-list-options.txt
index 4cf94c6..3301fde 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -172,6 +172,11 @@ explicitly.
Pretend as if all objects mentioned by reflogs are listed on the
command line as `commit`.
 
+--indexed-objects::
+   Pretend as if all trees and blobs used by the index are listed
+   on the command line.  Note that you probably want to use
+   `--objects`, too.
+
 --ignore-missing::
Upon seeing an invalid object name in the input, pretend as if
the bad input was not given.
diff --git a/revision.c b/revision.c
index 8030fc8..a6c5dc3 100644
--- a/revision.c
+++ b/revision.c
@@ -17,6 +17,7 @@
 #include mailmap.h
 #include commit-slab.h
 #include dir.h
+#include cache-tree.h
 
 volatile show_early_output_fn_t show_early_output;
 
@@ -1303,6 +1304,53 @@ void add_reflogs_to_pending(struct rev_info *revs, 
unsigned flags)
for_each_reflog(handle_one_reflog, cb);
 }
 
+static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
+  struct strbuf *path)
+{
+   size_t baselen = path-len;
+   int i;
+
+   if (it-entry_count = 0) {
+   struct tree *tree = lookup_tree(it-sha1);
+   add_pending_object_with_path(revs, tree-object, ,
+04, path-buf);
+   }
+
+   for (i = 0; i  it-subtree_nr; i++) {
+   struct cache_tree_sub *sub = it-down[i];
+   strbuf_addf(path, %s%s, baselen ? / : , sub-name);
+   add_cache_tree(sub-cache_tree, revs, path);
+   strbuf_setlen(path, baselen);
+   }
+
+}
+
+void add_index_objects_to_pending(struct rev_info *revs, unsigned flags)
+{
+   int i;
+
+   read_cache();
+   for (i = 0; i  active_nr; i++) {
+   struct cache_entry *ce = active_cache[i];
+   struct blob *blob;
+
+   if (S_ISGITLINK(ce-ce_mode))
+   continue;
+
+   blob = lookup_blob(ce-sha1);
+   if (!blob)
+   die(unable to add index blob to traversal);
+   add_pending_object_with_path(revs, blob-object, ,
+ce-ce_mode, ce-name);
+   }
+
+   if (active_cache_tree) {
+   struct strbuf path = STRBUF_INIT;
+   add_cache_tree(active_cache_tree, revs, path);
+   strbuf_release(path);
+   }
+}
+
 static int add_parents_only(struct rev_info *revs, const char *arg_, int flags)
 {
unsigned char sha1[20];
@@ -1653,6 +1701,7 @@ static int handle_revision_opt(struct rev_info *revs, int 
argc, const char **arg
!strcmp(arg, --reflog) || !strcmp(arg, --not) ||
!strcmp(arg, --no-walk) || !strcmp(arg, --do-walk) ||
!strcmp(arg, --bisect) || starts_with(arg, --glob=) ||
+   !strcmp(arg, --indexed-objects) ||
starts_with(arg, --exclude=) ||
starts_with(arg, --branches=) || starts_with(arg, --tags=) ||
starts_with(arg, --remotes=) || starts_with(arg, --no-walk=))
@@ -2082,6 +2131,8 @@ static int handle_revision_pseudo_opt(const char 
*submodule,
clear_ref_exclusion(revs-ref_excludes);
} else if (!strcmp(arg, --reflog)) {
add_reflogs_to_pending(revs, *flags);
+   } else if (!strcmp(arg, --indexed-objects)) {
+   add_index_objects_to_pending(revs, *flags);
} else if (!strcmp(arg, --not)) {
*flags ^= UNINTERESTING | BOTTOM;
} else if (!strcmp(arg, --no-walk)) {
diff --git a/revision.h b/revision.h
index e644044..e6dcd5d 100644
--- a/revision.h
+++ b/revision.h
@@ -277,6 +277,7 @@ extern void add_pending_sha1(struct rev_info *revs,
 
 extern void add_head_to_pending(struct rev_info *);
 extern void add_reflogs_to_pending(struct rev_info *, unsigned int flags);
+extern void add_index_objects_to_pending(struct rev_info *, unsigned int 
flags);
 
 enum commit_action {
commit_ignore,
diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh
index 3794e4c..2602086 100755
--- a/t/t6000-rev-list-misc.sh
+++ b/t/t6000-rev-list-misc.sh
@@ -73,4 +73,27 @@ test_expect_success 'symleft flag bit is propagated down 
from tag' '
test_cmp expect actual
 '
 
+test_expect_success 'rev-list can show index 

[PATCH v3 24/26] pack-objects: use argv_array

[Jeff King]

This saves us from having to bump the rp_av count when we
add new traversal options.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/pack-objects.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4df9499..b26276b 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -22,6 +22,7 @@
 #include pack-bitmap.h
 #include reachable.h
 #include sha1-array.h
+#include argv-array.h
 
 static const char *pack_usage[] = {
N_(git pack-objects --stdout [options...] [ ref-list |  
object-list]),
@@ -2614,8 +2615,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
int use_internal_rev_list = 0;
int thin = 0;
int all_progress_implied = 0;
-   const char *rp_av[6];
-   int rp_ac = 0;
+   struct argv_array rp = ARGV_ARRAY_INIT;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
struct option pack_objects_options[] = {
OPT_SET_INT('q', quiet, progress,
@@ -2705,24 +2705,24 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (pack_to_stdout != !base_name || argc)
usage_with_options(pack_usage, pack_objects_options);
 
-   rp_av[rp_ac++] = pack-objects;
+   argv_array_push(rp, pack-objects);
if (thin) {
use_internal_rev_list = 1;
-   rp_av[rp_ac++] = --objects-edge;
+   argv_array_push(rp, --objects-edge);
} else
-   rp_av[rp_ac++] = --objects;
+   argv_array_push(rp, --objects);
 
if (rev_list_all) {
use_internal_rev_list = 1;
-   rp_av[rp_ac++] = --all;
+   argv_array_push(rp, --all);
}
if (rev_list_reflog) {
use_internal_rev_list = 1;
-   rp_av[rp_ac++] = --reflog;
+   argv_array_push(rp, --reflog);
}
if (rev_list_unpacked) {
use_internal_rev_list = 1;
-   rp_av[rp_ac++] = --unpacked;
+   argv_array_push(rp, --unpacked);
}
 
if (!reuse_object)
@@ -2766,8 +2766,8 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
if (!use_internal_rev_list)
read_object_list_from_stdin();
else {
-   rp_av[rp_ac] = NULL;
-   get_object_list(rp_ac, rp_av);
+   get_object_list(rp.argc, rp.argv);
+   argv_array_clear(rp);
}
cleanup_preferred_base();
if (include_tag  nr_result)
-- 
2.1.2.596.g7379948

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

[PATCH v3 25/26] repack: pack objects mentioned by the index

[Jeff King]

When we pack all objects, we use only the objects reachable
from references and reflogs. This misses any objects which
are reachable from the index, but not yet referenced.

By itself this isn't a big deal; the objects can remain
loose until they are actually used in a commit. However, it
does create a problem when we drop packed but unreachable
objects. We try to optimize out the writing of objects that
we will immediately prune, which means we must follow the
same rules as prune in determining what is reachable. And
prune uses the index for this purpose.

This is rather uncommon in practice, as objects in the index
would not usually have been packed in the first place. But
it could happen in a sequence like:

  1. You make a commit on a branch that references blob X.

  2. You repack, moving X into the pack.

  3. You delete the branch (and its reflog), so that X is
 unreferenced.

  4. You git add blob X so that it is now referenced only
 by the index.

  5. You repack again with git-gc. The pack-objects we
 invoke will see that X is neither referenced nor
 recent and not bother loosening it.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/pack-objects.c   |  8 
 builtin/repack.c |  1 +
 t/t7701-repack-unpack-unreachable.sh | 13 +
 3 files changed, 22 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index b26276b..0cf95c9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2617,6 +2617,7 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
int all_progress_implied = 0;
struct argv_array rp = ARGV_ARRAY_INIT;
int rev_list_unpacked = 0, rev_list_all = 0, rev_list_reflog = 0;
+   int rev_list_index = 0;
struct option pack_objects_options[] = {
OPT_SET_INT('q', quiet, progress,
N_(do not show progress meter), 0),
@@ -2663,6 +2664,9 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
{ OPTION_SET_INT, 0, reflog, rev_list_reflog, NULL,
  N_(include objects referred by reflog entries),
  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
+   { OPTION_SET_INT, 0, indexed-objects, rev_list_index, NULL,
+ N_(include objects referred to by the index),
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 1 },
OPT_BOOL(0, stdout, pack_to_stdout,
 N_(output pack to stdout)),
OPT_BOOL(0, include-tag, include_tag,
@@ -2720,6 +2724,10 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
use_internal_rev_list = 1;
argv_array_push(rp, --reflog);
}
+   if (rev_list_index) {
+   use_internal_rev_list = 1;
+   argv_array_push(rp, --indexed-objects);
+   }
if (rev_list_unpacked) {
use_internal_rev_list = 1;
argv_array_push(rp, --unpacked);
diff --git a/builtin/repack.c b/builtin/repack.c
index 2aae05d..2845620 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -209,6 +209,7 @@ int cmd_repack(int argc, const char **argv, const char 
*prefix)
argv_array_push(cmd_args, --non-empty);
argv_array_push(cmd_args, --all);
argv_array_push(cmd_args, --reflog);
+   argv_array_push(cmd_args, --indexed-objects);
if (window)
argv_array_pushf(cmd_args, --window=%s, window);
if (window_memory)
diff --git a/t/t7701-repack-unpack-unreachable.sh 
b/t/t7701-repack-unpack-unreachable.sh
index b8d4cde..aad8a9c 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -109,4 +109,17 @@ test_expect_success 'do not bother loosening old objects' '
test_must_fail git cat-file -p $obj2
 '
 
+test_expect_success 'keep packed objects found only in index' '
+   echo my-unique-content file 
+   git add file 
+   git commit -m make it reachable 
+   git gc 
+   git reset HEAD^ 
+   git reflog expire --expire=now --all 
+   git add file 
+   test-chmtime =-86400 .git/objects/pack/* 
+   git gc --prune=1.hour.ago 
+   git cat-file blob :file
+'
+
 test_done
-- 
2.1.2.596.g7379948

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

[PATCH v3 26/26] pack-objects: double-check options before discarding objects

[Jeff King]

When we are given an expiration time like
--unpack-unreachable=2.weeks.ago, we avoid writing out old,
unreachable loose objects entirely, under the assumption
that running prune would simply delete them immediately
anyway. However, this is only valid if we computed the same
set of reachable objects as prune would.

In practice, this is the case, because only git-repack uses
the --unpack-unreachable option with an expiration, and it
always feeds as many objects into the pack as possible. But
we can double-check at runtime just to be sure.

Signed-off-by: Jeff King p...@peff.net
---
 builtin/pack-objects.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0cf95c9..64123d4 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2757,6 +2757,8 @@ int cmd_pack_objects(int argc, const char **argv, const 
char *prefix)
 
if (keep_unreachable  unpack_unreachable)
die(--keep-unreachable and --unpack-unreachable are 
incompatible.);
+   if (!rev_list_all || !rev_list_reflog || !rev_list_index)
+   unpack_unreachable_expiration = 0;
 
if (!use_internal_rev_list || !pack_to_stdout || 
is_repository_shallow())
use_bitmap_index = 0;
-- 
2.1.2.596.g7379948
--
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] pack-objects: turn off bitmaps when we split packs

[Jeff King]

If a pack.packSizeLimit is set, we may split the pack data
across multiple packfiles. This means we cannot generate
.bitmap files, as they require that all of the reachable
objects are in the same pack. We check that condition when
we are generating the list of objects to pack (and disable
bitmaps if we are not packing everything), but we forgot to
update it when we notice that we needed to split (which
doesn't happen until the actual write phase).

The resulting bitmaps are quite bogus (they mention entries
that do not exist in the pack!) and can cause a fetch or
push to send insufficient objects.

Signed-off-by: Jeff King p...@peff.net
---
You should generally avoid splitting packs anyway, as it introduces
other inefficiencies, so I'm not really concerned about making bitmaps
work here (besides which, it would require a big change to the on-disk
format). The important thing here is avoiding creating the broken
bitmaps.

 builtin/pack-objects.c  | 1 +
 t/t5310-pack-bitmaps.sh | 9 +
 2 files changed, 10 insertions(+)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d391934..a4022a7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -811,6 +811,7 @@ static void write_pack_file(void)
fixup_pack_header_footer(fd, sha1, pack_tmp_name,
 nr_written, sha1, offset);
close(fd);
+   write_bitmap_index = 0;
}
 
if (!pack_to_stdout) {
diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
index 0580258..6003490 100755
--- a/t/t5310-pack-bitmaps.sh
+++ b/t/t5310-pack-bitmaps.sh
@@ -170,4 +170,13 @@ test_expect_success JGIT 'jgit can read our bitmaps' '
)
 '
 
+test_expect_success 'splitting packs does not generate bogus bitmaps' '
+   test-genrandom foo $((1024 * 1024)) rand 
+   git add rand 
+   git commit -m commit with big file 
+   git -c pack.packSizeLimit=500k repack -adb 
+   git init --bare no-bitmaps.git 
+   git -C no-bitmaps.git fetch .. HEAD
+'
+
 test_done
-- 
2.1.2.596.g7379948
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 0/25] prune-safety

[Jeff King]

On Thu, Oct 16, 2014 at 09:13:39PM -0700, Junio C Hamano wrote:

 Just being curious, but would the same bug, if allowed to be triggered
 cutting repacking of your repository, have corrupted the resulting bitmap?

I didn't test, but yes, almost certainly. The bug was in list-objects.c,
which is used by pack-objects to generate the list of objects to pack,
as well as to build the bitmaps. So not only would it have corrupted the
bitmaps, a `git repack -ad`[1] would have dropped the object completely,
corrupting the repository!

-Peff

[1] git-gc uses `repack -Ad`, of course. So assuming you had packed more
recently than 2 weeks ago, it would have just been ejected to a
loose object. Small comfort. :)

-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