Re: Alternative to manual editing with git add --patch

[Johannes Schindelin]

Hi Sven & Junio,

On Thu, 15 Oct 2015, Sven Helmberger wrote:

> Am 14.10.2015 um 19:50 schrieb Junio C Hamano:
> > Sven Helmberger  writes:
> > 
> > As a quick-and-dirty change, you could invent a new variant of
> > 's'plit that breaks a N-line hunk into N hunks with 1-line each, but
> > obviously that would not be a pleasant-enough UI to be called usable
> > when you have a hunk that adds 100 lines.  Perhaps "Split this hunk
> > into two by ending the earlier one immediately before the line that
> > has this substring" or something might be an idea?
> > 
> 
> If we go by the style of interaction in git add --patch and git add
> --interactive, I think the most canonical solution would be to implement
> it like this.
> 
> If we know when we can't split the current patch any further ( the point
> at which selecting s changes nothing anymore), why shouldn't add --patch
> not work similar to add --interactive in that it prints the lines of the
> diff prefixed with numbers and the user can define a numerical range to
> "split off". Then they decide whether to add those lines or not and
> return to the line-numbers until they're trough with the patch.

This is all technically sound. From a usability perspective I would wish
more for a way to exclude or filter the lines by a pattern. Take for
example a diff like this (which is *quite* normal in my workflow):

-- snip --
diff --git a/80a8c9a..001a983 b/001a983
index 80a8c9a..001a983 100644
--- a/80a8c9a..001a983
+++ b/001a983
@@ -23,11 +23,15 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of

hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);
+error("hmap: %p", hmap);

-   if (!hmap)
+   if (!hmap) {
+   errno = EINVAL;
return MAP_FAILED;
+   }

temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
+error("temp: %p 0x%x", temp, (unsigned int)GetLastError());

if (!CloseHandle(hmap))
warning("unable to close file mapping handle");
-- snap --

(Yes, I am lazy and reuse the `error()` function for debug log messages.)

It is quite obvious what I would like to do in this case: keep the change
that sets the errno.

I would be really thankful if I had a shortcut in `git add -p` that let me
specify, say, "Xerror" to eXclude any + line (and replace the '-' by a ' '
in every - line) that contains the substring 'error'.

The way I envisage this command to work would be to present the filtered
diff in the next step:

-- snip --
@@ -23,11 +23,13 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of

hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);

-   if (!hmap)
+   if (!hmap) {
+   errno = EINVAL;
return MAP_FAILED;
+   }

temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);

if (!CloseHandle(hmap))
warning("unable to close file mapping handle");
-- snap --

Likewise, I could imagine that something like "Ierrno" to keep only +
lines with matching substrings (and treating - lines correspondingly).

Having said that, I find myself occasionally powering up a full-fledged
`git gui` just to stage individual lines. If I had an 'L' command in `git
add -p` instead that would present the following hunks, I would be really
happy:

-- snip --
@@ -23,6 +23,7 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of

hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);
+error("hmap: %p", hmap);

if (!hmap)
return MAP_FAILED;
-- snap --

and then

-- snip --
@@ -24,6 +24,5 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of
hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);

-   if (!hmap)
return MAP_FAILED;

temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
-- snap --

and then

-- snip --
@@ -24,6 +24,7 @@ void *git_mmap(void *start, size_t length, int prot, int 
flags, int fd, off_t of
hmap = CreateFileMapping((HANDLE)_get_osfhandle(fd), NULL,
PAGE_WRITECOPY, 0, 0, NULL);

+   if (!hmap) {
return MAP_FAILED;

temp = MapViewOfFileEx(hmap, FILE_MAP_COPY, h, l, length, start);
-- snap --

and so on.

What do you think?

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

Re: [PATCH v3 1/3] Add Travis CI support

[Lars Schneider]

On 15 Oct 2015, at 10:12, Matthieu Moy  wrote:

> Lars Schneider  writes:
> 
>> I was reluctant to this because I feared problems. Especially while
>> running tests in parallel.
> 
> Isn't the point of using a CI tool to notice problems? ;-)
> 
> More seriously, running tests in parallel shouldn't be a problem since
> each test runs in its own directory with HOME set to this private
> directory, so two diffent tests should not interfer. If there's an issue
> with parallel tests, we probably prefer discovering them than avoiding
> them.
OK. Great!


>> make -j2 9min 11sec:
>> https://travis-ci.org/larsxschneider/git/jobs/85478022
>> 
>> make 17min 20sec:
>> https://travis-ci.org/larsxschneider/git/jobs/85432398
> 
> Since the tests are essentially IO-bound and not CPU-bound, it may even
> make sense to use -j3 here.
Hehe you're right.

make -j3 6min 2sec
https://travis-ci.org/larsxschneider/git/jobs/85497307

just for fun I tried a few more values and -j5 seems to be the best with 4min 
27sec
https://travis-ci.org/larsxschneider/git/jobs/85501015

-j6 is slower again. Do you see a reason not to use "-j5"?

Thanks,
Lars

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

[PATCH] Use the alternates of the source repository for dissociating clone

[Alexander Riesen]

The "--dissociate" option required reference repositories, which sometimes
demanded a look into the objects/info/alternates by the user. As this
is something which can be figured out automatically, do it in the
clone unless there is no other reference repositories.

Signed-off-by: Alex Riesen 
---

I often (enough) did something like this (preparing for manual backup):

# Run mc (the Midnight Commander, it is a console (ncurses) file manager 
with
# two panels).
# Select a repository.
$ git clone --mirror --dissociate \
  $(sed -e 's|\(.*\)/objects|--reference \1| -- 
%f/.git/objects/info/alternates' 2>/dev/null) \

  %f %D/%f
# (except that this is one long line in mc)

The '%f' expands to a selected directory on the current panel, %D - to the
current directory on the other panel. But as I see, the combination of the
options "--dissociate" and no given references is not used for anything but
giving the warning yet. So maybe it can be used as a shortcut for the above?

 builtin/clone.c | 42 ++
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 578da85..344bf21 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -791,6 +791,38 @@ static void write_refspec_config(const char 
*src_ref_prefix,
 strbuf_release();
 }

+static int copy_alternates_to_references(const char *src_repo)
+{
+int refcnt = -1;
+char *src_alternates = mkpathdup("%s/objects/info/alternates", src_repo);
+FILE *in = fopen(src_alternates, "r");
+if (in) {
+struct strbuf line = STRBUF_INIT;
+refcnt = 0;
+while (strbuf_getline(, in, '\n') != EOF) {
+struct string_list_item item;
+if (line.len < 8 || line.buf[0] == '#')
+continue;
+++refcnt;
+if (!strcmp(line.buf + line.len - 8, "/objects"))
+line.buf[line.len - 8] = '\0';
+if (is_absolute_path(line.buf)) {
+item.string = line.buf;
+add_one_reference(, NULL);
+continue;
+}
+item.string = mkpathdup("%s/objects/%s", src_repo, line.buf);
+normalize_path_copy(item.string, item.string);
+add_one_reference(, NULL);
+free(item.string);
+}
+strbuf_release();
+fclose(in);
+}
+free(src_alternates);
+return refcnt;
+}
+
 static void dissociate_from_references(void)
 {
 static const char* argv[] = { "repack", "-a", "-d", NULL };
@@ -947,10 +979,6 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)

 if (option_reference.nr)
 setup_reference();
-else if (option_dissociate) {
-warning(_("--dissociate given, but there is no --reference"));
-option_dissociate = 0;
-}

 fetch_pattern = value.buf;
 refspec = parse_fetch_refspec(1, _pattern);
@@ -976,6 +1004,12 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
 warning(_("--local is ignored"));
 transport->cloning = 1;

+if (!option_reference.nr && option_dissociate &&
+copy_alternates_to_references(path) <= 0) {
+warning(_("--dissociate given, but there is no --reference"));
+option_dissociate = 0;
+}
+
 if (!transport->get_refs_list || (!is_local && !transport->fetch))
 die(_("Don't know how to clone %s"), transport->url);

--
2.6.1.150.gb633014

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

[PATCH 0/3] stripspace: Implement and use --count-lines option

[Tobias Klauser]

This patch set implements some of the project ideas around git stripspace
suggested on [1].

[1] https://git.wiki.kernel.org/index.php/SmallProjectsIdeas

The first patch moves the stripspace() function to the
strbuf module (adding a prefix and changing all users accordingly, also
a wrapper is introduced in case any topic branches still depend on the
old name). The second patch introduces option --count-lines to git
stripspace and also adds documentation and tests accordingly, Finally,
the third patch changes git-rebase--interactive.sh to replace commands
like:

git stripspace ... | wc -l

with:

git stripspace --count-lines ...

Tobias Klauser (3):
  strbuf: make stripspace() part of strbuf
  stripspace: Implement --count-lines option
  git rebase -i: Use newly added --count-lines option for stripspace

 Documentation/git-stripspace.txt |  13 -
 builtin/am.c |   2 +-
 builtin/branch.c |   2 +-
 builtin/commit.c |   6 +-
 builtin/merge.c  |   2 +-
 builtin/notes.c  |   6 +-
 builtin/stripspace.c | 122 ++-
 builtin/tag.c|   2 +-
 git-rebase--interactive.sh   |   6 +-
 strbuf.c |  72 +++
 strbuf.h |  14 -
 t/t0030-stripspace.sh|  36 
 12 files changed, 177 insertions(+), 106 deletions(-)

-- 
2.6.1.145.gb27dacc


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

[PATCH 1/3] strbuf: make stripspace() part of strbuf

[Tobias Klauser]

Rename stripspace() to strbuf_stripspace() and move it to the strbuf
module as suggested in [1].

Also switch all current users of stripspace() to the new function name
and  keep a temporary wrapper inline function for topic branches still
using stripspace().

[1] 
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf

Signed-off-by: Tobias Klauser 
---
 builtin/am.c |  2 +-
 builtin/branch.c |  2 +-
 builtin/commit.c |  6 ++---
 builtin/merge.c  |  2 +-
 builtin/notes.c  |  6 ++---
 builtin/stripspace.c | 69 ++--
 builtin/tag.c|  2 +-
 strbuf.c | 66 +
 strbuf.h | 11 -
 9 files changed, 88 insertions(+), 78 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 4f77e07..fbe9152 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1343,7 +1343,7 @@ static int parse_mail(struct am_state *state, const char 
*mail)
strbuf_addstr(, "\n\n");
if (strbuf_read_file(, am_path(state, "msg"), 0) < 0)
die_errno(_("could not read '%s'"), am_path(state, "msg"));
-   stripspace(, 0);
+   strbuf_stripspace(, 0);
 
if (state->signoff)
am_signoff();
diff --git a/builtin/branch.c b/builtin/branch.c
index 3ba4d1b..3f48746 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -794,7 +794,7 @@ static int edit_branch_description(const char *branch_name)
strbuf_release();
return -1;
}
-   stripspace(, 1);
+   strbuf_stripspace(, 1);
 
strbuf_addf(, "branch.%s.description", branch_name);
status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
diff --git a/builtin/commit.c b/builtin/commit.c
index 63772d0..dca09e2 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -775,7 +775,7 @@ static int prepare_to_commit(const char *index_file, const 
char *prefix,
s->hints = 0;
 
if (clean_message_contents)
-   stripspace(, 0);
+   strbuf_stripspace(, 0);
 
if (signoff)
append_signoff(, ignore_non_trailer(), 0);
@@ -1014,7 +1014,7 @@ static int template_untouched(struct strbuf *sb)
if (!template_file || strbuf_read_file(, template_file, 0) <= 0)
return 0;
 
-   stripspace(, cleanup_mode == CLEANUP_ALL);
+   strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
if (!skip_prefix(sb->buf, tmpl.buf, ))
start = sb->buf;
strbuf_release();
@@ -1726,7 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
wt_status_truncate_message_at_cut_line();
 
if (cleanup_mode != CLEANUP_NONE)
-   stripspace(, cleanup_mode == CLEANUP_ALL);
+   strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
if (template_untouched() && !allow_empty_message) {
rollback_index_files();
fprintf(stderr, _("Aborting commit; you did not edit the 
message.\n"));
diff --git a/builtin/merge.c b/builtin/merge.c
index a0edaca..e6741f3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -806,7 +806,7 @@ static void prepare_to_commit(struct commit_list 
*remoteheads)
abort_commit(remoteheads, NULL);
}
read_merge_msg();
-   stripspace(, 0 < option_edit);
+   strbuf_stripspace(, 0 < option_edit);
if (!msg.len)
abort_commit(remoteheads, _("Empty commit message."));
strbuf_release(_msg);
diff --git a/builtin/notes.c b/builtin/notes.c
index 3608c64..bb23d55 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -192,7 +192,7 @@ static void prepare_note_data(const unsigned char *object, 
struct note_data *d,
if (launch_editor(d->edit_path, >buf, NULL)) {
die(_("Please supply the note contents using either -m 
or -F option"));
}
-   stripspace(>buf, 1);
+   strbuf_stripspace(>buf, 1);
}
 }
 
@@ -215,7 +215,7 @@ static int parse_msg_arg(const struct option *opt, const 
char *arg, int unset)
if (d->buf.len)
strbuf_addch(>buf, '\n');
strbuf_addstr(>buf, arg);
-   stripspace(>buf, 0);
+   strbuf_stripspace(>buf, 0);
 
d->given = 1;
return 0;
@@ -232,7 +232,7 @@ static int parse_file_arg(const struct option *opt, const 
char *arg, int unset)
die_errno(_("cannot read '%s'"), arg);
} else if (strbuf_read_file(>buf, arg, 1024) < 0)
die_errno(_("could not open or read '%s'"), arg);
-   stripspace(>buf, 0);
+   strbuf_stripspace(>buf, 0);
 
d->given = 1;
return 0;
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index 1259ed7..f677093 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -1,71 +1,6 @@
 

[PATCH 2/3] stripspace: Implement --count-lines option

[Tobias Klauser]

As suggested in the small project ideas [1], implement a --count-lines
options for git stripspace to be able to omit calling

  git stripspace --strip-comments < infile | wc -l

e.g. in git-rebase--interactive.sh. The above command can now be
replaced by:

  git stripspace --strip-comments --count-lines < infile

This will also make it easier to port git-rebase--interactive.sh to C
later on.

Furthermore, add the corresponding documentation and tests.

[1] 
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27

Signed-off-by: Tobias Klauser 
---
 Documentation/git-stripspace.txt | 13 -
 builtin/stripspace.c | 57 ++--
 strbuf.c | 12 ++---
 strbuf.h |  5 ++--
 t/t0030-stripspace.sh| 36 +
 5 files changed, 92 insertions(+), 31 deletions(-)

diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt
index 60328d5..411e17c 100644
--- a/Documentation/git-stripspace.txt
+++ b/Documentation/git-stripspace.txt
@@ -9,7 +9,7 @@ git-stripspace - Remove unnecessary whitespace
 SYNOPSIS
 
 [verse]
-'git stripspace' [-s | --strip-comments] < input
+'git stripspace' [-s | --strip-comments] [-C | --count-lines] < input
 'git stripspace' [-c | --comment-lines] < input
 
 DESCRIPTION
@@ -44,6 +44,11 @@ OPTIONS
be terminated with a newline. On empty lines, only the comment character
will be prepended.
 
+-C::
+--count-lines::
+   Output the number of resulting lines after stripping. This is equivalent
+   to calling 'git stripspace | wc -l'.
+
 EXAMPLES
 
 
@@ -88,6 +93,12 @@ Use 'git stripspace --strip-comments' to obtain:
 |The end.$
 -
 
+Use 'git stripspace --count-lines' to obtain:
+
+-
+|5$
+-
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index f677093..7882edd 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "strbuf.h"
 
 static void comment_lines(struct strbuf *buf)
@@ -12,45 +13,51 @@ static void comment_lines(struct strbuf *buf)
free(msg);
 }
 
-static const char *usage_msg = "\n"
-"  git stripspace [-s | --strip-comments] < input\n"
-"  git stripspace [-c | --comment-lines] < input";
+static const char * const usage_msg[] = {
+   N_("git stripspace [-s | --strip-comments] [-C | --count-lines] < 
input"),
+   N_("git stripspace [-c | --comment-lines] < input"),
+   NULL
+};
 
 int cmd_stripspace(int argc, const char **argv, const char *prefix)
 {
struct strbuf buf = STRBUF_INIT;
-   int strip_comments = 0;
-   enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = 
STRIP_SPACE;
-
-   if (argc == 2) {
-   if (!strcmp(argv[1], "-s") ||
-   !strcmp(argv[1], "--strip-comments")) {
-   strip_comments = 1;
-   } else if (!strcmp(argv[1], "-c") ||
-  !strcmp(argv[1], "--comment-lines")) {
-   mode = COMMENT_LINES;
-   } else {
-   mode = INVAL;
-   }
-   } else if (argc > 1) {
-   mode = INVAL;
-   }
+   int comment_mode = 0, count_lines = 0, strip_comments = 0;
+   size_t lines = 0;
+
+   const struct option options[] = {
+   OPT_BOOL('s', "strip-comments", _comments,
+N_("skip and remove all lines starting with comment 
character")),
+   OPT_BOOL('c', "comment-lines", _mode,
+N_("prepend comment character and blank to each 
line")),
+   OPT_BOOL('C', "count-lines", _lines, N_("print line 
count")),
+   OPT_END()
+   };
 
-   if (mode == INVAL)
-   usage(usage_msg);
+   argc = parse_options(argc, argv, prefix, options, usage_msg,
+PARSE_OPT_KEEP_DASHDASH);
 
-   if (strip_comments || mode == COMMENT_LINES)
+   if (comment_mode && (count_lines || strip_comments))
+   usage_with_options(usage_msg, options);
+
+   if (strip_comments || comment_mode)
git_config(git_default_config, NULL);
 
if (strbuf_read(, 0, 1024) < 0)
die_errno("could not read the input");
 
-   if (mode == STRIP_SPACE)
-   strbuf_stripspace(, strip_comments);
+   if (!comment_mode)
+   lines = strbuf_stripspace(, strip_comments);
else
comment_lines();
 
-   write_or_die(1, buf.buf, buf.len);
+   if (!count_lines)
+   write_or_die(1, buf.buf, buf.len);
+   else {
+   struct strbuf tmp = STRBUF_INIT;
+   strbuf_addf(, "%zu\n", lines);
+   

[PATCH 3/3] git rebase -i: Use newly added --count-lines option for stripspace

[Tobias Klauser]

Use the newly added --count-lines option for 'git stripspace' to count
lines instead of piping the entire output to 'wc -l'.

Signed-off-by: Tobias Klauser 
---
 git-rebase--interactive.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index f01637b..c40ca90 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -120,9 +120,9 @@ mark_action_done () {
sed -e 1q < "$todo" >> "$done"
sed -e 1d < "$todo" >> "$todo".new
mv -f "$todo".new "$todo"
-   new_count=$(git stripspace --strip-comments <"$done" | wc -l)
+   new_count=$(git stripspace --strip-comments --count-lines <"$done")
echo $new_count >"$msgnum"
-   total=$(($new_count + $(git stripspace --strip-comments <"$todo" | wc 
-l)))
+   total=$(($new_count + $(git stripspace --strip-comments --count-lines 
<"$todo")))
echo $total >"$end"
if test "$last_count" != "$new_count"
then
@@ -1247,7 +1247,7 @@ test -s "$todo" || echo noop >> "$todo"
 test -n "$autosquash" && rearrange_squash "$todo"
 test -n "$cmd" && add_exec_commands "$todo"
 
-todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
+todocount=$(git stripspace --strip-comments --count-lines <"$todo")
 todocount=${todocount##* }
 
 cat >>"$todo" 

about rev-parse's quietness

[stefan.naewe]

Hello there.

I'm using a bash function that does a combination of 'ls -l', 'git status', and 
'git branch'
to give me a quick overview of where I stand in the current git repo.

I planned to extend that function to also list the to-be-pushed commits in the
current branch, something like 

$ git log --oneline --abbrev-commit --decorate @{u}..

But I don't want to run that 'git log...' if there's no upstream configured
for HEAD (and I don't want to see any error!)

OK. That's a task for 'git rev-parse' I thought:

$ up=$(git rev-parse --verify --quiet @{u}) &&
  git log --left-right --oneline --abbrev-commit --decorate $up
fatal: no upstream configured for branch 'master'

Wait. What?

I thought rev-parse with '--verify' and '--quiet' is just that - quiet -
in case "...the first argument is not a valid object name" ?

Let's see:
$ git rev-parse --verify --quiet master@{xyz} || echo No
No

$ git rev-parse --verify --quiet master@{u} || echo No
fatal: no upstream configured for branch 'master'
No

I don't want to see that error (and I know I could just redirect stdout/stderr 
to /dev/null ...)

So: Are my expectations about 'rev-parse --verify --quiet' wrong or am I doing
something stupid ?

Thanks,
  Stefan
-- 

/dev/random says: Disclaimer: Written by a highly caffeinated mammal.
python -c "print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF
N�r��yb�X��ǧv�^�)޺{.n�+ا���ܨ}���Ơz�:+v���zZ+��+zf���h���~i���z��w���?�&�)ߢf

Re: [PATCH v3 1/3] Add Travis CI support

[Johannes Schindelin]

Hi,

On Thu, 15 Oct 2015, Matthieu Moy wrote:

> Lars Schneider  writes:
> 
> > I was reluctant to this because I feared problems. Especially while
> > running tests in parallel.
> 
> Isn't the point of using a CI tool to notice problems? ;-)
> 
> More seriously, running tests in parallel shouldn't be a problem since
> each test runs in its own directory with HOME set to this private
> directory, so two diffent tests should not interfer. If there's an issue
> with parallel tests, we probably prefer discovering them than avoiding
> them.

I vividly remember pulling my hair out over BuildHive (i.e. the Java-based
Jenkins alternative to Travis) failures that turned out to be
resource-related: I simply overloaded the BuildHive node.

So I assumed that the problems Lars referred to would not be problems of
Git, or the test suite, but of overwhelming a free service with an
unreasonable load.

> > make -j2 9min 11sec:
> > https://travis-ci.org/larsxschneider/git/jobs/85478022
> >
> > make 17min 20sec:
> > https://travis-ci.org/larsxschneider/git/jobs/85432398
> 
> Since the tests are essentially IO-bound and not CPU-bound, it may even
> make sense to use -j3 here.

I would like to caution against overloading Travis. They are really nice
to us, we should try to be nice to them, too.

Ciao,
Dscho
--
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] Falis on commit --amend when already pushed

[Matthieu Moy]

Tomohiro Koana  writes:

> Thank you for the reply.

Please, do not top-post on this list. Look at how other people write
emails and try to mimick them.

> I've taken a look at the list for small project ideas and our idea was
> on the list,
> which goes to show that what we are working on is reasonable enough :D

You may also want to look at how Mercurial does this. They have a
complete system to classify commits:
https://www.mercurial-scm.org/wiki/Phases

You're not going to do something as ambitious for a school project, but
it may give you hints on what can be done.

> As for the test, we're not sure how to write a test because it
> involves remote branches, so we'd love to have advice on it.

"remote" does not have to be actually remote as in "on another
computer". It can be another repository on the same machine.

Loot at what other tests are doing in t/ (there's a README file there
too). Start with

  cd t/
  ls t*push*.sh

to get examples.

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

Re: [PATCH v3 1/3] Add Travis CI support

[Jean-Noël Avila]

Le 15/10/2015 00:52, Lars Schneider a écrit :
> On 12 Oct 2015, at 22:20, Matthieu Moy  wrote:
>
>> larsxschnei...@gmail.com writes:
>>
>>> --- /dev/null
>>> +++ b/.travis.yml
>>> @@ -0,0 +1,46 @@
>>> +language: c
>>> +
>>> +os:
>>> +  - linux
>>> +  - osx
>>> +
>>> +compiler:
>>> +  - clang
>>> +  - gcc
>>> +
>>> +before_install:
>>> +  - >
>>> +export GIT_TEST_OPTS=" --quiet";
>>> +case "${TRAVIS_OS_NAME:-linux}" in
>>> +linux)
>>> +  wget -q https://package.perforce.com/perforce.pubkey -O - \
>>> +| sudo apt-key add -
>>> +  echo 'deb http://package.perforce.com/apt/ubuntu precise release' \
>>> +| sudo tee -a /etc/apt/sources.list
>>> +  wget -q https://packagecloud.io/gpg.key -O - | sudo apt-key add -
>>> +  echo 'deb https://packagecloud.io/github/git-lfs/debian/ jessie 
>>> main' \
>>> +| sudo tee -a /etc/apt/sources.list
>>> +  sudo apt-get update -qq
>>> +  sudo apt-get install -y apt-transport-https
>>> +  sudo apt-get install perforce-server git-lfs
>> Sorry if this has been discussed already, but do you really need these
>> "sudo" calls?
>>
>> They trigger builds on the legacy Travis CI infrastructure:
>>
>>  
>> http://docs.travis-ci.com/user/migrating-from-legacy/?utm_source=legacy-notice_medium=banner_campaign=legacy-upgrade
>>
>> No big deal, but getting rid of sudo would be cool, and documenting why
>> it can't easily be done in commit message and/or comments would be nice
>> otherwise.
> I would like to get rid of the "sudo" calls, too. Unfortunately I wasn't able 
> to achieve this so far because these packages are not white listed on Travis 
> CI (see Jean-Noël answer in this thread). I tried to download and install the 
> *.deb files manually using dpkg without luck. Any idea or hint?
> Nevertheless I don't think this is a problem as Travis CI states that "sudo 
> isn't possible (__right now__)" on the new infrastructure. They need to find 
> a solutions because I believe many projects will run into this issue...
> http://docs.travis-ci.com/user/migrating-from-legacy/?utm_source=legacy-notice_medium=banner_campaign=legacy-upgrade#Using-sudo-isn%E2%80%99t-possible-(right-now)
>
> - Lars


There's another hope. The requirements can be installed in the user's
HOME and cached between builds. In this case, no need for sudo.
Obviously, that rules out using apt and dpkg.

Jean-Noël
--
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: How to rebase when some commit hashes are in some commit messages

[Francois-Xavier Le Bail]

On 13/10/2015 19:07, Jacob Keller wrote:

> b) you are rebasing a commit which references another commit in the same 
> rebase
> 
> I see no valid reason to reference a sha1 in this case. If you're
> referencing as a "fixes", then you are being silly since you can just
> squash the fix into the original commit and thus prevent introduction
> of bug at all.

squash need manual process, renaming the sha1 not.
--
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: How to rebase when some commit hashes are in some commit messages

[Johannes Schindelin]

Hi Francois-Xavier,

On Thu, 15 Oct 2015, Francois-Xavier Le Bail wrote:

> On 13/10/2015 15:29, Philip Oakley wrote:
>
> > Thus the only sha1 numbers that could be used are those that are
> > within the (possibly implied) instruction sheet (which will list the
> > current sha1s that will be converted by rebase to new sha1's).
> 
> Yes.

So what happens for commits that are in the pick list but then end up not
being rewritten at all, e.g. when a patch has been applied upstream (and
the --cherry logic did not detect that) and then you end up with a "No
changes to commit"? And what if a patch ends up in merge conflicts and the
user just skips it? And what if the referenced commit is to be picked
*afterwards* due to the commits being reordered?

It would appear that the strategy you propose is still too ill-defined to
make for a robust feature.

Ciao,
Johannes

P.S.: The recommended way to refer to a commit is not only using the SHA-1
but also mentioning the one-line, and even the date. That way, even
rebased commits can found most of the time. This is not fool-proof, by
far, of course, but still better than trying to rewrite a SHA-1 and
failing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] Add Travis CI support

[Matthieu Moy]

Lars Schneider  writes:

> I would like to get rid of the "sudo" calls, too. Unfortunately I
> wasn't able to achieve this so far because these packages are not
> white listed on Travis CI (see Jean-Noël answer in this thread).

I think this would deserve a mention in the commit message, but I won't
insist on that.

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

Re: [PATCH] Falis on commit --amend when already pushed

[Tomohiro Koana]

Thank you for the reply.

I've taken a look at the list for small project ideas and our idea was
on the list,
which goes to show that what we are working on is reasonable enough :D

We'll read the documentations for git contributions thoroughly and
send a patch again.

As for the test, we're not sure how to write a test because it
involves remote branches, so we'd love to have advice on it.

On Thu, Oct 15, 2015 at 3:44 PM, Matthieu Moy
 wrote:
> Tomohiro Koana  writes:
>
>> Hello all,
>>
>> I'm a third year student at the University of Tokyo and, in our
>> "Diving into open-source software" class, my friends and I decided to
>> work with git. Our final, hopefully, is contributing to git.
>
> Welcome on board :-). I give the same class to my students (in Ensimag,
> Grenoble, France). You can have a look at
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas for a list of
> ideas of things you can do.
>
> The first contact with an open-source community is usually hard (the
> quality expectation is much higher that your usual lab works), but you
> are going to learn a lot!
>
>> One improvement that we thought of was not letting users to amend
>> commit when the commit is already pushed to the remote server.
>
> This is a good introduction, but not a good commit message. The commit
> message is not about what you "thought", but about what the commit is
> doing, and more importantly _why_ it is doing that and doing it this
> way. See it as an argument like "You should accept this patch
> because " (even if you won't actually write it like this). Read some
> existing messages ("git log --no-merges") to see what I mean.
>
> Please, read Documentation/SubmittingPatches and
> Documentation/CodingGuidelines in Git's source tree.
>
>> --- a/builtin/commit.c
>>
>> +++ b/builtin/commit.c
>>
>> @@ -32,6 +32,7 @@
>>
>>  #include "sequencer.h"
>>
>>  #include "notes-utils.h"
>>
>>  #include "mailmap.h"
>>
>> +#include "remote.h"
>
> The patch is whitespace-damaged (there are extra blank lines). Use
> either "git send-email" or http://submitgit.herokuapp.com/ to submit
> your patches.
>
>> + stat_tracking_info(branch, , , _base);
>>
>> + if (amend && ours == 0) {
>>
>> + die(_("This commit is already pushed to the remote -- cannot amend."));
>>
>> + }
>
> I don't know the API well enough so I can't say whether this correctly
> detects already pushed branch, but this looks suspiciously simple. Are
> you not just detecting the presence of a remote-tracking branch? What
> you should do is to detect whether the remote-tracking branch contains
> the current commit.
>
> Also, this is clearly not acceptable in its current form: there are many
> valid use-cases to amend an already-pushed commit, so you can't break
> the flow of people using this. It must 1) be configurable, and 2) unless
> you have a really good reason, backward-compatible by default.
>
> Also, it lacks tests.
>
> Actually, the idea you have is far, far more difficult than what you
> probably thought.
>
> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Falis on commit --amend when already pushed

[Matthieu Moy]

Tomohiro Koana  writes:

> Hello all,
>
> I'm a third year student at the University of Tokyo and, in our
> "Diving into open-source software" class, my friends and I decided to
> work with git. Our final, hopefully, is contributing to git.

Welcome on board :-). I give the same class to my students (in Ensimag,
Grenoble, France). You can have a look at
https://git.wiki.kernel.org/index.php/SmallProjectsIdeas for a list of
ideas of things you can do.

The first contact with an open-source community is usually hard (the
quality expectation is much higher that your usual lab works), but you
are going to learn a lot!

> One improvement that we thought of was not letting users to amend
> commit when the commit is already pushed to the remote server.

This is a good introduction, but not a good commit message. The commit
message is not about what you "thought", but about what the commit is
doing, and more importantly _why_ it is doing that and doing it this
way. See it as an argument like "You should accept this patch
because " (even if you won't actually write it like this). Read some
existing messages ("git log --no-merges") to see what I mean.

Please, read Documentation/SubmittingPatches and
Documentation/CodingGuidelines in Git's source tree.

> --- a/builtin/commit.c
>
> +++ b/builtin/commit.c
>
> @@ -32,6 +32,7 @@
>
>  #include "sequencer.h"
>
>  #include "notes-utils.h"
>
>  #include "mailmap.h"
>
> +#include "remote.h"

The patch is whitespace-damaged (there are extra blank lines). Use
either "git send-email" or http://submitgit.herokuapp.com/ to submit
your patches.

> + stat_tracking_info(branch, , , _base);
>
> + if (amend && ours == 0) {
>
> + die(_("This commit is already pushed to the remote -- cannot amend."));
>
> + }

I don't know the API well enough so I can't say whether this correctly
detects already pushed branch, but this looks suspiciously simple. Are
you not just detecting the presence of a remote-tracking branch? What
you should do is to detect whether the remote-tracking branch contains
the current commit.

Also, this is clearly not acceptable in its current form: there are many
valid use-cases to amend an already-pushed commit, so you can't break
the flow of people using this. It must 1) be configurable, and 2) unless
you have a really good reason, backward-compatible by default.

Also, it lacks tests.

Actually, the idea you have is far, far more difficult than what you
probably thought.

-- 
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: How to rebase when some commit hashes are in some commit messages

[Francois-Xavier Le Bail]

On 13/10/2015 15:29, Philip Oakley wrote:
> From: "Konstantin Khomoutov" 
>> On Tue, 13 Oct 2015 10:50:40 +0200
>> Francois-Xavier Le Bail  wrote:
>>
>>> >> For example, if I rebase the following commits, I would want that
>>> >> if the commit hash 222... become 777...,
>>> >> the message
>>> >> "Update test output for "
>>> >> become
>>> >> "Update test output for 777..."
>>> >>
>>> >> Is it possible currently? And if yes how?
>>> >
>>> > AFAIK, it's not possible other than by editing the message by hand.
>>>
>>> It seems to me useful to be able to do it. Can we hope a new option?
>>
>> How do you think this could be practically implemented?
>>
>> A couple of things which immediately spring to my mind:
>>
>> To begin with, you are free to specify just a few first characters of
>> the commit name you're referring to.  So the alogrythm which finds the
>> relevant commits them has to be smart to somehow avoid misfires.  Or
>> have knobs to tune it (like -M of `git log`).
>>
>> OK, suppose that this is solved through the usage of some agreed-upon
>> keywords in the commit message.  Say, you adopt a policy to put
>> something like
>>
>>  X-Refers-To: 2dd8a9d9bb33ebffccb2ff516497adc8535bcab4
>>
>> in your commit message to make the finder tool happy.
>>
>> Now think how exactly it should work.  First, any commit at all might
>> mention the name of the target commit in its commit message.  Okay,
>> let's suppose there will be some way to somehow prune the possible DAG
>> down.  Then what happens if the commit to change is a part of the chain
>> of commits reachable from some branch other than that you're rebasing?
>> Automatically rebasing it would rewrite that commits and all commits
>> "after" it -- possibly resulting in what the "Recovering from upstream
>> rebase" part of the git-rebase(1) manual page deals with.
>>
>> Having said that, the feature you're after appears to me to be a
>> sensible thing to have but the possibility of its generic implementation
>> appears to be moot.
>>
>> Note that to deal with narrow simple cases (all possibly affected
>> commits leave on the same branch you're rebasing, and come later than
>> the rebase's anchor) you could write a script which uses `git log` to
>> find those commits which need special care.
> 
> My tuppence is that the only sha1's that could/would be rewritten would be 
> those for the commits within the rebase. During rebasing it is expected that 
> the user is re-adjusting things for later
> upstream consumption, with social controls and understandings with colleagues.
> 
> Thus the only sha1 numbers that could be used are those that are within the 
> (possibly implied) instruction sheet (which will list the current sha1s that 
> will be converted by rebase to new sha1's).

Yes.

> It should be clear that the sha1's are always backward references (because of 
> the impossibility of including a forward reference to an as yet un-created 
> future commit's sha1).
> 
> The key question (for me) is whether short sha1s are accepted, or if they 
> must be full 40 char sha1's (perhaps an option). There are already options 
> for making sure that short refs are not ambiguous.

I think full 40 sha1 is more secure to avoid confusion with previous or future 
sha1.

> It sound to me like a sensible small project for those that have such a 
> workflow. I'm not sure if it should work with a patch based flow when 
> submitting upstream - I'm a little fuzzy on how would the
> upstream maintainer know which sha1 referred to which patch.
> 
> 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

[BUG] t7063-status-untracked-cache flaky?

[Lars Schneider]

Hi,

I noticed that "t7063-status-untracked-cache.sh" occasionally fails with "not 
ok 24 - test sparse status with untracked cache".

E.g. on OS X compiled with gcc:
https://travis-ci.org/larsxschneider/git/jobs/85432514

E.g. on Linux compiled with gcc:
https://travis-ci.org/larsxschneider/git/jobs/84986975

The test was added with commit 7687252. I have not really investigated the 
problem yet but the "avoid_racy" method caught my attention. Is this test known 
to be flaky? Would an increased sleep time in "avoid_racy" help?

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

Re: [PATCH v3 1/3] Add Travis CI support

[Lars Schneider]

On 13 Oct 2015, at 12:32, Jean-Noël Avila  wrote:

> Le 11/10/2015 19:55, larsxschnei...@gmail.com a écrit :
>> +
>> +before_script: make
>> +
>> +script: make --quiet test
> 
> Travis can be used in container mode but that would need getting rid of
> "sudo" command and only installing from white-listed sources
> (https://github.com/travis-ci/apt-source-whitelist/blob/master/ubuntu.json)
> 
> Anyway, even within the present VM mode, 1.5 cores are available, so it
> makes sense to add "-j2" to every make commands.
> 
I was reluctant to this because I feared problems. Especially while running 
tests in parallel. However, the result looks quite good.

make -j2 9min 11sec:
https://travis-ci.org/larsxschneider/git/jobs/85478022

make 17min 20sec:
https://travis-ci.org/larsxschneider/git/jobs/85432398

If there is no argument against running test in parallel then I will add it to 
the next roll.

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

Re: [PATCH v3 1/3] Add Travis CI support

[Matthieu Moy]

Lars Schneider  writes:

> I was reluctant to this because I feared problems. Especially while
> running tests in parallel.

Isn't the point of using a CI tool to notice problems? ;-)

More seriously, running tests in parallel shouldn't be a problem since
each test runs in its own directory with HOME set to this private
directory, so two diffent tests should not interfer. If there's an issue
with parallel tests, we probably prefer discovering them than avoiding
them.

> make -j2 9min 11sec:
> https://travis-ci.org/larsxschneider/git/jobs/85478022
>
> make 17min 20sec:
> https://travis-ci.org/larsxschneider/git/jobs/85432398

Since the tests are essentially IO-bound and not CPU-bound, it may even
make sense to use -j3 here.

-- 
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: How to rebase when some commit hashes are in some commit messages

[Francois-Xavier Le Bail]

On 13/10/2015 20:00, Mike Rappazzo wrote:
> It seems reasonable that this could be added as a feature of
> interactive rebase.  The todo list could be automatically adjusted to
> "reword" for those commits which are referring to other commits within
> the same rebase.  As each commit is re-written, a mapping could be
> kept of old sha1 -> new sha1.  Then when one of the reworded commits
> is being applied, the old sha1 -> new sha1 mapping could be used to
> add a line to the $COMMIT_MSG.

Even for non-interactive rebase, it could work.
--
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: Alternative to manual editing with git add --patch

[Johannes Schindelin]

Hi Sven,

On Thu, 15 Oct 2015, Sven Helmberger wrote:

> Am 15.10.2015 um 12:11 schrieb Johannes Schindelin:
> > 
> > This is all technically sound. From a usability perspective I would wish
> > more for a way to exclude or filter the lines by a pattern. 
> 
> Why not do both?

Because sometimes more is less. If users are overwhelmed with many, many
options, they are *less* likely to benefit from the few that are easy to
use because they won't find out about them.

Ciao,
Johannes
--
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: Alternative to manual editing with git add --patch

[Sven Helmberger]

Am 15.10.2015 um 17:06 schrieb Johannes Schindelin:

> 
> Because sometimes more is less. If users are overwhelmed with many, many
> options, they are *less* likely to benefit from the few that are easy to
> use because they won't find out about them.
> 

Going from "I want to split at 'x'" to doing so seems just fine. More
than "Ok.. let's find the match I want to search for to have the split I
want", which seems like the opposite of usable.

There are 13 options now not counting help. Not sure it matters much if
we increase that to 14 or 15.

Regards,
Sven

--
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: Alternative to manual editing with git add --patch

[Sven Helmberger]

Am 15.10.2015 um 12:11 schrieb Johannes Schindelin:
> 
> This is all technically sound. From a usability perspective I would wish
> more for a way to exclude or filter the lines by a pattern. 

Why not do both?

The only thing that is unfortunate is that the "/" already is a command.

Which would point to either a solution along the lines of "s"
being the split-by-line and "s/" being the split-by-regex?

Or is this an argument for introducing yet another interaction mode
entered when "s" fails to split further -- with simple "/" and ""
being the options in that mode.

Regards,
Sven Helmberger


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

Re: [PATCH] Use the alternates of the source repository for dissociating clone

[Johannes Schindelin]

Hi Alex,

On Thu, 15 Oct 2015, Alexander Riesen wrote:

> The "--dissociate" option required reference repositories, which sometimes
> demanded a look into the objects/info/alternates by the user. As this
> is something which can be figured out automatically, do it in the
> clone unless there is no other reference repositories.

Would it not make sense to reuse the copy_alternates() function to simply
copy the alternates and let `--dissociate` run its course with the copied
.objects/info/alternate file? That would make for less new code...

Ciao,
Dscho
--
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] Allow "clone --dissociate" to dissociate from alternates

[Alexander Riesen]

The option requiring the explicit reference repositories is a bit of overkill:
the alternates in the original repository *are* reference repositories and
would be dissociated from should one pass any reference repository (even an
unrelated one).

Signed-off-by: Alex Riesen 
---

On 10/15/2015 04:11 PM, Johannes Schindelin wrote:

On Thu, 15 Oct 2015, Alexander  Riesen wrote:

>
>> The "--dissociate" option required reference repositories, which sometimes
>> demanded a look into the objects/info/alternates by the user. As this
>> is something which can be figured out automatically, do it in the
>> clone unless there is no other reference repositories.
>
> Would it not make sense to reuse the copy_alternates() function to simply
> copy the alternates and let `--dissociate` run its course with the copied
> .objects/info/alternate file? That would make for less new code...

IIUC, I should validate the alternates in the source repository...
But, the only thing the user looses if it is not validated, is the nice
warning regarding no reference repositories to dissociate from, right?

So maybe we can just remove the reset of option_dissociate and be done with
it? I would actually suggest removing the warning as well: the alternates are
something to dissociate from. And I see no harm otherwise.

How about this instead?

 builtin/clone.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 578da85..b33d6f9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -947,10 +947,6 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)

 if (option_reference.nr)
 setup_reference();
-else if (option_dissociate) {
-warning(_("--dissociate given, but there is no --reference"));
-option_dissociate = 0;
-}

 fetch_pattern = value.buf;
 refspec = parse_fetch_refspec(1, _pattern);
--
2.6.1.151.g74e8091


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

Re: [PATCH v3] merge: fix cache_entry use-after-free

[René Scharfe]

Am 15.10.2015 um 21:02 schrieb David Turner:

On Thu, 2015-10-15 at 05:35 +0200, René Scharfe wrote:

Am 15.10.2015 um 00:07 schrieb David Turner:

From: Keith McGuigan 

During merges, we would previously free entries that we no longer need
in the destination index.  But those entries might also be stored in
the dir_entry cache, and when a later call to add_to_index found them,
they would be used after being freed.

To prevent this, add a ref count for struct cache_entry.  Whenever
a cache entry is added to a data structure, the ref count is incremented;
when it is removed from the data structure, it is decremented.  When
it hits zero, the cache_entry is freed.

Signed-off-by: Keith McGuigan 
Signed-off-by: David Turner 
---

Fix type of ref_count (from unsigned int to int).


   cache.h| 27 +++
   name-hash.c|  7 ++-
   read-cache.c   |  6 +-
   split-index.c  | 13 -
   unpack-trees.c |  6 --
   5 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/cache.h b/cache.h
index 752031e..7906026 100644
--- a/cache.h
+++ b/cache.h
@@ -149,6 +149,7 @@ struct stat_data {

   struct cache_entry {
struct hashmap_entry ent;
+   int ref_count; /* count the number of refs to this in dir_hash */
struct stat_data ce_stat_data;
unsigned int ce_mode;
unsigned int ce_flags;
@@ -213,6 +214,32 @@ struct cache_entry {
   struct pathspec;

   /*
+ * Increment the cache_entry reference count.  Should be called
+ * whenever a pointer to a cache_entry is retained in a data structure,
+ * thus marking it as alive.
+ */
+static inline void add_ce_ref(struct cache_entry *ce)
+{
+   assert(ce != NULL && ce->ref_count >= 0);
+   ce->ref_count++;
+}
+
+/*
+ * Decrement the cache_entry reference count.  Should be called whenever
+ * a pointer to a cache_entry is dropped.  Once the counter drops to 0
+ * the cache_entry memory will be safely freed.
+ */
+static inline void drop_ce_ref(struct cache_entry *ce)
+{
+   if (ce != NULL) {
+   assert(ce->ref_count >= 0);


Shouldn't this be "> 0" to prevent double frees?


No.  If the ref_count is 1, then there is still some reference to the
ce.  If it is 0, there is no reference to it, and the next check (< 1)
will succeed and the ce will get freed.


+   if (--ce->ref_count < 1) {
+   free(ce);
+   }
+   }
+}


OK, let me think out loud, step by step:

Given ref_count == 1 then the assert passes, ref_count gets decremented 
to 0, which is less than 1, so ce is freed.


Given ref_count == 0 then the assert passes, refcount gets decremented 
to -1, which is less than 1, so ce is freed again.


Where did I go wrong?

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

Make "git checkout" automatically update submodules?

[Kannan Goundan]

Git submodules seem to be a great fit for one of our repos.  The biggest
pain point is that it's too easy to forget to update submodules.

1. I often forget since most repos don't need it.
2. Infrequent users of our repo almost never know to update submodules and
end up coming to us with strange build errors.
3. Existing scripts that work with Git repos are usually not built to handle
submodules.

In the common case of the submodule content having no local edits, it would
be nice if "git checkout" automatically updated submodules [1].  If there
are local edits, it could error out (maybe override with
"--ignore-modified-submodules" or something).

I'm not a Git expert, though.  Is there a reason something like this isn't
already implemented?  Maybe there's an existing write-up or mailing list
thread I can read to get some background information?

Thanks!

[1] Our post-checkout procedure is:

git submodule sync
git submodule update --init
git submodule foreach --recursive \
  'git submodule sync ; git submodule update --init'

(Not sure if this is correct.  Different articles/blogs suggest a slightly
different set of commands.)

--
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: Make "git checkout" automatically update submodules?

[Junio C Hamano]

Kannan Goundan  writes:

> Git submodules seem to be a great fit for one of our repos.  The biggest
> pain point is that it's too easy to forget to update submodules.
> ...
> In the common case of the submodule content having no local edits, it would
> be nice if "git checkout" automatically updated submodules [1].  If there
> are local edits, it could error out (maybe override with
> "--ignore-modified-submodules" or something).
>
> I'm not a Git expert, though.  Is there a reason something like this isn't
> already implemented?  Maybe there's an existing write-up or mailing list
> thread I can read to get some background information?

I think it is mostly because the area has a lot of corner cases and
different workflows.  For example ...

> [1] Our post-checkout procedure is:
>
> git submodule sync
> git submodule update --init
> git submodule foreach --recursive \
>   'git submodule sync ; git submodule update --init'

... this will clearly not work well for everybody, as you do not
want to instantiate _all_ the submodules _unconditionally_.  Well,
"you" (Kannan) and your project may want to, but not necessarily
other large projects (e.g. imagine Android using submodules).

So you can view the current status being "nothing is instantiated by
default", which _is_ far from satisfactory than the ideal case where
perhaps the project can specify in .gitmodules which submodules are
to be instantiated by default, add labels to modules in .gitmodules
so that "git clone" of a top-level project with submodules can be
told "git clone --init-submodules=" to instantiate the modules
that match the given label after the top-level is cloned, etc. etc.

But such a way to allow these more complicated situations to be
handled nicely has not been designed by anybody, so for now "nothing
instantiated by default" stands as the safest default.
--
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 04/12] git submodule update: Announce outcome of submodule operation to stderr

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 git-submodule.sh|  2 +-
 t/t7406-submodule-update.sh | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 56a0524..bb8b2c7 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -780,7 +780,7 @@ Maybe you want to use 'update --init'?")"
 
if (clear_local_git_env; cd "$sm_path" && $command 
"$sha1")
then
-   say "$say_msg"
+   say >&2 "$say_msg"
elif test -n "$must_die_on_failure"
then
die_with_status 2 "$die_msg"
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dda3929..f65b81c 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -111,8 +111,8 @@ test_expect_success 'submodule update does not fetch 
already present commits' '
(cd super &&
  git submodule update > ../actual 2> ../actual.err
) &&
-   test_i18ncmp expected actual &&
-   ! test -s actual.err
+   test_i18ncmp expected actual.err &&
+   ! test -s actual
 '
 
 test_expect_success 'submodule update should fail due to local changes' '
@@ -702,8 +702,8 @@ test_expect_success 'submodule update places git-dir in 
superprojects git-dir re
rm -rf super_update_r2 &&
git clone super_update_r super_update_r2 &&
(cd super_update_r2 &&
-git submodule update --init --recursive >actual &&
-test_i18ngrep "Submodule path .submodule/subsubmodule.: checked out" 
actual &&
+git submodule update --init --recursive 2>actual.err &&
+test_i18ngrep "Submodule path .submodule/subsubmodule.: checked out" 
actual.err &&
 (cd submodule/subsubmodule &&
  git log > ../../expected
 ) &&
@@ -770,8 +770,8 @@ test_expect_success 'submodule update --recursive drops 
module name before recur
 (cd deeper/submodule/subsubmodule &&
  git checkout HEAD^
 ) &&
-git submodule update --recursive deeper/submodule >actual &&
-test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked 
out" actual
+git submodule update --recursive deeper/submodule 2>actual.err &&
+test_i18ngrep "Submodule path .deeper/submodule/subsubmodule.: checked 
out" actual.err
)
 '
 test_done
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 06/12] git submodule update: Handle unmerged submodules in C

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 15 +++
 git-submodule.sh|  6 +-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 47dc9cb..f81f37a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -284,11 +284,18 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
for (i = 0; i < list.nr; i++) {
const struct cache_entry *ce = list.entries[i];
 
-   if (ce_stage(ce))
-   printf("%06o %s U\t", ce->ce_mode, 
sha1_to_hex(null_sha1));
-   else
-   printf("%06o %s %d\t", ce->ce_mode, 
sha1_to_hex(ce->sha1), ce_stage(ce));
+   char *env_prefix = getenv("prefix");
+   if (ce_stage(ce)) {
+   if (env_prefix)
+   fprintf(stderr, "Skipping unmerged submodule 
%s/%s",
+   env_prefix, ce->name);
+   else
+   fprintf(stderr, "Skipping unmerged submodule 
%s",
+   ce->name);
+   continue;
+   }
 
+   printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce));
utf8_fprintf(stdout, "%s\n", ce->name);
}
return 0;
diff --git a/git-submodule.sh b/git-submodule.sh
index d2d80e2..0754ecd 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -661,11 +661,7 @@ cmd_update()
while read mode sha1 stage sm_path
do
die_if_unmatched "$mode"
-   if test "$stage" = U
-   then
-   echo >&2 "Skipping unmerged submodule $prefix$sm_path"
-   continue
-   fi
+
name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
if ! test -z "$update"
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 08/12] git submodule update: check for "none" in C

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 38 --
 git-submodule.sh|  8 +---
 2 files changed, 37 insertions(+), 9 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f81f37a..73954ac 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,9 +255,15 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int git_submodule_config(const char *var, const char *value, void *cb)
+{
+   return parse_submodule_config_option(var, value);
+}
+
 static int module_list_or_clone(int argc, const char **argv, const char 
*prefix)
 {
int i;
+   char *update = NULL;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
 
@@ -265,6 +271,9 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
OPT_STRING(0, "prefix", ,
   N_("path"),
   N_("alternative anchor for relative paths")),
+   OPT_STRING(0, "update", ,
+  N_("string"),
+  N_("update command for submodules")),
OPT_END()
};
 
@@ -281,20 +290,45 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
return 1;
}
 
+   gitmodules_config();
+   /* Overlay the parsed .gitmodules file with .git/config */
+   git_config(git_submodule_config, NULL);
+
for (i = 0; i < list.nr; i++) {
+   const struct submodule *sub = NULL;
+   const char *displaypath = NULL;
const struct cache_entry *ce = list.entries[i];
+   struct strbuf sb = STRBUF_INIT;
+   const char *update_module = NULL;
 
char *env_prefix = getenv("prefix");
if (ce_stage(ce)) {
if (env_prefix)
-   fprintf(stderr, "Skipping unmerged submodule 
%s/%s",
+   fprintf(stderr, "Skipping unmerged submodule 
%s/%s\n",
env_prefix, ce->name);
else
-   fprintf(stderr, "Skipping unmerged submodule 
%s",
+   fprintf(stderr, "Skipping unmerged submodule 
%s\n",
ce->name);
continue;
}
 
+   sub = submodule_from_path(null_sha1, ce->name);
+   if (env_prefix)
+   displaypath = relative_path(env_prefix, ce->name, );
+   else
+   displaypath = ce->name;
+
+   if (update)
+   update_module = update;
+   if (!update_module)
+   update_module = sub->update;
+   if (!update_module)
+   update_module = "checkout";
+   if (!strcmp(update_module, "none")) {
+   fprintf(stderr, "Skipping submodule '%s'\n", 
displaypath);
+   continue;
+   }
+
printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce));
utf8_fprintf(stdout, "%s\n", ce->name);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index 0754ecd..227fed6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -656,7 +656,7 @@ cmd_update()
fi
 
cloned_modules=
-   git submodule--helper list-or-clone --prefix "$wt_prefix" "$@" | {
+   git submodule--helper list-or-clone --prefix "$wt_prefix" 
${update:+--update "$update"} "$@" | {
err=
while read mode sha1 stage sm_path
do
@@ -677,12 +677,6 @@ cmd_update()
 
displaypath=$(relative_path "$prefix$sm_path")
 
-   if test "$update_module" = "none"
-   then
-   echo >&2 "Skipping submodule '$displaypath'"
-   continue
-   fi
-
if test -z "$url"
then
# Only mention uninitialized submodules when its
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 05/12] git submodule update: Use its own list implementation.

[Stefan Beller]

Discussions turned out that we cannot parallelize the whole loop below
`git submodule--helper list` in `git submodule update`, because some
changes should be done only one at a time, such as messing up a submodule
and leave it up to the user to cleanup the conflicted rebase or merge.

The submodules which are need to be cloned however do not expect to create
problems which require attention by the user one at a time, so we want to
parallelize that first.

To do so we will start with a literal copy of `git submodule--helper list`
and port over features gradually.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 40 
 git-submodule.sh|  2 +-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..47dc9cb 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -255,6 +255,45 @@ static int module_clone(int argc, const char **argv, const 
char *prefix)
return 0;
 }
 
+static int module_list_or_clone(int argc, const char **argv, const char 
*prefix)
+{
+   int i;
+   struct pathspec pathspec;
+   struct module_list list = MODULE_LIST_INIT;
+
+   struct option module_list_options[] = {
+   OPT_STRING(0, "prefix", ,
+  N_("path"),
+  N_("alternative anchor for relative paths")),
+   OPT_END()
+   };
+
+   const char *const git_submodule_helper_usage[] = {
+   N_("git submodule--helper list [--prefix=] [...]"),
+   NULL
+   };
+
+   argc = parse_options(argc, argv, prefix, module_list_options,
+git_submodule_helper_usage, 0);
+
+   if (module_list_compute(argc, argv, prefix, , ) < 0) {
+   printf("#unmatched\n");
+   return 1;
+   }
+
+   for (i = 0; i < list.nr; i++) {
+   const struct cache_entry *ce = list.entries[i];
+
+   if (ce_stage(ce))
+   printf("%06o %s U\t", ce->ce_mode, 
sha1_to_hex(null_sha1));
+   else
+   printf("%06o %s %d\t", ce->ce_mode, 
sha1_to_hex(ce->sha1), ce_stage(ce));
+
+   utf8_fprintf(stdout, "%s\n", ce->name);
+   }
+   return 0;
+}
+
 struct cmd_struct {
const char *cmd;
int (*fn)(int, const char **, const char *);
@@ -264,6 +303,7 @@ static struct cmd_struct commands[] = {
{"list", module_list},
{"name", module_name},
{"clone", module_clone},
+   {"list-or-clone", module_list_or_clone}
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
diff --git a/git-submodule.sh b/git-submodule.sh
index bb8b2c7..d2d80e2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -656,7 +656,7 @@ cmd_update()
fi
 
cloned_modules=
-   git submodule--helper list --prefix "$wt_prefix" "$@" | {
+   git submodule--helper list-or-clone --prefix "$wt_prefix" "$@" | {
err=
while read mode sha1 stage sm_path
do
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 09/12] git submodule update: Check url in C

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 21 +
 git-submodule.sh| 10 --
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 73954ac..7a2fd4e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -300,6 +300,7 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
const struct cache_entry *ce = list.entries[i];
struct strbuf sb = STRBUF_INIT;
const char *update_module = NULL;
+   const char *url = NULL;
 
char *env_prefix = getenv("prefix");
if (ce_stage(ce)) {
@@ -329,6 +330,26 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
continue;
}
 
+   /*
+* Looking up the url in .git/config.
+* We cannot fall back to .gitmodules as we only want to process
+* configured submodules. This renders the submodule lookup API
+* useless, as it cannot lookup without fallback.
+*/
+   strbuf_reset();
+   strbuf_addf(, "submodule.%s.url", sub->name);
+   git_config_get_string_const(sb.buf, );
+   if (!url) {
+   /*
+* Only mention uninitialized submodules when its
+* path have been specified
+*/
+   if (pathspec.nr)
+   fprintf(stderr, _("Submodule path '%s' not 
initialized\n"
+   "Maybe you want to use 'update 
--init'?"), displaypath);
+   continue;
+   }
+
printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce));
utf8_fprintf(stdout, "%s\n", ce->name);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index 227fed6..80f41b2 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -677,16 +677,6 @@ cmd_update()
 
displaypath=$(relative_path "$prefix$sm_path")
 
-   if test -z "$url"
-   then
-   # Only mention uninitialized submodules when its
-   # path have been specified
-   test "$#" != "0" &&
-   say >&2 "$(eval_gettext "Submodule path '\$displaypath' 
not initialized
-Maybe you want to use 'update --init'?")"
-   continue
-   fi
-
if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
then
git submodule--helper clone ${GIT_QUIET:+--quiet} 
--prefix "$prefix" --path "$sm_path" --name "$name" --url "$url" "$reference" 
"$depth" || exit
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 11/12] submodule--helper: Do not emit submodules to process directly.

[Stefan Beller]

This will allow us to refactor the loop to use the parallel process
API.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d1684cf..fa8c008 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -297,6 +297,8 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
char *update = NULL;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
+   struct string_list projectlines = STRING_LIST_INIT_DUP;
+   struct string_list_item *item;
 
struct option module_list_options[] = {
OPT_STRING(0, "prefix", ,
@@ -403,9 +405,15 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
return 1;
}
}
+   strbuf_reset();
+   strbuf_addf(, "%06o %s %d %d\t%s\n", ce->ce_mode,
+   sha1_to_hex(ce->sha1), ce_stage(ce),
+   just_cloned, ce->name);
+   string_list_append(, sb.buf);
+   }
 
-   printf("%06o %s %d %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce), just_cloned);
-   utf8_fprintf(stdout, "%s\n", ce->name);
+   for_each_string_list_item(item, ) {
+   utf8_fprintf(stdout, "%s", item->string);
}
return 0;
 }
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 02/12] git submodule update: Announce uninitialized modules on stderr

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 git-submodule.sh   |  2 +-
 t/t7400-submodule-basic.sh | 12 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 578ec48..eea27f8 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -693,7 +693,7 @@ cmd_update()
# Only mention uninitialized submodules when its
# path have been specified
test "$#" != "0" &&
-   say "$(eval_gettext "Submodule path '\$displaypath' not 
initialized
+   say >&2 "$(eval_gettext "Submodule path '\$displaypath' 
not initialized
 Maybe you want to use 'update --init'?")"
continue
fi
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 540771c..32a89b8 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -462,9 +462,9 @@ test_expect_success 'update --init' '
git config --remove-section submodule.example &&
test_must_fail git config submodule.example.url &&
 
-   git submodule update init > update.out &&
-   cat update.out &&
-   test_i18ngrep "not initialized" update.out &&
+   git submodule update init 2> update.err &&
+   cat update.err &&
+   test_i18ngrep "not initialized" update.err &&
test_must_fail git rev-parse --resolve-git-dir init/.git &&
 
git submodule update --init init &&
@@ -480,9 +480,9 @@ test_expect_success 'update --init from subdirectory' '
mkdir -p sub &&
(
cd sub &&
-   git submodule update ../init >update.out &&
-   cat update.out &&
-   test_i18ngrep "not initialized" update.out &&
+   git submodule update ../init 2>update.err &&
+   cat update.err &&
+   test_i18ngrep "not initialized" update.err &&
test_must_fail git rev-parse --resolve-git-dir ../init/.git &&
 
git submodule update --init ../init
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 01/12] git submodule update: Announce skipping submodules on stderr

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 git-submodule.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 8b0eb9a..578ec48 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -684,7 +684,7 @@ cmd_update()
 
if test "$update_module" = "none"
then
-   echo "Skipping submodule '$displaypath'"
+   echo >&2 "Skipping submodule '$displaypath'"
continue
fi
 
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 07/12] submodule config: keep update strategy around

[Stefan Beller]

We need the submodule update strategies in a later patch.

Signed-off-by: Stefan Beller 
---
 submodule-config.c | 11 +++
 submodule-config.h |  1 +
 2 files changed, 12 insertions(+)

diff --git a/submodule-config.c b/submodule-config.c
index 393de53..175bcbb 100644
--- a/submodule-config.c
+++ b/submodule-config.c
@@ -194,6 +194,7 @@ static struct submodule *lookup_or_create_by_name(struct 
submodule_cache *cache,
 
submodule->path = NULL;
submodule->url = NULL;
+   submodule->update = NULL;
submodule->fetch_recurse = RECURSE_SUBMODULES_NONE;
submodule->ignore = NULL;
 
@@ -326,6 +327,16 @@ static int parse_config(const char *var, const char 
*value, void *data)
free((void *) submodule->url);
strbuf_addstr(, value);
submodule->url = strbuf_detach(, NULL);
+   } else if (!strcmp(item.buf, "update")) {
+   if (!value)
+   ret = config_error_nonbool(var);
+   else if (!me->overwrite && submodule->update != NULL)
+   warn_multiple_config(me->commit_sha1, submodule->name,
+"update");
+   else {
+   free((void *)submodule->update);
+   submodule->update = xstrdup(value);
+   }
}
 
 release_return:
diff --git a/submodule-config.h b/submodule-config.h
index 9061e4e..f9e2a29 100644
--- a/submodule-config.h
+++ b/submodule-config.h
@@ -14,6 +14,7 @@ struct submodule {
const char *url;
int fetch_recurse;
const char *ignore;
+   const char *update;
/* the sha1 blob id of the responsible .gitmodules file */
unsigned char gitmodules_sha1[20];
 };
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 03/12] git submodule update: Move branch calculation to where it's needed

[Stefan Beller]

The branch variable is used only once so calculate it only when needed.

Signed-off-by: Stefan Beller 
---
 git-submodule.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index eea27f8..56a0524 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -668,7 +668,6 @@ cmd_update()
fi
name=$(git submodule--helper name "$sm_path") || exit
url=$(git config submodule."$name".url)
-   branch=$(get_submodule_config "$name" branch master)
if ! test -z "$update"
then
update_module=$update
@@ -718,6 +717,7 @@ Maybe you want to use 'update --init'?")"
die "$(eval_gettext "Unable to fetch in 
submodule path '\$sm_path'")"
fi
remote_name=$(clear_local_git_env; cd "$sm_path" && 
get_default_remote)
+   branch=$(get_submodule_config "$name" branch master)
sha1=$(clear_local_git_env; cd "$sm_path" &&
git rev-parse --verify 
"${remote_name}/${branch}") ||
die "$(eval_gettext "Unable to find current 
${remote_name}/${branch} revision in submodule path '\$sm_path'")"
-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 10/12] git submodule update: Clone projects from within C

[Stefan Beller]

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 56 -
 git-submodule.sh| 12 ++
 2 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 7a2fd4e..d1684cf 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -260,9 +260,40 @@ static int git_submodule_config(const char *var, const 
char *value, void *cb)
return parse_submodule_config_option(var, value);
 }
 
+static void fill_clone_command(struct child_process *cp, int quiet,
+  const char *prefix, const char *path,
+  const char *name, const char *url,
+  const char *reference, const char *depth)
+{
+   cp->git_cmd = 1;
+   argv_array_push(>args, "submodule--helper");
+   argv_array_push(>args, "clone");
+   if (quiet)
+   argv_array_push(>args, "--quiet");
+
+   if (prefix) {
+   argv_array_push(>args, "--prefix");
+   argv_array_push(>args, prefix);
+   }
+   argv_array_push(>args, "--path");
+   argv_array_push(>args, path);
+
+   argv_array_push(>args, "--name");
+   argv_array_push(>args, name);
+
+   argv_array_push(>args, "--url");
+   argv_array_push(>args, url);
+   if (reference)
+   argv_array_push(>args, reference);
+   if (depth)
+   argv_array_push(>args, depth);
+}
+
 static int module_list_or_clone(int argc, const char **argv, const char 
*prefix)
 {
int i;
+   int quiet;
+   char *reference = NULL, *depth = NULL;
char *update = NULL;
struct pathspec pathspec;
struct module_list list = MODULE_LIST_INIT;
@@ -274,6 +305,13 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
OPT_STRING(0, "update", ,
   N_("string"),
   N_("update command for submodules")),
+   OPT_STRING(0, "reference", , "",
+   N_("Use the local reference repository "
+  "instead of a full clone")),
+   OPT_STRING(0, "depth", , "",
+   N_("Create a shallow clone truncated to the "
+  "specified number of revisions")),
+   OPT__QUIET(, N_("do't print cloning progress")),
OPT_END()
};
 
@@ -301,6 +339,7 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
struct strbuf sb = STRBUF_INIT;
const char *update_module = NULL;
const char *url = NULL;
+   int just_cloned = 0;
 
char *env_prefix = getenv("prefix");
if (ce_stage(ce)) {
@@ -350,7 +389,22 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
continue;
}
 
-   printf("%06o %s %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce));
+   strbuf_reset();
+   strbuf_addf(, "%s/.git", ce->name);
+   just_cloned = !file_exists(sb.buf);
+
+   if (just_cloned) {
+   struct child_process cp = CHILD_PROCESS_INIT;
+   fill_clone_command(, quiet, prefix, ce->name,
+  sub->name, url, reference, depth);
+
+   if (run_command()) {
+   printf("#unmatched\n");
+   return 1;
+   }
+   }
+
+   printf("%06o %s %d %d\t", ce->ce_mode, sha1_to_hex(ce->sha1), 
ce_stage(ce), just_cloned);
utf8_fprintf(stdout, "%s\n", ce->name);
}
return 0;
diff --git a/git-submodule.sh b/git-submodule.sh
index 80f41b2..28f1757 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -656,9 +656,14 @@ cmd_update()
fi
 
cloned_modules=
-   git submodule--helper list-or-clone --prefix "$wt_prefix" 
${update:+--update "$update"} "$@" | {
+   git submodule--helper list-or-clone ${GIT_QUIET:+--quiet} \
+   --prefix "$wt_prefix" \
+   ${update:+--update "$update"} \
+   ${reference:+--reference "$reference"} \
+   ${depth:+--depth "$depth"} \
+   "$@" | {
err=
-   while read mode sha1 stage sm_path
+   while read mode sha1 stage just_cloned sm_path
do
die_if_unmatched "$mode"
 
@@ -677,9 +682,8 @@ cmd_update()
 
displaypath=$(relative_path "$prefix$sm_path")
 
-   if ! test -d "$sm_path"/.git && ! test -f "$sm_path"/.git
+   if test "$just_cloned" = 1
then
-   git submodule--helper clone ${GIT_QUIET:+--quiet} 
--prefix 

[RFC PATCHv1 00/12] git submodule update in C with parallel cloning

[Stefan Beller]

So eventually we want to have projects with lots of submodules such as Android
(which would have O(1000) submodules).

The very first thing a user does is cloning a project, so we want to impress
with speed there as well. git clone however is lazy and just calls
`git submodule update --init --recursive`. So we need to make that fast.

This series rewrites parts of git submodule update in C and in the second-last
patch it separates cloning and doing the other actions(checkout/rebase/merge 
etc)
by doing the cloning first and then the rest.

The last patch (which is broken in the first version of the series), then 
proceeds to put the cloning of the submodules into the get_next_task callback
of the parallel process API.

That said, the first few patches introduce some churn in the behavior and tests
of Git, so maybe put your eyes there?

Thanks for any advice,
Stefan

Stefan Beller (12):
  git submodule update: Announce skipping submodules on stderr
  git submodule update: Announce uninitialized modules on stderr
  git submodule update: Move branch calculation to where it's needed
  git submodule update: Announce outcome of submodule operation to
stderr
  git submodule update: Use its own list implementation.
  git submodule update: Handle unmerged submodules in C
  submodule config: keep update strategy around
  git submodule update: check for "none" in C
  git submodule update: Check url in C
  git submodule update: Clone projects from within C
  submodule--helper: Do not emit submodules to process directly.
  WIP/broken Clone all outstanding submodules in parallel

 builtin/submodule--helper.c | 221 
 git-submodule.sh|  38 +++-
 submodule-config.c  |  11 +++
 submodule-config.h  |   1 +
 t/t7400-submodule-basic.sh  |  12 +--
 t/t7406-submodule-update.sh |  12 +--
 6 files changed, 256 insertions(+), 39 deletions(-)

-- 
2.5.0.277.gfdc362b.dirty

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

[PATCH 12/12] WIP/broken Clone all outstanding submodules in parallel

[Stefan Beller]

Our tests scream at this patch, it's just to show what I plan to do.
Essentially moving the content of the loop into the get_next_task
callback from the run_processes_parallel.

Signed-off-by: Stefan Beller 
---
 builtin/submodule--helper.c | 181 +---
 1 file changed, 119 insertions(+), 62 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index fa8c008..c66aa53 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -289,65 +289,40 @@ static void fill_clone_command(struct child_process *cp, 
int quiet,
argv_array_push(>args, depth);
 }
 
-static int module_list_or_clone(int argc, const char **argv, const char 
*prefix)
-{
-   int i;
-   int quiet;
-   char *reference = NULL, *depth = NULL;
-   char *update = NULL;
+struct submodule_list_or_clone {
struct pathspec pathspec;
-   struct module_list list = MODULE_LIST_INIT;
-   struct string_list projectlines = STRING_LIST_INIT_DUP;
-   struct string_list_item *item;
-
-   struct option module_list_options[] = {
-   OPT_STRING(0, "prefix", ,
-  N_("path"),
-  N_("alternative anchor for relative paths")),
-   OPT_STRING(0, "update", ,
-  N_("string"),
-  N_("update command for submodules")),
-   OPT_STRING(0, "reference", , "",
-   N_("Use the local reference repository "
-  "instead of a full clone")),
-   OPT_STRING(0, "depth", , "",
-   N_("Create a shallow clone truncated to the "
-  "specified number of revisions")),
-   OPT__QUIET(, N_("do't print cloning progress")),
-   OPT_END()
-   };
-
-   const char *const git_submodule_helper_usage[] = {
-   N_("git submodule--helper list [--prefix=] [...]"),
-   NULL
-   };
-
-   argc = parse_options(argc, argv, prefix, module_list_options,
-git_submodule_helper_usage, 0);
-
-   if (module_list_compute(argc, argv, prefix, , ) < 0) {
-   printf("#unmatched\n");
-   return 1;
-   }
+   struct module_list list;
+   struct string_list projectlines;
+   int count;
+   int quiet;
+   char *reference;
+   char *depth;
+   char *update;
+   char *env_prefix;
+   const char *prefix;
+   int print_unmatched;
+};
 
-   gitmodules_config();
-   /* Overlay the parsed .gitmodules file with .git/config */
-   git_config(git_submodule_config, NULL);
+static int get_next_task(void **pp_task_cb,
+struct child_process *cp,
+struct strbuf *err,
+void *pp_cb)
+{
+   struct submodule_list_or_clone *pp = pp_cb;
 
-   for (i = 0; i < list.nr; i++) {
+   for (; pp->count < pp->list.nr; pp->count++) {
const struct submodule *sub = NULL;
const char *displaypath = NULL;
-   const struct cache_entry *ce = list.entries[i];
+   const struct cache_entry *ce = pp->list.entries[pp->count];
struct strbuf sb = STRBUF_INIT;
const char *update_module = NULL;
const char *url = NULL;
int just_cloned = 0;
 
-   char *env_prefix = getenv("prefix");
if (ce_stage(ce)) {
-   if (env_prefix)
+   if (pp->env_prefix)
fprintf(stderr, "Skipping unmerged submodule 
%s/%s\n",
-   env_prefix, ce->name);
+   pp->env_prefix, ce->name);
else
fprintf(stderr, "Skipping unmerged submodule 
%s\n",
ce->name);
@@ -355,13 +330,13 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
}
 
sub = submodule_from_path(null_sha1, ce->name);
-   if (env_prefix)
-   displaypath = relative_path(env_prefix, ce->name, );
+   if (pp->env_prefix)
+   displaypath = relative_path(pp->env_prefix, ce->name, 
);
else
displaypath = ce->name;
 
-   if (update)
-   update_module = update;
+   if (pp->update)
+   update_module = pp->update;
if (!update_module)
update_module = sub->update;
if (!update_module)
@@ -385,7 +360,7 @@ static int module_list_or_clone(int argc, const char 
**argv, const char *prefix)
 * Only mention uninitialized submodules when its
   

[PATCH] pull: add angle brackets to usage string

[Alex Henrie]

Signed-off-by: Alex Henrie 
---
 builtin/pull.c   | 2 +-
 contrib/examples/git-pull.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a39bb0a..bf3fd3f 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -66,7 +66,7 @@ static int parse_opt_rebase(const struct option *opt, const 
char *arg, int unset
 }
 
 static const char * const pull_usage[] = {
-   N_("git pull [options] [ [...]]"),
+   N_("git pull [] [ [...]]"),
NULL
 };
 
diff --git a/contrib/examples/git-pull.sh b/contrib/examples/git-pull.sh
index 6b3a03f..bcf362e 100755
--- a/contrib/examples/git-pull.sh
+++ b/contrib/examples/git-pull.sh
@@ -8,7 +8,7 @@ SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
 OPTIONS_STUCKLONG=Yes
 OPTIONS_SPEC="\
-git pull [options] [ [...]]
+git pull [] [ [...]]
 
 Fetch one or more remote refs and integrate it/them with the current HEAD.
 --
-- 
2.6.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: about rev-parse's quietness

[Junio C Hamano]

 writes:

> Let's see:
> $ git rev-parse --verify --quiet master@{xyz} || echo No
> No
>
> $ git rev-parse --verify --quiet master@{u} || echo No
> fatal: no upstream configured for branch 'master'
> No

Yup, that is unfortunate.  The above two pass different codepaths in
sha1_name.c::get_sha1_basic().

 * The code first notices "master@{anything}" has @{anything}
   suffix, which could be @{123} (counted reflog), @{1.hour.ago}
   (timed reflog), @{-1} (prior checkout---valid only when nothing
   comes before "@{...}") or @{upstream}/@{push}.

 * The first two goes through dwim_log(), together with
   'master@{xyz}' to locate the reflog of 'master' (i.e. what comes
   before that @{anything}) and identifies that we need to look at
   the log for 'refs/heads/master', looks up an appropriate entry,
   and the processing ends there, returning success.  However, 'xyz'
   is not a valid specification for either counted or timed reflog,
   so the function returns failure for 'master@{xyz}', letting the
   remainder of the callchain to take care of it.
 
 * The third one goes to interpret_nth_prior_checkout() and the
   processing ends there, returning success.

 * The last one goes to dwim_ref(), together with just 'master'
   without any @{anything}.  It tries to turn master@{upstream} or
   master@{push} to refs/remotes/origin/master and then it is used
   to grab the object name pointed by it.

The helper function dwim_ref() calls to turn master@{upstream} to
refs/remotes/origin/master, interpret_branch_name(), is designed to
die() when fed a ref without upstream (or @{push} when there is no
such thing).

Naively, one would imagine that the die() can be demoted to an
internal error return to the caller (just like 'master@{xyz}' is
rejected silently in the dwim_log() codepath) and everything else
should go well.  It is unfortunately not sufficient, though, as the
other callers to interpret_branch_name() rely on the function to
die and are not prepared to show the error messages themselves.

So I think this is fixable and the right approach to fix it is as
hinted above, i.e.

 * audit callers of interpret_branch_name() and find the ones that
   depend on the function to die (i.e. it does not handle the case
   where interpret_branch_name() notices that 'master@{upstream}'
   does not exist).  Change them to die with an appropriate message
   themselves.

 * change interpret_branch_mark(), which is actually where the die()
   happens, so that it can notice a request to use 'master@{upstream}' 
   when there is no such upstream and signal an error to the caller.

I am not saying that you should be the one to fix it, though ;-) The
above is to help those who may be interested to look into the codepath
to do so.

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 v3] merge: fix cache_entry use-after-free

[Junio C Hamano]

David Turner  writes:

>> > +static inline void drop_ce_ref(struct cache_entry *ce)
>> > +{
>> > +  if (ce != NULL) {
>> > +  assert(ce->ref_count >= 0);
>> 
>> Shouldn't this be "> 0" to prevent double frees?
>
> No.  If the ref_count is 1, then there is still some reference to the
> ce.  If it is 0, there is no reference to it, and the next check (< 1)
> will succeed and the ce will get freed.  
>
>> > +  if (--ce->ref_count < 1) {
>> > +  free(ce);
>> > +  }
>> > +  }
>> > +}

Hmm, but it still feels fuzzy, no?  I cannot tell from the above
exchange if a ce with ref_count==0 upon entry to this function is
supposed to have somebody pointing at it, somebody just assigned
NULL (or another pointer) to the last pointer that was pointing at
it, or what else.  If the expected calling sequence were:

drop_ce_ref(thing->ce);
thing->ce = NULL;

and the original thing->ce were the last reference to the cache
entry about to be freed, its refcnt is better be 1 not 0.  And when
this function decrements refcnt down to 0, the cache entry is freed.

Admittedly, if the original refcnt were 0, with signed refcnt, it
will decrement to -1 and it will be freed, too, but I do not think
that is what was intended---refcnt is initialized to 0 upon creating
an unreferenced cache entry, and set_index_entry() and friends that
store a pointer to anything calls add_ce_ref(), so I read the code
as intending to make "0 means no pointer points at it".

As far as I can tell, the only effect of this assert() that uses >=0
not >0 is to avoid triggering it on this calling sequence:

new_ce = new_cache_entry();
drop_ce_ref(new_ce);

that is, you create because you _might_ use it, and then later
decide not to use it (and the free() part wouldn't have worked
correctly with unsigned refcnt ;-).

By the way, the log message says "During merges, we would previously
free entries that we no longer need in the destination index.  But
those entries might also be stored in the dir_entry cache," as if
this issue were present and waiting to trigger for all merges for
all people.  Given that Linus does hundreds of merges in a day
during the merge window (and I do several dozens a day), I am quite
surprised that nobody noticed this issue.  If there is a more
specific condition that allows this bug to trigger (e.g. "dir-entry
cache is used only under such and such conditions") that the log
message does not talk about to explain why this bug was not seen
widely, it would be a good thing to add.  It is very puzzling
otherwise.



--
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 16/31] mailinfo: move metainfo_charset to struct mailinfo

[Eric Sunshine]

On Wed, Oct 14, 2015 at 4:45 PM, Junio C Hamano  wrote:
> This requires us to pass the struct down to decode_header() and
> convert_to_utf8() callchain.
>
> Signed-off-by: Junio C Hamano 
> ---
> @@ -520,23 +519,24 @@ static struct strbuf *decode_b_segment(const struct 
> strbuf *b_seg)
> -static void convert_to_utf8(struct strbuf *line, const char *charset)
> +static void convert_to_utf8(struct mailinfo *mi,
> +   struct strbuf *line, const char *charset)
>  {
> char *out;
>
> -   if (!charset || !*charset)
> +   if (!mi->metainfo_charset || !charset || !*charset)
> return;

Mental note: convert_to_utf8() is updated to return early when
metainfo_charset==NULL.

> -   if (same_encoding(metainfo_charset, charset))
> +   if (same_encoding(mi->metainfo_charset, charset))
> return;
> -   out = reencode_string(line->buf, metainfo_charset, charset);
> +   out = reencode_string(line->buf, mi->metainfo_charset, charset);
> if (!out)
> die("cannot convert from %s to %s",
> -   charset, metainfo_charset);
> +   charset, mi->metainfo_charset);
> strbuf_attach(line, out, strlen(out), strlen(out));
>  }
>
> -static void decode_header(struct strbuf *it)
> +static void decode_header(struct mailinfo *mi, struct strbuf *it)
>  {
> char *in, *ep, *cp;
> struct strbuf outbuf = STRBUF_INIT, *dec;
> @@ -599,8 +599,7 @@ static void decode_header(struct strbuf *it)
> dec = decode_q_segment(, 1);
> break;
> }
> -   if (metainfo_charset)
> -   convert_to_utf8(dec, charset_q.buf);
> +   convert_to_utf8(mi, dec, charset_q.buf);

It's safe to drop the conditional here since convert_to_utf8() now
checks metainfo_charset. Okay.

> strbuf_addbuf(, dec);
> strbuf_release(dec);
> @@ -745,8 +744,7 @@ static int handle_commit_msg(struct mailinfo *mi, struct 
> strbuf *line)
> mi->header_stage = 0;
>
> /* normalize the log message to UTF-8. */
> -   if (metainfo_charset)
> -   convert_to_utf8(line, charset.buf);
> +   convert_to_utf8(mi, line, charset.buf);

Ditto.

> if (mi->use_scissors && is_scissors_line(line)) {
> int i;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Use the alternates of the source repository for dissociating clone

[Junio C Hamano]

Alexander Riesen  writes:

> The "--dissociate" option required reference repositories, which sometimes
> demanded a look into the objects/info/alternates by the user. As this
> is something which can be figured out automatically, do it in the
> clone unless there is no other reference repositories.

I do not quite get this.

Before "clone" with or without "--dissociate" there is no
objects/info/alternates (before "clone", there is no ".git" to find
that file in the first place).

Are you talking about making a clone of a repository that was
created with "clone --reference", to borrow from the same
third repository the original is borrowing from?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Matthieu Moy]

Tobias Klauser  writes:

> --- a/Documentation/git-stripspace.txt
> +++ b/Documentation/git-stripspace.txt
> @@ -9,7 +9,7 @@ git-stripspace - Remove unnecessary whitespace
>  SYNOPSIS
>  
>  [verse]
> -'git stripspace' [-s | --strip-comments] < input
> +'git stripspace' [-s | --strip-comments] [-C | --count-lines] < input

I'm not sure it's a good idea to introduce a one-letter shortcut (-C).
In scripts, --count-lines is self-explanatory hence more readable than
-C (which is even more confusing since "git -C foo stripspace" and "git
stripspace -C" have totally different meanings. Outside scripts, I'm not
sure the command would be useful. I'd rather avoid polluting the
one-letter-option namespace.

> +Use 'git stripspace --count-lines' to obtain:
> +
> +-
> +|5$
> +-

In the examples above, I read the | as part of the input (unlike $ which
is used only to show the end of line). So the | should not be here. I
don't think you need the $ either, the --count-lines option is no longer
about trailing whitespaces.

> +static const char * const usage_msg[] = {

Stick the * to usage_msg please.

No time to review the rest for now sorry.

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

Re: [PATCH v3 1/3] Add Travis CI support

[Matthieu Moy]

Johannes Schindelin  writes:

> On Thu, 15 Oct 2015, Matthieu Moy wrote:
>
>> Since the tests are essentially IO-bound and not CPU-bound, it may even
>> make sense to use -j3 here.
>
> I would like to caution against overloading Travis. They are really nice
> to us, we should try to be nice to them, too.

Right.

Using a bit of parallelism shouldn't harm (we put a heavier load on
them, but for a shorter time), so using -j2 or -j3 seems OK to me, but I
wouldn't go higher to remain gentle with Travis CI.

Note: I'm writting this without having a real idea of what consequence
of -jN have on their infrastructure, take my writing as wild guess, not
as real arguments.

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

Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf

[Matthieu Moy]

Tobias Klauser  writes:

> [1] 
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf

I don't think we want such references in the commit message. It does
make sense in a "below ---" comment, but commit messages are here to
stay forever, while the SmallProjectsIdeas entries are meant to be
deleted when they are completed.

(Same in other patches)

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

Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf

[Matthieu Moy]

Tobias Klauser  writes:

> Also switch all current users of stripspace() to the new function name
> and  keep a temporary wrapper inline function for topic branches still
> using stripspace().

Since you have this temporary wrapper, it would have made sense to split
the patch into 1) move and rename the function, and 2) change the
callsites to strbuf_stripspace. This way 2) would be absolutely trivial
to review.

OTOH, this patch is already easy to review, so you may consider it's OK
like this.

Reviewed-by: Matthieu Moy 

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

Re: [PATCH 1/3] strbuf: make stripspace() part of strbuf

[Junio C Hamano]

Tobias Klauser  writes:

> Rename stripspace() to strbuf_stripspace() and move it to the strbuf
> module as suggested in [1].
>
> Also switch all current users of stripspace() to the new function name
> and  keep a temporary wrapper inline function for topic branches still
> using stripspace().
>
> [1] 
> https://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf

I think Matthieu already mentioned this, but please explain why it
is a good idea to do this in your own words here, without forcing
readers to go to other places to find out.  Giving credit to an
external page for giving you an inspiration to do something is good,
but the proposed log message needs to justify itself.  In other
words, when you are challenged to defend this change, you are not
allowed to say "SmallProjectIdeas page said it is a good thing to
do.  I just did it without questioning it." ;-)  Instead you are
expected to justify it yourself.

> Signed-off-by: Tobias Klauser 
> ---

A good rule of thumb to see if it is a good time to do this kind of
change is to do this:

$ git log -p maint..pu | grep stripspace

which shows only one mention in a context, so this change may cause
textual conflict with something else somewhere nearby but won't hurt
other topics in flight.

The patch itself looks OK.

Thanks.

>  builtin/am.c |  2 +-
>  builtin/branch.c |  2 +-
>  builtin/commit.c |  6 ++---
>  builtin/merge.c  |  2 +-
>  builtin/notes.c  |  6 ++---
>  builtin/stripspace.c | 69 
> ++--
>  builtin/tag.c|  2 +-
>  strbuf.c | 66 +
>  strbuf.h | 11 -
>  9 files changed, 88 insertions(+), 78 deletions(-)
>
> diff --git a/builtin/am.c b/builtin/am.c
> index 4f77e07..fbe9152 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1343,7 +1343,7 @@ static int parse_mail(struct am_state *state, const 
> char *mail)
>   strbuf_addstr(, "\n\n");
>   if (strbuf_read_file(, am_path(state, "msg"), 0) < 0)
>   die_errno(_("could not read '%s'"), am_path(state, "msg"));
> - stripspace(, 0);
> + strbuf_stripspace(, 0);
>  
>   if (state->signoff)
>   am_signoff();
> diff --git a/builtin/branch.c b/builtin/branch.c
> index 3ba4d1b..3f48746 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -794,7 +794,7 @@ static int edit_branch_description(const char 
> *branch_name)
>   strbuf_release();
>   return -1;
>   }
> - stripspace(, 1);
> + strbuf_stripspace(, 1);
>  
>   strbuf_addf(, "branch.%s.description", branch_name);
>   status = git_config_set(name.buf, buf.len ? buf.buf : NULL);
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 63772d0..dca09e2 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -775,7 +775,7 @@ static int prepare_to_commit(const char *index_file, 
> const char *prefix,
>   s->hints = 0;
>  
>   if (clean_message_contents)
> - stripspace(, 0);
> + strbuf_stripspace(, 0);
>  
>   if (signoff)
>   append_signoff(, ignore_non_trailer(), 0);
> @@ -1014,7 +1014,7 @@ static int template_untouched(struct strbuf *sb)
>   if (!template_file || strbuf_read_file(, template_file, 0) <= 0)
>   return 0;
>  
> - stripspace(, cleanup_mode == CLEANUP_ALL);
> + strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
>   if (!skip_prefix(sb->buf, tmpl.buf, ))
>   start = sb->buf;
>   strbuf_release();
> @@ -1726,7 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char 
> *prefix)
>   wt_status_truncate_message_at_cut_line();
>  
>   if (cleanup_mode != CLEANUP_NONE)
> - stripspace(, cleanup_mode == CLEANUP_ALL);
> + strbuf_stripspace(, cleanup_mode == CLEANUP_ALL);
>   if (template_untouched() && !allow_empty_message) {
>   rollback_index_files();
>   fprintf(stderr, _("Aborting commit; you did not edit the 
> message.\n"));
> diff --git a/builtin/merge.c b/builtin/merge.c
> index a0edaca..e6741f3 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -806,7 +806,7 @@ static void prepare_to_commit(struct commit_list 
> *remoteheads)
>   abort_commit(remoteheads, NULL);
>   }
>   read_merge_msg();
> - stripspace(, 0 < option_edit);
> + strbuf_stripspace(, 0 < option_edit);
>   if (!msg.len)
>   abort_commit(remoteheads, _("Empty commit message."));
>   strbuf_release(_msg);
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 3608c64..bb23d55 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -192,7 +192,7 @@ static void prepare_note_data(const unsigned char 
> *object, struct note_data *d,
>   if (launch_editor(d->edit_path, >buf, NULL)) {
> 

Re: [BUG] t7063-status-untracked-cache flaky?

[David Turner]

On Thu, 2015-10-15 at 09:52 +0200, Lars Schneider wrote:
> Hi,
> 
> I noticed that "t7063-status-untracked-cache.sh" occasionally fails with "not 
> ok 24 - test sparse status with untracked cache".
> 
> E.g. on OS X compiled with gcc:
> https://travis-ci.org/larsxschneider/git/jobs/85432514
> 
> E.g. on Linux compiled with gcc:
> https://travis-ci.org/larsxschneider/git/jobs/84986975
> 
> The test was added with commit 7687252. I have not really investigated the 
> problem yet but the "avoid_racy" method caught my attention. Is this test 
> known to be flaky? Would an increased sleep time in "avoid_racy" help?

I've never seen this test be flaky before your report.  I just tested
it, locally, though, and I can reproduce.  After 53 successes, it
failed.  Will investigate further.

--
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 v4 14/26] refs.c: move is_branch to the common code

[David Turner]

From: Ronnie Sahlberg 

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 5 -
 refs.c  | 5 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 345bee7..5c6c743 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -3003,11 +3003,6 @@ static int log_ref_write(const char *refname, const 
unsigned char *old_sha1,
return ret;
 }
 
-int is_branch(const char *refname)
-{
-   return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
-}
-
 /*
  * Write sha1 into the open lockfile, then close the lockfile. On
  * errors, rollback the lockfile, fill in *err and
diff --git a/refs.c b/refs.c
index d79ba82..f2f8052 100644
--- a/refs.c
+++ b/refs.c
@@ -732,3 +732,8 @@ int check_refname_format(const char *refname, int flags)
return -1; /* Refname has only one component. */
return 0;
 }
+
+int is_branch(const char *refname)
+{
+   return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 03/26] refs-be-files.c: rename refs to refs-be-files

[David Turner]

From: Ronnie Sahlberg 

Rename refs.c to refs-be-files.c to indicate that this file now
holds the implementation for the files based refs backend.
A smaller portion of the code in this file is backend agnostic and will
be moved to a a new refs.c file that will hold all the common refs code
that is shared across all backends.

A second reason for first moving all the code to the new file and then
move the backend agnostic code back to refs.c instead of the other way
around is because the code that will eventually remain in this new
refs-be-files.c file is so entangled that it would then be very
difficult to break the split up into small independent patches/chunks.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 Makefile  | 2 +-
 refs.c => refs-be-files.c | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename refs.c => refs-be-files.c (100%)

diff --git a/Makefile b/Makefile
index 0d9f5dd..173b9d4 100644
--- a/Makefile
+++ b/Makefile
@@ -762,7 +762,7 @@ LIB_OBJS += quote.o
 LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
 LIB_OBJS += reflog-walk.o
-LIB_OBJS += refs.o
+LIB_OBJS += refs-be-files.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
diff --git a/refs.c b/refs-be-files.c
similarity index 100%
rename from refs.c
rename to refs-be-files.c
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 16/26] refs.c: move ref iterators to the common code

[David Turner]

From: Ronnie Sahlberg 

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 82 -
 refs.c  | 81 
 2 files changed, 81 insertions(+), 82 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index c8b44ab..14319cb 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1668,23 +1668,6 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags,
return ret;
 }
 
-/* The argument to filter_refs */
-struct ref_filter {
-   const char *pattern;
-   each_ref_fn *fn;
-   void *cb_data;
-};
-
-static int filter_refs(const char *refname, const struct object_id *oid,
-  int flags, void *data)
-{
-   struct ref_filter *filter = (struct ref_filter *)data;
-
-   if (wildmatch(filter->pattern, refname, 0, NULL))
-   return 0;
-   return filter->fn(refname, oid, flags, filter->cb_data);
-}
-
 enum peel_status {
/* object was peeled successfully: */
PEEL_PEELED = 0,
@@ -1950,37 +1933,6 @@ int for_each_ref_in_submodule(const char *submodule, 
const char *prefix,
 {
return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
-
-int for_each_tag_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in("refs/tags/", fn, cb_data);
-}
-
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
-}
-
-int for_each_branch_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in("refs/heads/", fn, cb_data);
-}
-
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
-}
-
-int for_each_remote_ref(each_ref_fn fn, void *cb_data)
-{
-   return for_each_ref_in("refs/remotes/", fn, cb_data);
-}
-
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
-{
-   return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, 
cb_data);
-}
-
 int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(_cache, git_replace_ref_base, fn,
@@ -2012,40 +1964,6 @@ int for_each_namespaced_ref(each_ref_fn fn, void 
*cb_data)
return ret;
 }
 
-int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
-   const char *prefix, void *cb_data)
-{
-   struct strbuf real_pattern = STRBUF_INIT;
-   struct ref_filter filter;
-   int ret;
-
-   if (!prefix && !starts_with(pattern, "refs/"))
-   strbuf_addstr(_pattern, "refs/");
-   else if (prefix)
-   strbuf_addstr(_pattern, prefix);
-   strbuf_addstr(_pattern, pattern);
-
-   if (!has_glob_specials(pattern)) {
-   /* Append implied '/' '*' if not present. */
-   strbuf_complete(_pattern, '/');
-   /* No need to check for '*', there is none. */
-   strbuf_addch(_pattern, '*');
-   }
-
-   filter.pattern = real_pattern.buf;
-   filter.fn = fn;
-   filter.cb_data = cb_data;
-   ret = for_each_ref(filter_refs, );
-
-   strbuf_release(_pattern);
-   return ret;
-}
-
-int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
-{
-   return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
-}
-
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(_cache, "", fn, 0,
diff --git a/refs.c b/refs.c
index 642a2ab..f7aa9b3 100644
--- a/refs.c
+++ b/refs.c
@@ -746,3 +746,84 @@ const char *prettify_refname(const char *name)
starts_with(name, "refs/remotes/") ? 13 :
0);
 }
+
+/* The argument to filter_refs */
+struct ref_filter {
+   const char *pattern;
+   each_ref_fn *fn;
+   void *cb_data;
+};
+
+static int filter_refs(const char *refname, const struct object_id *oid, int 
flags,
+  void *data)
+{
+   struct ref_filter *filter = (struct ref_filter *)data;
+
+   if (wildmatch(filter->pattern, refname, 0, NULL))
+   return 0;
+   return filter->fn(refname, oid, flags, filter->cb_data);
+}
+
+int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
+   const char *prefix, void *cb_data)
+{
+   struct strbuf real_pattern = STRBUF_INIT;
+   struct ref_filter filter;
+   int ret;
+
+   if (!prefix && !starts_with(pattern, "refs/"))
+   strbuf_addstr(_pattern, "refs/");
+   else if (prefix)
+   strbuf_addstr(_pattern, prefix);
+   strbuf_addstr(_pattern, pattern);
+
+   if (!has_glob_specials(pattern)) {
+   /* Append implied '/' '*' if not present. */
+   strbuf_complete(_pattern, '/');
+

[PATCH v4 12/26] refs.c: move resolve_refdup to common

[David Turner]

From: Ronnie Sahlberg 

This function can be shared across all refs backends so move it
to the common code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 7 ---
 refs.c  | 7 +++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 728751a..589e10d 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1777,13 +1777,6 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags,
return ret;
 }
 
-char *resolve_refdup(const char *refname, int resolve_flags,
-unsigned char *sha1, int *flags)
-{
-   return xstrdup_or_null(resolve_ref_unsafe(refname, resolve_flags,
- sha1, flags));
-}
-
 /* The argument to filter_refs */
 struct ref_filter {
const char *pattern;
diff --git a/refs.c b/refs.c
index 8a619a6..fcd5ddd 100644
--- a/refs.c
+++ b/refs.c
@@ -616,3 +616,10 @@ int ref_exists(const char *refname)
unsigned char sha1[20];
return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);
 }
+
+char *resolve_refdup(const char *refname, int resolve_flags,
+unsigned char *sha1, int *flags)
+{
+   return xstrdup_or_null(resolve_ref_unsafe(refname, resolve_flags,
+ sha1, flags));
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 18/26] refs: move transaction functions into common code

[David Turner]

The common ref code will build up a ref transaction.  Backends will
then commit it.  So the transaction creation and update functions should
be in the common code.  We also need to move the ref structs into
the common code so that alternate backends can access them.

Later, we will modify struct ref_update to support alternate backends.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 198 
 refs.c  | 108 +++
 refs.h  |  91 +-
 3 files changed, 198 insertions(+), 199 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index ffc3813..73eb1e2 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -13,41 +13,6 @@ struct ref_lock {
struct object_id old_oid;
 };
 
-/*
- * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken
- * refs (i.e., because the reference is about to be deleted anyway).
- */
-#define REF_DELETING   0x02
-
-/*
- * Used as a flag in ref_update::flags when a loose ref is being
- * pruned.
- */
-#define REF_ISPRUNING  0x04
-
-/*
- * Used as a flag in ref_update::flags when the reference should be
- * updated to new_sha1.
- */
-#define REF_HAVE_NEW   0x08
-
-/*
- * Used as a flag in ref_update::flags when old_sha1 should be
- * checked.
- */
-#define REF_HAVE_OLD   0x10
-
-/*
- * Used as a flag in ref_update::flags when the lockfile needs to be
- * committed.
- */
-#define REF_NEEDS_COMMIT 0x20
-
-/*
- * 0x40 is REF_FORCE_CREATE_REFLOG, so skip it if you're adding a
- * value to ref_update::flags
- */
-
 struct ref_entry;
 
 /*
@@ -3289,169 +3254,6 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
 }
 
-/**
- * Information needed for a single ref update. Set new_sha1 to the new
- * value or to null_sha1 to delete the ref. To check the old value
- * while the ref is locked, set (flags & REF_HAVE_OLD) and set
- * old_sha1 to the old value, or to null_sha1 to ensure the ref does
- * not exist before update.
- */
-struct ref_update {
-   /*
-* If (flags & REF_HAVE_NEW), set the reference to this value:
-*/
-   unsigned char new_sha1[20];
-   /*
-* If (flags & REF_HAVE_OLD), check that the reference
-* previously had this value:
-*/
-   unsigned char old_sha1[20];
-   /*
-* One or more of REF_HAVE_NEW, REF_HAVE_OLD, REF_NODEREF,
-* REF_DELETING, and REF_ISPRUNING:
-*/
-   unsigned int flags;
-   struct ref_lock *lock;
-   int type;
-   char *msg;
-   const char refname[FLEX_ARRAY];
-};
-
-/*
- * Transaction states.
- * OPEN:   The transaction is in a valid state and can accept new updates.
- * An OPEN transaction can be committed.
- * CLOSED: A closed transaction is no longer active and no other operations
- * than free can be used on it in this state.
- * A transaction can either become closed by successfully committing
- * an active transaction or if there is a failure while building
- * the transaction thus rendering it failed/inactive.
- */
-enum ref_transaction_state {
-   REF_TRANSACTION_OPEN   = 0,
-   REF_TRANSACTION_CLOSED = 1
-};
-
-/*
- * Data structure for holding a reference transaction, which can
- * consist of checks and updates to multiple references, carried out
- * as atomically as possible.  This structure is opaque to callers.
- */
-struct ref_transaction {
-   struct ref_update **updates;
-   size_t alloc;
-   size_t nr;
-   enum ref_transaction_state state;
-};
-
-struct ref_transaction *ref_transaction_begin(struct strbuf *err)
-{
-   assert(err);
-
-   return xcalloc(1, sizeof(struct ref_transaction));
-}
-
-void ref_transaction_free(struct ref_transaction *transaction)
-{
-   int i;
-
-   if (!transaction)
-   return;
-
-   for (i = 0; i < transaction->nr; i++) {
-   free(transaction->updates[i]->msg);
-   free(transaction->updates[i]);
-   }
-   free(transaction->updates);
-   free(transaction);
-}
-
-static struct ref_update *add_update(struct ref_transaction *transaction,
-const char *refname)
-{
-   size_t len = strlen(refname) + 1;
-   struct ref_update *update = xcalloc(1, sizeof(*update) + len);
-
-   memcpy((char *)update->refname, refname, len); /* includes NUL */
-   ALLOC_GROW(transaction->updates, transaction->nr + 1, 
transaction->alloc);
-   transaction->updates[transaction->nr++] = update;
-   return update;
-}
-
-int ref_transaction_update(struct ref_transaction *transaction,
-  const char *refname,
-  const unsigned char *new_sha1,
-  const unsigned char *old_sha1,
-  unsigned int flags, const char *msg,
-

[PATCH v4 00/26] refs backend pre-vtable patches

[David Turner]

Michael and Junio want to split the refs-backend patches into multiple
sections: pre-vtable, vtable, and maybe lmdb.

This is the pre-vtable section.  It includes multiple fixes suggested
by Michael:

1. When we move chunks around between files, make sure we don't cut
new versions and paste old versions!

2. Some rearrangement of patches

3. Made fewer things public, since they are really only needed for
files-backend optimizations.

4. Comment fixes (and moving doc-comments to refs.h when functions
become public)

Also, on my own, I moved Jeff's repository format patch into this
chunk, since it should be relatively non-controversial.

David Turner (9):
  refs: make repack_without_refs and is_branch public
  refs: move transaction functions into common code
  refs.c: move refname_is_safe to the common code
  refs.c: move copy_msg to the common code
  refs.c: move peel_object to the common code
  refs.c: move should_autocreate_reflog to common code
  initdb: move safe_create_dir into common code
  refs: make files_log_ref_write functions public
  refs: break out ref conflict checks

Jeff King (1):
  introduce "extensions" form of core.repositoryformatversion

Ronnie Sahlberg (16):
  refs.c: create a public version of verify_refname_available
  refs-be-files.c: rename refs to refs-be-files
  refs.c: add a new refs.c file to hold all common refs code
  refs.c: move update_ref to refs.c
  refs.c: move delete_pseudoref and delete_ref to the common code
  refs.c: move read_ref_at to the common refs file
  refs.c: move the hidden refs functions to the common code
  refs.c: move dwim and friend functions to the common refs code
  refs.c: move warn_if_dangling_symref* to the common code
  refs.c: move read_ref, read_ref_full and ref_exists to the common code
  refs.c: move resolve_refdup to common
  refs.c: move check_refname_format to the common code
  refs.c: move is_branch to the common code
  refs.c: move prettify_refname to the common code
  refs.c: move ref iterators to the common code
  refs.c: move head_ref_namespaced to the common code

 Documentation/technical/repository-version.txt |   81 +
 Makefile   |1 +
 builtin/init-db.c  |   12 -
 cache.h|   11 +
 path.c |   12 +
 refs-be-files.c| 3513 
 refs.c | 5172 
 refs.h |  187 +-
 setup.c|   37 +-
 t/t1302-repo-version.sh|   38 +
 10 files changed, 4645 insertions(+), 4419 deletions(-)
 create mode 100644 Documentation/technical/repository-version.txt
 create mode 100644 refs-be-files.c

-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 02/26] refs: make repack_without_refs and is_branch public

[David Turner]

is_branch was already non-static, but this patch declares it in the
header.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs.c | 5 +++--
 refs.h | 2 ++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index fe71ea0..84abc82 100644
--- a/refs.c
+++ b/refs.c
@@ -2816,8 +2816,9 @@ int pack_refs(unsigned int flags)
 
 /*
  * Rewrite the packed-refs file, omitting any refs listed in
- * 'refnames'. On error, leave packed-refs unchanged, write an error
- * message to 'err', and return a nonzero value.
+ * 'refnames'. On error, packed-refs will be unchanged, the return
+ * value is nonzero, and a message about the error is written to the
+ * 'err' strbuf.
  *
  * The refs in 'refnames' needn't be sorted. `err` must not be NULL.
  */
diff --git a/refs.h b/refs.h
index 7367a7f..8408bef 100644
--- a/refs.h
+++ b/refs.h
@@ -237,6 +237,8 @@ int pack_refs(unsigned int flags);
 int verify_refname_available(const char *newname, struct string_list *extra,
 struct string_list *skip, struct strbuf *err);
 
+extern int is_branch(const char *refname);
+
 /*
  * Flags controlling ref_transaction_update(), ref_transaction_create(), etc.
  * REF_NODEREF: act on the ref directly, instead of dereferencing
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 19/26] refs.c: move refname_is_safe to the common code

[David Turner]

This function does not contain any backend specific code so we move it
to the common code. This function might be used by other refs backends.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 33 -
 refs.c  | 24 
 refs.h  | 11 +++
 3 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 73eb1e2..8d966f1 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -196,39 +196,6 @@ static struct ref_dir *get_ref_dir(struct ref_entry *entry)
return dir;
 }
 
-/*
- * Check if a refname is safe.
- * For refs that start with "refs/" we consider it safe as long they do
- * not try to resolve to outside of refs/.
- *
- * For all other refs we only consider them safe iff they only contain
- * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
- * "config").
- */
-static int refname_is_safe(const char *refname)
-{
-   if (starts_with(refname, "refs/")) {
-   char *buf;
-   int result;
-
-   buf = xmalloc(strlen(refname) + 1);
-   /*
-* Does the refname try to escape refs/?
-* For example: refs/foo/../bar is safe but refs/foo/../../bar
-* is not.
-*/
-   result = !normalize_path_copy(buf, refname + strlen("refs/"));
-   free(buf);
-   return result;
-   }
-   while (*refname) {
-   if (!isupper(*refname) && *refname != '_')
-   return 0;
-   refname++;
-   }
-   return 1;
-}
-
 static struct ref_entry *create_ref_entry(const char *refname,
  const unsigned char *sha1, int flag,
  int check_name)
diff --git a/refs.c b/refs.c
index be08787..3eb3a28 100644
--- a/refs.c
+++ b/refs.c
@@ -950,3 +950,27 @@ int ref_transaction_verify(struct ref_transaction 
*transaction,
  NULL, old_sha1,
  flags, NULL, err);
 }
+
+int refname_is_safe(const char *refname)
+{
+   if (starts_with(refname, "refs/")) {
+   char *buf;
+   int result;
+
+   buf = xmalloc(strlen(refname) + 1);
+   /*
+* Does the refname try to escape refs/?
+* For example: refs/foo/../bar is safe but refs/foo/../../bar
+* is not.
+*/
+   result = !normalize_path_copy(buf, refname + strlen("refs/"));
+   free(buf);
+   return result;
+   }
+   while (*refname) {
+   if (!isupper(*refname) && *refname != '_')
+   return 0;
+   refname++;
+   }
+   return 1;
+}
diff --git a/refs.h b/refs.h
index 3d24c79..be94e18 100644
--- a/refs.h
+++ b/refs.h
@@ -396,6 +396,17 @@ extern int for_each_reflog(each_ref_fn, void *);
  */
 extern int check_refname_format(const char *refname, int flags);
 
+/*
+ * Check if a refname is safe.
+ * For refs that start with "refs/" we consider it safe as long they do
+ * not try to resolve to outside of refs/.
+ *
+ * For all other refs we only consider them safe iff they only contain
+ * upper case characters and '_' (like "HEAD" AND "MERGE_HEAD", and not like
+ * "config").
+ */
+extern int refname_is_safe(const char *refname);
+
 extern const char *prettify_refname(const char *refname);
 
 extern char *shorten_unambiguous_ref(const char *refname, int strict);
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 10/26] refs.c: move warn_if_dangling_symref* to the common code

[David Turner]

From: Ronnie Sahlberg 

These functions do not use any backend specific code so we move
them to the common code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 52 
 refs.c  | 52 
 2 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 3609fb7..9c53f54 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1944,58 +1944,6 @@ int peel_ref(const char *refname, unsigned char *sha1)
return peel_object(base, sha1);
 }
 
-struct warn_if_dangling_data {
-   FILE *fp;
-   const char *refname;
-   const struct string_list *refnames;
-   const char *msg_fmt;
-};
-
-static int warn_if_dangling_symref(const char *refname, const struct object_id 
*oid,
-  int flags, void *cb_data)
-{
-   struct warn_if_dangling_data *d = cb_data;
-   const char *resolves_to;
-   struct object_id junk;
-
-   if (!(flags & REF_ISSYMREF))
-   return 0;
-
-   resolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);
-   if (!resolves_to
-   || (d->refname
-   ? strcmp(resolves_to, d->refname)
-   : !string_list_has_string(d->refnames, resolves_to))) {
-   return 0;
-   }
-
-   fprintf(d->fp, d->msg_fmt, refname);
-   fputc('\n', d->fp);
-   return 0;
-}
-
-void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
-{
-   struct warn_if_dangling_data data;
-
-   data.fp = fp;
-   data.refname = refname;
-   data.refnames = NULL;
-   data.msg_fmt = msg_fmt;
-   for_each_rawref(warn_if_dangling_symref, );
-}
-
-void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct 
string_list *refnames)
-{
-   struct warn_if_dangling_data data;
-
-   data.fp = fp;
-   data.refname = NULL;
-   data.refnames = refnames;
-   data.msg_fmt = msg_fmt;
-   for_each_rawref(warn_if_dangling_symref, );
-}
-
 /*
  * Call fn for each reference in the specified ref_cache, omitting
  * references not in the containing_dir of base.  fn is called for all
diff --git a/refs.c b/refs.c
index 53296e9..fab7603 100644
--- a/refs.c
+++ b/refs.c
@@ -546,3 +546,55 @@ char *shorten_unambiguous_ref(const char *refname, int 
strict)
free(short_name);
return xstrdup(refname);
 }
+
+struct warn_if_dangling_data {
+   FILE *fp;
+   const char *refname;
+   const struct string_list *refnames;
+   const char *msg_fmt;
+};
+
+static int warn_if_dangling_symref(const char *refname, const struct object_id 
*oid,
+  int flags, void *cb_data)
+{
+   struct warn_if_dangling_data *d = cb_data;
+   const char *resolves_to;
+   struct object_id junk;
+
+   if (!(flags & REF_ISSYMREF))
+   return 0;
+
+   resolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);
+   if (!resolves_to
+   || (d->refname
+   ? strcmp(resolves_to, d->refname)
+   : !string_list_has_string(d->refnames, resolves_to))) {
+   return 0;
+   }
+
+   fprintf(d->fp, d->msg_fmt, refname);
+   fputc('\n', d->fp);
+   return 0;
+}
+
+void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname)
+{
+   struct warn_if_dangling_data data;
+
+   data.fp = fp;
+   data.refname = refname;
+   data.refnames = NULL;
+   data.msg_fmt = msg_fmt;
+   for_each_rawref(warn_if_dangling_symref, );
+}
+
+void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct 
string_list *refnames)
+{
+   struct warn_if_dangling_data data;
+
+   data.fp = fp;
+   data.refname = NULL;
+   data.refnames = refnames;
+   data.msg_fmt = msg_fmt;
+   for_each_rawref(warn_if_dangling_symref, );
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 07/26] refs.c: move read_ref_at to the common refs file

[David Turner]

From: Ronnie Sahlberg 

Move read_ref_at() to the refs.c file since this function does not
contain any backend specific code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 118 
 refs.c  | 118 
 2 files changed, 118 insertions(+), 118 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index d969066..04c3206 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -3470,124 +3470,6 @@ int create_symref(const char *ref_target, const char 
*refs_heads_master,
return 0;
 }
 
-struct read_ref_at_cb {
-   const char *refname;
-   unsigned long at_time;
-   int cnt;
-   int reccnt;
-   unsigned char *sha1;
-   int found_it;
-
-   unsigned char osha1[20];
-   unsigned char nsha1[20];
-   int tz;
-   unsigned long date;
-   char **msg;
-   unsigned long *cutoff_time;
-   int *cutoff_tz;
-   int *cutoff_cnt;
-};
-
-static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
-   const char *email, unsigned long timestamp, int tz,
-   const char *message, void *cb_data)
-{
-   struct read_ref_at_cb *cb = cb_data;
-
-   cb->reccnt++;
-   cb->tz = tz;
-   cb->date = timestamp;
-
-   if (timestamp <= cb->at_time || cb->cnt == 0) {
-   if (cb->msg)
-   *cb->msg = xstrdup(message);
-   if (cb->cutoff_time)
-   *cb->cutoff_time = timestamp;
-   if (cb->cutoff_tz)
-   *cb->cutoff_tz = tz;
-   if (cb->cutoff_cnt)
-   *cb->cutoff_cnt = cb->reccnt - 1;
-   /*
-* we have not yet updated cb->[n|o]sha1 so they still
-* hold the values for the previous record.
-*/
-   if (!is_null_sha1(cb->osha1)) {
-   hashcpy(cb->sha1, nsha1);
-   if (hashcmp(cb->osha1, nsha1))
-   warning("Log for ref %s has gap after %s.",
-   cb->refname, show_date(cb->date, 
cb->tz, DATE_MODE(RFC2822)));
-   }
-   else if (cb->date == cb->at_time)
-   hashcpy(cb->sha1, nsha1);
-   else if (hashcmp(nsha1, cb->sha1))
-   warning("Log for ref %s unexpectedly ended on %s.",
-   cb->refname, show_date(cb->date, cb->tz,
-  DATE_MODE(RFC2822)));
-   hashcpy(cb->osha1, osha1);
-   hashcpy(cb->nsha1, nsha1);
-   cb->found_it = 1;
-   return 1;
-   }
-   hashcpy(cb->osha1, osha1);
-   hashcpy(cb->nsha1, nsha1);
-   if (cb->cnt > 0)
-   cb->cnt--;
-   return 0;
-}
-
-static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
- const char *email, unsigned long timestamp,
- int tz, const char *message, void *cb_data)
-{
-   struct read_ref_at_cb *cb = cb_data;
-
-   if (cb->msg)
-   *cb->msg = xstrdup(message);
-   if (cb->cutoff_time)
-   *cb->cutoff_time = timestamp;
-   if (cb->cutoff_tz)
-   *cb->cutoff_tz = tz;
-   if (cb->cutoff_cnt)
-   *cb->cutoff_cnt = cb->reccnt;
-   hashcpy(cb->sha1, osha1);
-   if (is_null_sha1(cb->sha1))
-   hashcpy(cb->sha1, nsha1);
-   /* We just want the first entry */
-   return 1;
-}
-
-int read_ref_at(const char *refname, unsigned int flags, unsigned long 
at_time, int cnt,
-   unsigned char *sha1, char **msg,
-   unsigned long *cutoff_time, int *cutoff_tz, int *cutoff_cnt)
-{
-   struct read_ref_at_cb cb;
-
-   memset(, 0, sizeof(cb));
-   cb.refname = refname;
-   cb.at_time = at_time;
-   cb.cnt = cnt;
-   cb.msg = msg;
-   cb.cutoff_time = cutoff_time;
-   cb.cutoff_tz = cutoff_tz;
-   cb.cutoff_cnt = cutoff_cnt;
-   cb.sha1 = sha1;
-
-   for_each_reflog_ent_reverse(refname, read_ref_at_ent, );
-
-   if (!cb.reccnt) {
-   if (flags & GET_SHA1_QUIETLY)
-   exit(128);
-   else
-   die("Log for %s is empty.", refname);
-   }
-   if (cb.found_it)
-   return 0;
-
-   for_each_reflog_ent(refname, read_ref_at_ent_oldest, );
-
-   return 1;
-}
-
 int reflog_exists(const char *refname)
 {
struct stat st;
diff --git a/refs.c b/refs.c
index f4f0677..8245118 100644
--- a/refs.c
+++ b/refs.c
@@ -174,3 +174,121 @@ int delete_ref(const char *refname, const unsigned char 

[PATCH v4 01/26] refs.c: create a public version of verify_refname_available

[David Turner]

From: Ronnie Sahlberg 

Create a public version of verify_refname_available that backends can
provide.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs.c | 65 ++---
 refs.h | 20 
 2 files changed, 50 insertions(+), 35 deletions(-)

diff --git a/refs.c b/refs.c
index 132eff5..fe71ea0 100644
--- a/refs.c
+++ b/refs.c
@@ -279,7 +279,7 @@ struct ref_dir {
  * presence of an empty subdirectory does not block the creation of a
  * similarly-named reference.  (The fact that reference names with the
  * same leading components can conflict *with each other* is a
- * separate issue that is regulated by verify_refname_available().)
+ * separate issue that is regulated by verify_refname_available_dir().)
  *
  * Please note that the name field contains the fully-qualified
  * reference (or subdirectory) name.  Space could be saved by only
@@ -897,25 +897,13 @@ static int nonmatching_ref_fn(struct ref_entry *entry, 
void *vdata)
 /*
  * Return 0 if a reference named refname could be created without
  * conflicting with the name of an existing reference in dir.
- * Otherwise, return a negative value and write an explanation to err.
- * If extras is non-NULL, it is a list of additional refnames with
- * which refname is not allowed to conflict. If skip is non-NULL,
- * ignore potential conflicts with refs in skip (e.g., because they
- * are scheduled for deletion in the same operation). Behavior is
- * undefined if the same name is listed in both extras and skip.
- *
- * Two reference names conflict if one of them exactly matches the
- * leading components of the other; e.g., "refs/foo/bar" conflicts
- * with both "refs/foo" and with "refs/foo/bar/baz" but not with
- * "refs/foo/bar" or "refs/foo/barbados".
- *
- * extras and skip must be sorted.
+ * See verify_refname_available for details.
  */
-static int verify_refname_available(const char *refname,
-   const struct string_list *extras,
-   const struct string_list *skip,
-   struct ref_dir *dir,
-   struct strbuf *err)
+static int verify_refname_available_dir(const char *refname,
+   const struct string_list *extras,
+   const struct string_list *skip,
+   struct ref_dir *dir,
+   struct strbuf *err)
 {
const char *slash;
int pos;
@@ -2464,9 +2452,12 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 */
strbuf_git_path(_ref_file, "%s", orig_refname);
if (remove_empty_directories(_ref_file)) {
+   struct ref_dir *loose_refs;
+   loose_refs = get_loose_refs(_cache);
last_errno = errno;
-   if (!verify_refname_available(orig_refname, extras, 
skip,
- 
get_loose_refs(_cache), err))
+   if (!verify_refname_available_dir(orig_refname, extras,
+ skip, loose_refs,
+ err))
strbuf_addf(err, "there are still refs under 
'%s'",
orig_refname);
goto error_return;
@@ -2479,8 +2470,9 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
if (!refname) {
last_errno = errno;
if (last_errno != ENOTDIR ||
-   !verify_refname_available(orig_refname, extras, skip,
- get_loose_refs(_cache), err))
+   !verify_refname_available_dir(orig_refname, extras, skip,
+ get_loose_refs(_cache),
+ err))
strbuf_addf(err, "unable to resolve reference %s: %s",
orig_refname, strerror(last_errno));
 
@@ -2493,8 +2485,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * our refname.
 */
if (is_null_oid(>old_oid) &&
-   verify_refname_available(refname, extras, skip,
-get_packed_refs(_cache), err)) {
+   verify_refname_available_dir(refname, extras, skip,
+get_packed_refs(_cache), err)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -3127,10 +3119,7 @@ static int rename_ref_available(const char *oldname, 
const char *newname)

[PATCH v4 21/26] refs.c: move peel_object to the common code

[David Turner]

This function does not contain any backend specific code so we
move it to the common code.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 53 -
 refs.c  | 23 +++
 refs.h  | 34 ++
 3 files changed, 57 insertions(+), 53 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 4b6bf29..ef600d8 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1600,59 +1600,6 @@ const char *resolve_ref_unsafe(const char *refname, int 
resolve_flags,
return ret;
 }
 
-enum peel_status {
-   /* object was peeled successfully: */
-   PEEL_PEELED = 0,
-
-   /*
-* object cannot be peeled because the named object (or an
-* object referred to by a tag in the peel chain), does not
-* exist.
-*/
-   PEEL_INVALID = -1,
-
-   /* object cannot be peeled because it is not a tag: */
-   PEEL_NON_TAG = -2,
-
-   /* ref_entry contains no peeled value because it is a symref: */
-   PEEL_IS_SYMREF = -3,
-
-   /*
-* ref_entry cannot be peeled because it is broken (i.e., the
-* symbolic reference cannot even be resolved to an object
-* name):
-*/
-   PEEL_BROKEN = -4
-};
-
-/*
- * Peel the named object; i.e., if the object is a tag, resolve the
- * tag recursively until a non-tag is found.  If successful, store the
- * result to sha1 and return PEEL_PEELED.  If the object is not a tag
- * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
- * and leave sha1 unchanged.
- */
-static enum peel_status peel_object(const unsigned char *name, unsigned char 
*sha1)
-{
-   struct object *o = lookup_unknown_object(name);
-
-   if (o->type == OBJ_NONE) {
-   int type = sha1_object_info(name, NULL);
-   if (type < 0 || !object_as_type(o, type, 0))
-   return PEEL_INVALID;
-   }
-
-   if (o->type != OBJ_TAG)
-   return PEEL_NON_TAG;
-
-   o = deref_tag_noverify(o);
-   if (!o)
-   return PEEL_INVALID;
-
-   hashcpy(sha1, o->sha1);
-   return PEEL_PEELED;
-}
-
 /*
  * Peel the entry (if possible) and return its new peel_status.  If
  * repeel is true, re-peel the entry even if there is an old peeled
diff --git a/refs.c b/refs.c
index 16280d5..e7f6c77 100644
--- a/refs.c
+++ b/refs.c
@@ -4,6 +4,8 @@
 #include "cache.h"
 #include "refs.h"
 #include "lockfile.h"
+#include "object.h"
+#include "tag.h"
 
 static int is_per_worktree_ref(const char *refname)
 {
@@ -995,3 +997,24 @@ int refname_is_safe(const char *refname)
}
return 1;
 }
+
+enum peel_status peel_object(const unsigned char *name, unsigned char *sha1)
+{
+   struct object *o = lookup_unknown_object(name);
+
+   if (o->type == OBJ_NONE) {
+   int type = sha1_object_info(name, NULL);
+   if (type < 0 || !object_as_type(o, type, 0))
+   return PEEL_INVALID;
+   }
+
+   if (o->type != OBJ_TAG)
+   return PEEL_NON_TAG;
+
+   o = deref_tag_noverify(o);
+   if (!o)
+   return PEEL_INVALID;
+
+   hashcpy(sha1, o->sha1);
+   return PEEL_PEELED;
+}
diff --git a/refs.h b/refs.h
index fc0611d..3fb480b 100644
--- a/refs.h
+++ b/refs.h
@@ -76,6 +76,40 @@ extern int is_branch(const char *refname);
  */
 extern int peel_ref(const char *refname, unsigned char *sha1);
 
+enum peel_status {
+   /* object was peeled successfully: */
+   PEEL_PEELED = 0,
+
+   /*
+* object cannot be peeled because the named object (or an
+* object referred to by a tag in the peel chain), does not
+* exist.
+*/
+   PEEL_INVALID = -1,
+
+   /* object cannot be peeled because it is not a tag: */
+   PEEL_NON_TAG = -2,
+
+   /* ref_entry contains no peeled value because it is a symref: */
+   PEEL_IS_SYMREF = -3,
+
+   /*
+* ref_entry cannot be peeled because it is broken (i.e., the
+* symbolic reference cannot even be resolved to an object
+* name):
+*/
+   PEEL_BROKEN = -4
+};
+
+/*
+ * Peel the named object; i.e., if the object is a tag, resolve the
+ * tag recursively until a non-tag is found.  If successful, store the
+ * result to sha1 and return PEEL_PEELED.  If the object is not a tag
+ * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
+ * and leave sha1 unchanged.
+ */
+enum peel_status peel_object(const unsigned char *name, unsigned char *sha1);
+
 /**
  * Resolve refname in the nested "gitlink" repository that is located
  * at path.  If the resolution is successful, return 0 and set sha1 to
-- 
2.4.2.644.g97b850b-twtrsrc

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a 

[PATCH v4 05/26] refs.c: move update_ref to refs.c

[David Turner]

From: Ronnie Sahlberg 

Move update_ref() to the refs.c file since this function does not
contain any backend specific code.  Move the ref classifier functions
as well, since update_ref depends on them.

Based on Ronnie Sahlberg's patch

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 117 +---
 refs.c  | 116 +++
 2 files changed, 117 insertions(+), 116 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 84abc82..72a9bc4 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2675,8 +2675,6 @@ struct pack_refs_cb_data {
struct ref_to_prune *ref_to_prune;
 };
 
-static int is_per_worktree_ref(const char *refname);
-
 /*
  * An each_ref_entry_fn that is run over loose references only.  If
  * the loose reference can be packed, add an entry in the packed ref
@@ -2691,7 +2689,7 @@ static int pack_if_possible_fn(struct ref_entry *entry, 
void *cb_data)
int is_tag_ref = starts_with(entry->name, "refs/tags/");
 
/* Do not pack per-worktree refs: */
-   if (is_per_worktree_ref(entry->name))
+   if (ref_type(entry->name) == REF_TYPE_PER_WORKTREE)
return 0;
 
/* ALWAYS pack tags */
@@ -2887,77 +2885,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-static int is_per_worktree_ref(const char *refname)
-{
-   return !strcmp(refname, "HEAD") ||
-   starts_with(refname, "refs/bisect/");
-}
-
-static int is_pseudoref_syntax(const char *refname)
-{
-   const char *c;
-
-   for (c = refname; *c; c++) {
-   if (!isupper(*c) && *c != '-' && *c != '_')
-   return 0;
-   }
-
-   return 1;
-}
-
-enum ref_type ref_type(const char *refname)
-{
-   if (is_per_worktree_ref(refname))
-   return REF_TYPE_PER_WORKTREE;
-   if (is_pseudoref_syntax(refname))
-   return REF_TYPE_PSEUDOREF;
-   return REF_TYPE_NORMAL;
-}
-
-static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
-  const unsigned char *old_sha1, struct strbuf *err)
-{
-   const char *filename;
-   int fd;
-   static struct lock_file lock;
-   struct strbuf buf = STRBUF_INIT;
-   int ret = -1;
-
-   strbuf_addf(, "%s\n", sha1_to_hex(sha1));
-
-   filename = git_path("%s", pseudoref);
-   fd = hold_lock_file_for_update(, filename, LOCK_DIE_ON_ERROR);
-   if (fd < 0) {
-   strbuf_addf(err, "Could not open '%s' for writing: %s",
-   filename, strerror(errno));
-   return -1;
-   }
-
-   if (old_sha1) {
-   unsigned char actual_old_sha1[20];
-
-   if (read_ref(pseudoref, actual_old_sha1))
-   die("could not read ref '%s'", pseudoref);
-   if (hashcmp(actual_old_sha1, old_sha1)) {
-   strbuf_addf(err, "Unexpected sha1 when writing %s", 
pseudoref);
-   rollback_lock_file();
-   goto done;
-   }
-   }
-
-   if (write_in_full(fd, buf.buf, buf.len) != buf.len) {
-   strbuf_addf(err, "Could not write to '%s'", filename);
-   rollback_lock_file();
-   goto done;
-   }
-
-   commit_lock_file();
-   ret = 0;
-done:
-   strbuf_release();
-   return ret;
-}
-
 static int delete_pseudoref(const char *pseudoref, const unsigned char 
*old_sha1)
 {
static struct lock_file lock;
@@ -4106,48 +4033,6 @@ int ref_transaction_verify(struct ref_transaction 
*transaction,
  flags, NULL, err);
 }
 
-int update_ref(const char *msg, const char *refname,
-  const unsigned char *new_sha1, const unsigned char *old_sha1,
-  unsigned int flags, enum action_on_err onerr)
-{
-   struct ref_transaction *t = NULL;
-   struct strbuf err = STRBUF_INIT;
-   int ret = 0;
-
-   if (ref_type(refname) == REF_TYPE_PSEUDOREF) {
-   ret = write_pseudoref(refname, new_sha1, old_sha1, );
-   } else {
-   t = ref_transaction_begin();
-   if (!t ||
-   ref_transaction_update(t, refname, new_sha1, old_sha1,
-  flags, msg, ) ||
-   ref_transaction_commit(t, )) {
-   ret = 1;
-   ref_transaction_free(t);
-   }
-   }
-   if (ret) {
-   const char *str = "update_ref failed for ref '%s': %s";
-
-   switch (onerr) {
-   case UPDATE_REFS_MSG_ON_ERR:
-   error(str, refname, err.buf);
-   break;
-   

[PATCH v4 26/26] introduce "extensions" form of core.repositoryformatversion

[David Turner]

From: Jeff King 

Normally we try to avoid bumps of the whole-repository
core.repositoryformatversion field. However, it is
unavoidable if we want to safely change certain aspects of
git in a backwards-incompatible way (e.g., modifying the set
of ref tips that we must traverse to generate a list of
unreachable, safe-to-prune objects).

If we were to bump the repository version for every such
change, then any implementation understanding version `X`
would also have to understand `X-1`, `X-2`, and so forth,
even though the incompatibilities may be in orthogonal parts
of the system, and there is otherwise no reason we cannot
implement one without the other (or more importantly, that
the user cannot choose to use one feature without the other,
weighing the tradeoff in compatibility only for that
particular feature).

This patch documents the existing repositoryformatversion
strategy and introduces a new format, "1", which lets a
repository specify that it must run with an arbitrary set of
extensions. This can be used, for example:

 - to inform git that the objects should not be pruned based
   only on the reachability of the ref tips (e.g, because it
   has "clone --shared" children)

 - that the refs are stored in a format besides the usual
   "refs" and "packed-refs" directories

Because we bump to format "1", and because format "1"
requires that a running git knows about any extensions
mentioned, we know that older versions of the code will not
do something dangerous when confronted with these new
formats.

For example, if the user chooses to use database storage for
refs, they may set the "extensions.refbackend" config to
"db". Older versions of git will not understand format "1"
and bail. Versions of git which understand "1" but do not
know about "refbackend", or which know about "refbackend"
but not about the "db" backend, will refuse to run. This is
annoying, of course, but much better than the alternative of
claiming that there are no refs in the repository, or
writing to a location that other implementations will not
read.

Note that we are only defining the rules for format 1 here.
We do not ever write format 1 ourselves; it is a tool that
is meant to be used by users and future extensions to
provide safety with older implementations.

Signed-off-by: Jeff King 
---
 Documentation/technical/repository-version.txt | 81 ++
 cache.h|  6 ++
 setup.c| 37 +++-
 t/t1302-repo-version.sh| 38 
 4 files changed, 159 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/technical/repository-version.txt

diff --git a/Documentation/technical/repository-version.txt 
b/Documentation/technical/repository-version.txt
new file mode 100644
index 000..3d7106d
--- /dev/null
+++ b/Documentation/technical/repository-version.txt
@@ -0,0 +1,81 @@
+Git Repository Format Versions
+==
+
+Every git repository is marked with a numeric version in the
+`core.repositoryformatversion` key of its `config` file. This version
+specifies the rules for operating on the on-disk repository data. An
+implementation of git which does not understand a particular version
+advertised by an on-disk repository MUST NOT operate on that repository;
+doing so risks not only producing wrong results, but actually losing
+data.
+
+Because of this rule, version bumps should be kept to an absolute
+minimum. Instead, we generally prefer these strategies:
+
+  - bumping format version numbers of individual data files (e.g.,
+index, packfiles, etc). This restricts the incompatibilities only to
+those files.
+
+  - introducing new data that gracefully degrades when used by older
+clients (e.g., pack bitmap files are ignored by older clients, which
+simply do not take advantage of the optimization they provide).
+
+A whole-repository format version bump should only be part of a change
+that cannot be independently versioned. For instance, if one were to
+change the reachability rules for objects, or the rules for locking
+refs, that would require a bump of the repository format version.
+
+Note that this applies only to accessing the repository's disk contents
+directly. An older client which understands only format `0` may still
+connect via `git://` to a repository using format `1`, as long as the
+server process understands format `1`.
+
+The preferred strategy for rolling out a version bump (whether whole
+repository or for a single file) is to teach git to read the new format,
+and allow writing the new format with a config switch or command line
+option (for experimentation or for those who do not care about backwards
+compatibility with older gits). Then after a long period to allow the
+reading capability to become common, we may switch to writing the new
+format by default.
+
+The currently defined format versions are:

[PATCH v4 24/26] refs: make files_log_ref_write functions public

[David Turner]

Because HEAD and stash are per-worktree, refs.c needs to go through
the files backend to write these refs.

In this patch, we make one files backend internal functions
public. Later, we will use this to handle reflog updates for
per-worktree symbolic refs (HEAD).

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 8 
 refs.h  | 5 +
 2 files changed, 13 insertions(+)

diff --git a/refs-be-files.c b/refs-be-files.c
index 7c39afa..1bda3e4 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2733,6 +2733,14 @@ static int log_ref_write(const char *refname, const 
unsigned char *old_sha1,
 const unsigned char *new_sha1, const char *msg,
 int flags, struct strbuf *err)
 {
+   return files_log_ref_write(refname, old_sha1, new_sha1, msg, flags,
+  err);
+}
+
+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+   const unsigned char *new_sha1, const char *msg,
+   int flags, struct strbuf *err)
+{
struct strbuf sb = STRBUF_INIT;
int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, , flags,
  err);
diff --git a/refs.h b/refs.h
index 3fce9c9..7aed0a2 100644
--- a/refs.h
+++ b/refs.h
@@ -619,6 +619,11 @@ enum ref_type ref_type(const char *refname);
  */
 int copy_reflog_msg(char *buf, const char *msg);
 
+int files_log_ref_write(const char *refname, const unsigned char *old_sha1,
+   const unsigned char *new_sha1, const char *msg,
+   int flags, struct strbuf *err);
+
+
 enum expire_reflog_flags {
EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 08/26] refs.c: move the hidden refs functions to the common code

[David Turner]

From: Ronnie Sahlberg 

Move the hidden refs functions to the refs.c file since these
functions do not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 51 ---
 refs.c  | 51 +++
 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 04c3206..dad23c7 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -4215,57 +4215,6 @@ char *shorten_unambiguous_ref(const char *refname, int 
strict)
return xstrdup(refname);
 }
 
-static struct string_list *hide_refs;
-
-int parse_hide_refs_config(const char *var, const char *value, const char 
*section)
-{
-   if (!strcmp("transfer.hiderefs", var) ||
-   /* NEEDSWORK: use parse_config_key() once both are merged */
-   (starts_with(var, section) && var[strlen(section)] == '.' &&
-!strcmp(var + strlen(section), ".hiderefs"))) {
-   char *ref;
-   int len;
-
-   if (!value)
-   return config_error_nonbool(var);
-   ref = xstrdup(value);
-   len = strlen(ref);
-   while (len && ref[len - 1] == '/')
-   ref[--len] = '\0';
-   if (!hide_refs) {
-   hide_refs = xcalloc(1, sizeof(*hide_refs));
-   hide_refs->strdup_strings = 1;
-   }
-   string_list_append(hide_refs, ref);
-   }
-   return 0;
-}
-
-int ref_is_hidden(const char *refname)
-{
-   int i;
-
-   if (!hide_refs)
-   return 0;
-   for (i = hide_refs->nr - 1; i >= 0; i--) {
-   const char *match = hide_refs->items[i].string;
-   int neg = 0;
-   int len;
-
-   if (*match == '!') {
-   neg = 1;
-   match++;
-   }
-
-   if (!starts_with(refname, match))
-   continue;
-   len = strlen(match);
-   if (!refname[len] || refname[len] == '/')
-   return !neg;
-   }
-   return 0;
-}
-
 struct expire_reflog_cb {
unsigned int flags;
reflog_expiry_should_prune_fn *should_prune_fn;
diff --git a/refs.c b/refs.c
index 8245118..6e5c8f8 100644
--- a/refs.c
+++ b/refs.c
@@ -292,3 +292,54 @@ int read_ref_at(const char *refname, unsigned int flags, 
unsigned long at_time,
 
return 1;
 }
+
+static struct string_list *hide_refs;
+
+int parse_hide_refs_config(const char *var, const char *value, const char 
*section)
+{
+   if (!strcmp("transfer.hiderefs", var) ||
+   /* NEEDSWORK: use parse_config_key() once both are merged */
+   (starts_with(var, section) && var[strlen(section)] == '.' &&
+!strcmp(var + strlen(section), ".hiderefs"))) {
+   char *ref;
+   int len;
+
+   if (!value)
+   return config_error_nonbool(var);
+   ref = xstrdup(value);
+   len = strlen(ref);
+   while (len && ref[len - 1] == '/')
+   ref[--len] = '\0';
+   if (!hide_refs) {
+   hide_refs = xcalloc(1, sizeof(*hide_refs));
+   hide_refs->strdup_strings = 1;
+   }
+   string_list_append(hide_refs, ref);
+   }
+   return 0;
+}
+
+int ref_is_hidden(const char *refname)
+{
+   int i;
+
+   if (!hide_refs)
+   return 0;
+   for (i = hide_refs->nr - 1; i >= 0; i--) {
+   const char *match = hide_refs->items[i].string;
+   int neg = 0;
+   int len;
+
+   if (*match == '!') {
+   neg = 1;
+   match++;
+   }
+
+   if (!starts_with(refname, match))
+   continue;
+   len = strlen(match);
+   if (!refname[len] || refname[len] == '/')
+   return !neg;
+   }
+   return 0;
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 13/26] refs.c: move check_refname_format to the common code

[David Turner]

From: Ronnie Sahlberg 

This function does not contain any backend specific code so we
move it to the common code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 109 
 refs.c  | 109 
 2 files changed, 109 insertions(+), 109 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 589e10d..345bee7 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -14,27 +14,6 @@ struct ref_lock {
 };
 
 /*
- * How to handle various characters in refnames:
- * 0: An acceptable character for refs
- * 1: End-of-component
- * 2: ., look for a preceding . to reject .. in refs
- * 3: {, look for a preceding @ to reject @{ in refs
- * 4: A bad character: ASCII control characters, and
- *":", "?", "[", "\", "^", "~", SP, or TAB
- * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set
- */
-static unsigned char refname_disposition[256] = {
-   1, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-   4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4, 4,
-   4, 0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 2, 1,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0, 0, 0, 4,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 4, 0, 4, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 3, 0, 0, 4, 4
-};
-
-/*
  * Flag passed to lock_ref_sha1_basic() telling it to tolerate broken
  * refs (i.e., because the reference is about to be deleted anyway).
  */
@@ -69,94 +48,6 @@ static unsigned char refname_disposition[256] = {
  * value to ref_update::flags
  */
 
-/*
- * Try to read one refname component from the front of refname.
- * Return the length of the component found, or -1 if the component is
- * not legal.  It is legal if it is something reasonable to have under
- * ".git/refs/"; We do not like it if:
- *
- * - any path component of it begins with ".", or
- * - it has double dots "..", or
- * - it has ASCII control characters, or
- * - it has ":", "?", "[", "\", "^", "~", SP, or TAB anywhere, or
- * - it has "*" anywhere unless REFNAME_REFSPEC_PATTERN is set, or
- * - it ends with a "/", or
- * - it ends with ".lock", or
- * - it contains a "@{" portion
- */
-static int check_refname_component(const char *refname, int *flags)
-{
-   const char *cp;
-   char last = '\0';
-
-   for (cp = refname; ; cp++) {
-   int ch = *cp & 255;
-   unsigned char disp = refname_disposition[ch];
-   switch (disp) {
-   case 1:
-   goto out;
-   case 2:
-   if (last == '.')
-   return -1; /* Refname contains "..". */
-   break;
-   case 3:
-   if (last == '@')
-   return -1; /* Refname contains "@{". */
-   break;
-   case 4:
-   return -1;
-   case 5:
-   if (!(*flags & REFNAME_REFSPEC_PATTERN))
-   return -1; /* refspec can't be a pattern */
-
-   /*
-* Unset the pattern flag so that we only accept
-* a single asterisk for one side of refspec.
-*/
-   *flags &= ~ REFNAME_REFSPEC_PATTERN;
-   break;
-   }
-   last = ch;
-   }
-out:
-   if (cp == refname)
-   return 0; /* Component has zero length. */
-   if (refname[0] == '.')
-   return -1; /* Component starts with '.'. */
-   if (cp - refname >= LOCK_SUFFIX_LEN &&
-   !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN))
-   return -1; /* Refname ends with ".lock". */
-   return cp - refname;
-}
-
-int check_refname_format(const char *refname, int flags)
-{
-   int component_len, component_count = 0;
-
-   if (!strcmp(refname, "@"))
-   /* Refname is a single character '@'. */
-   return -1;
-
-   while (1) {
-   /* We are at the start of a path component. */
-   component_len = check_refname_component(refname, );
-   if (component_len <= 0)
-   return -1;
-
-   component_count++;
-   if (refname[component_len] == '\0')
-   break;
-   /* Skip to next component. */
-   refname += component_len + 1;
-   }
-
-   if (refname[component_len - 1] == '.')
-   return -1; /* Refname ends with '.'. */
-   if (!(flags & REFNAME_ALLOW_ONELEVEL) && component_count < 2)
-   return -1; /* Refname has only 

[PATCH v4 04/26] refs.c: add a new refs.c file to hold all common refs code

[David Turner]

From: Ronnie Sahlberg 

Create a new refs.c file that will be used to hold all the refs
code that is backend agnostic and will be shared across all backends.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 Makefile | 1 +
 refs.c   | 3 +++
 2 files changed, 4 insertions(+)
 create mode 100644 refs.c

diff --git a/Makefile b/Makefile
index 173b9d4..b37eae3 100644
--- a/Makefile
+++ b/Makefile
@@ -763,6 +763,7 @@ LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs-be-files.o
+LIB_OBJS += refs.o
 LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
diff --git a/refs.c b/refs.c
new file mode 100644
index 000..77492ff
--- /dev/null
+++ b/refs.c
@@ -0,0 +1,3 @@
+/*
+ * Common refs code for all backends.
+ */
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 15/26] refs.c: move prettify_refname to the common code

[David Turner]

From: Ronnie Sahlberg 

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 9 -
 refs.c  | 9 +
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 5c6c743..c8b44ab 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2052,15 +2052,6 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
   DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
-const char *prettify_refname(const char *name)
-{
-   return name + (
-   starts_with(name, "refs/heads/") ? 11 :
-   starts_with(name, "refs/tags/") ? 10 :
-   starts_with(name, "refs/remotes/") ? 13 :
-   0);
-}
-
 static void unlock_ref(struct ref_lock *lock)
 {
/* Do not free lock->lk -- atexit() still looks at them */
diff --git a/refs.c b/refs.c
index f2f8052..642a2ab 100644
--- a/refs.c
+++ b/refs.c
@@ -737,3 +737,12 @@ int is_branch(const char *refname)
 {
return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
 }
+
+const char *prettify_refname(const char *name)
+{
+   return name + (
+   starts_with(name, "refs/heads/") ? 11 :
+   starts_with(name, "refs/tags/") ? 10 :
+   starts_with(name, "refs/remotes/") ? 13 :
+   0);
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 20/26] refs.c: move copy_msg to the common code

[David Turner]

Rename copy_msg to copy_reflog_msg and make it public.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 28 +---
 refs.c  | 21 +
 refs.h  |  7 +++
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 8d966f1..4b6bf29 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2654,32 +2654,6 @@ static int commit_ref(struct ref_lock *lock)
return 0;
 }
 
-/*
- * copy the reflog message msg to buf, which has been allocated sufficiently
- * large, while cleaning up the whitespaces.  Especially, convert LF to space,
- * because reflog file is one line per entry.
- */
-static int copy_msg(char *buf, const char *msg)
-{
-   char *cp = buf;
-   char c;
-   int wasspace = 1;
-
-   *cp++ = '\t';
-   while ((c = *msg++)) {
-   if (wasspace && isspace(c))
-   continue;
-   wasspace = isspace(c);
-   if (wasspace)
-   c = ' ';
-   *cp++ = c;
-   }
-   while (buf < cp && isspace(cp[-1]))
-   cp--;
-   *cp++ = '\n';
-   return cp - buf;
-}
-
 static int should_autocreate_reflog(const char *refname)
 {
if (!log_all_ref_updates)
@@ -2774,7 +2748,7 @@ static int log_ref_write_fd(int fd, const unsigned char 
*old_sha1,
sha1_to_hex(new_sha1),
committer);
if (msglen)
-   len += copy_msg(logrec + len - 1, msg) - 1;
+   len += copy_reflog_msg(logrec + len - 1, msg) - 1;
 
written = len <= maxlen ? write_in_full(fd, logrec, len) : -1;
free(logrec);
diff --git a/refs.c b/refs.c
index 3eb3a28..16280d5 100644
--- a/refs.c
+++ b/refs.c
@@ -828,6 +828,27 @@ int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn fn, void *c
return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, 
cb_data);
 }
 
+int copy_reflog_msg(char *buf, const char *msg)
+{
+   char *cp = buf;
+   char c;
+   int wasspace = 1;
+
+   *cp++ = '\t';
+   while ((c = *msg++)) {
+   if (wasspace && isspace(c))
+   continue;
+   wasspace = isspace(c);
+   if (wasspace)
+   c = ' ';
+   *cp++ = c;
+   }
+   while (buf < cp && isspace(cp[-1]))
+   cp--;
+   *cp++ = '\n';
+   return cp - buf;
+}
+
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/refs.h b/refs.h
index be94e18..fc0611d 100644
--- a/refs.h
+++ b/refs.h
@@ -576,6 +576,13 @@ enum ref_type {
 
 enum ref_type ref_type(const char *refname);
 
+/*
+ * Copy the reflog message msg to buf, which has been allocated sufficiently
+ * large, while cleaning up the whitespaces.  Especially, convert LF to space,
+ * because reflog file is one line per entry.
+ */
+int copy_reflog_msg(char *buf, const char *msg);
+
 enum expire_reflog_flags {
EXPIRE_REFLOGS_DRY_RUN = 1 << 0,
EXPIRE_REFLOGS_UPDATE_REF = 1 << 1,
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 06/26] refs.c: move delete_pseudoref and delete_ref to the common code

[David Turner]

From: Ronnie Sahlberg 

Move delete_pseudoref() and delete_ref() to the refs.c file since
these functions do not contain any backend specific code.  We can't
move delete_refs yet because it depends on the files-backend-specific
repack_without_refs.

Based on a patch by Ronnie Sahlberg.

Signed-off-by: David Turner 
Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 56 
 refs.c  | 57 +
 2 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 72a9bc4..d969066 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2885,62 +2885,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-static int delete_pseudoref(const char *pseudoref, const unsigned char 
*old_sha1)
-{
-   static struct lock_file lock;
-   const char *filename;
-
-   filename = git_path("%s", pseudoref);
-
-   if (old_sha1 && !is_null_sha1(old_sha1)) {
-   int fd;
-   unsigned char actual_old_sha1[20];
-
-   fd = hold_lock_file_for_update(, filename,
-  LOCK_DIE_ON_ERROR);
-   if (fd < 0)
-   die_errno(_("Could not open '%s' for writing"), 
filename);
-   if (read_ref(pseudoref, actual_old_sha1))
-   die("could not read ref '%s'", pseudoref);
-   if (hashcmp(actual_old_sha1, old_sha1)) {
-   warning("Unexpected sha1 when deleting %s", pseudoref);
-   rollback_lock_file();
-   return -1;
-   }
-
-   unlink(filename);
-   rollback_lock_file();
-   } else {
-   unlink(filename);
-   }
-
-   return 0;
-}
-
-int delete_ref(const char *refname, const unsigned char *old_sha1,
-  unsigned int flags)
-{
-   struct ref_transaction *transaction;
-   struct strbuf err = STRBUF_INIT;
-
-   if (ref_type(refname) == REF_TYPE_PSEUDOREF)
-   return delete_pseudoref(refname, old_sha1);
-
-   transaction = ref_transaction_begin();
-   if (!transaction ||
-   ref_transaction_delete(transaction, refname, old_sha1,
-  flags, NULL, ) ||
-   ref_transaction_commit(transaction, )) {
-   error("%s", err.buf);
-   ref_transaction_free(transaction);
-   strbuf_release();
-   return 1;
-   }
-   ref_transaction_free(transaction);
-   strbuf_release();
-   return 0;
-}
-
 int delete_refs(struct string_list *refnames)
 {
struct strbuf err = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index 610fab0..f4f0677 100644
--- a/refs.c
+++ b/refs.c
@@ -117,3 +117,60 @@ int update_ref(const char *msg, const char *refname,
ref_transaction_free(t);
return 0;
 }
+
+
+static int delete_pseudoref(const char *pseudoref, const unsigned char 
*old_sha1)
+{
+   static struct lock_file lock;
+   const char *filename;
+
+   filename = git_path("%s", pseudoref);
+
+   if (old_sha1 && !is_null_sha1(old_sha1)) {
+   int fd;
+   unsigned char actual_old_sha1[20];
+
+   fd = hold_lock_file_for_update(, filename,
+  LOCK_DIE_ON_ERROR);
+   if (fd < 0)
+   die_errno(_("Could not open '%s' for writing"), 
filename);
+   if (read_ref(pseudoref, actual_old_sha1))
+   die("could not read ref '%s'", pseudoref);
+   if (hashcmp(actual_old_sha1, old_sha1)) {
+   warning("Unexpected sha1 when deleting %s", pseudoref);
+   rollback_lock_file();
+   return -1;
+   }
+
+   unlink(filename);
+   rollback_lock_file();
+   } else {
+   unlink(filename);
+   }
+
+   return 0;
+}
+
+int delete_ref(const char *refname, const unsigned char *old_sha1,
+  unsigned int flags)
+{
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
+
+   if (ref_type(refname) == REF_TYPE_PSEUDOREF)
+   return delete_pseudoref(refname, old_sha1);
+
+   transaction = ref_transaction_begin();
+   if (!transaction ||
+   ref_transaction_delete(transaction, refname, old_sha1,
+  flags, NULL, ) ||
+   ref_transaction_commit(transaction, )) {
+   error("%s", err.buf);
+   ref_transaction_free(transaction);
+   strbuf_release();
+   return 1;
+   }
+   

[PATCH v4 11/26] refs.c: move read_ref, read_ref_full and ref_exists to the common code

[David Turner]

From: Ronnie Sahlberg 

These functions do not depend on the backend implementation so we
move them to the common code.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 18 --
 refs.c  | 18 ++
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 9c53f54..728751a 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1791,24 +1791,6 @@ struct ref_filter {
void *cb_data;
 };
 
-int read_ref_full(const char *refname, int resolve_flags, unsigned char *sha1, 
int *flags)
-{
-   if (resolve_ref_unsafe(refname, resolve_flags, sha1, flags))
-   return 0;
-   return -1;
-}
-
-int read_ref(const char *refname, unsigned char *sha1)
-{
-   return read_ref_full(refname, RESOLVE_REF_READING, sha1, NULL);
-}
-
-int ref_exists(const char *refname)
-{
-   unsigned char sha1[20];
-   return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);
-}
-
 static int filter_refs(const char *refname, const struct object_id *oid,
   int flags, void *data)
 {
diff --git a/refs.c b/refs.c
index fab7603..8a619a6 100644
--- a/refs.c
+++ b/refs.c
@@ -598,3 +598,21 @@ void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, 
const struct string_li
data.msg_fmt = msg_fmt;
for_each_rawref(warn_if_dangling_symref, );
 }
+
+int read_ref_full(const char *refname, int resolve_flags, unsigned char *sha1, 
int *flags)
+{
+   if (resolve_ref_unsafe(refname, resolve_flags, sha1, flags))
+   return 0;
+   return -1;
+}
+
+int read_ref(const char *refname, unsigned char *sha1)
+{
+   return read_ref_full(refname, RESOLVE_REF_READING, sha1, NULL);
+}
+
+int ref_exists(const char *refname)
+{
+   unsigned char sha1[20];
+   return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 22/26] refs.c: move should_autocreate_reflog to common code

[David Turner]

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 10 --
 refs.c  | 10 ++
 refs.h  |  2 ++
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index ef600d8..7c39afa 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2601,16 +2601,6 @@ static int commit_ref(struct ref_lock *lock)
return 0;
 }
 
-static int should_autocreate_reflog(const char *refname)
-{
-   if (!log_all_ref_updates)
-   return 0;
-   return starts_with(refname, "refs/heads/") ||
-   starts_with(refname, "refs/remotes/") ||
-   starts_with(refname, "refs/notes/") ||
-   !strcmp(refname, "HEAD");
-}
-
 int verify_refname_available(const char *newname, struct string_list *extra,
 struct string_list *skip, struct strbuf *err)
 {
diff --git a/refs.c b/refs.c
index e7f6c77..efc1c47 100644
--- a/refs.c
+++ b/refs.c
@@ -626,6 +626,16 @@ char *resolve_refdup(const char *refname, int 
resolve_flags,
  sha1, flags));
 }
 
+int should_autocreate_reflog(const char *refname)
+{
+   if (!log_all_ref_updates)
+   return 0;
+   return starts_with(refname, "refs/heads/") ||
+   starts_with(refname, "refs/remotes/") ||
+   starts_with(refname, "refs/notes/") ||
+   !strcmp(refname, "HEAD");
+}
+
 /*
  * How to handle various characters in refnames:
  * 0: An acceptable character for refs
diff --git a/refs.h b/refs.h
index 3fb480b..3fce9c9 100644
--- a/refs.h
+++ b/refs.h
@@ -58,6 +58,8 @@ extern const char *resolve_ref_unsafe(const char *refname, 
int resolve_flags,
 extern char *resolve_refdup(const char *refname, int resolve_flags,
unsigned char *sha1, int *flags);
 
+extern int should_autocreate_reflog(const char *refname);
+
 extern int read_ref_full(const char *refname, int resolve_flags,
 unsigned char *sha1, int *flags);
 extern int read_ref(const char *refname, unsigned char *sha1);
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 25/26] refs: break out ref conflict checks

[David Turner]

Create new function verify_no_descendants, to hold one of the ref
conflict checks used in verify_refname_available.  Multiple backends
will need this function, so it goes in the common code.

rename_ref_available also moves to the common code, because alternate
backends might need it and it has no files-backend-specific code.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 refs-be-files.c | 49 -
 refs.c  | 38 ++
 refs.h  | 15 +++
 3 files changed, 61 insertions(+), 41 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 1bda3e4..1f17348 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -729,6 +729,7 @@ static int verify_refname_available_dir(const char *refname,
struct strbuf *err)
 {
const char *slash;
+   const char *extra_refname;
int pos;
struct strbuf dirname = STRBUF_INIT;
int ret = -1;
@@ -834,33 +835,15 @@ static int verify_refname_available_dir(const char 
*refname,
}
}
 
-   if (extras) {
-   /*
-* Check for entries in extras that start with
-* "$refname/". We do that by looking for the place
-* where "$refname/" would be inserted in extras. If
-* there is an entry at that position that starts with
-* "$refname/" and is not in skip, then we have a
-* conflict.
-*/
-   for (pos = string_list_find_insert_index(extras, dirname.buf, 
0);
-pos < extras->nr; pos++) {
-   const char *extra_refname = extras->items[pos].string;
-
-   if (!starts_with(extra_refname, dirname.buf))
-   break;
-
-   if (!skip || !string_list_has_string(skip, 
extra_refname)) {
-   strbuf_addf(err, "cannot process '%s' and '%s' 
at the same time",
-   refname, extra_refname);
-   goto cleanup;
-   }
-   }
+   extra_refname = find_descendant_ref(dirname.buf, extras, skip);
+   if (extra_refname) {
+   strbuf_addf(err,
+   "cannot process '%s' and '%s' at the same time",
+   refname, extra_refname);
+   } else {
+   ret = 0;
}
 
-   /* No conflicts were found */
-   ret = 0;
-
 cleanup:
strbuf_release();
return ret;
@@ -2461,22 +2444,6 @@ out:
return ret;
 }
 
-static int rename_ref_available(const char *oldname, const char *newname)
-{
-   struct string_list skip = STRING_LIST_INIT_NODUP;
-   struct strbuf err = STRBUF_INIT;
-   int ret;
-
-   string_list_insert(, oldname);
-   ret = !verify_refname_available(newname, NULL, , );
-   if (!ret)
-   error("%s", err.buf);
-
-   string_list_clear(, 0);
-   strbuf_release();
-   return ret;
-}
-
 static int write_ref_to_lockfile(struct ref_lock *lock,
 const unsigned char *sha1, struct strbuf *err);
 static int commit_ref_update(struct ref_lock *lock,
diff --git a/refs.c b/refs.c
index efc1c47..72d96ed 100644
--- a/refs.c
+++ b/refs.c
@@ -1028,3 +1028,41 @@ enum peel_status peel_object(const unsigned char *name, 
unsigned char *sha1)
hashcpy(sha1, o->sha1);
return PEEL_PEELED;
 }
+
+const char *find_descendant_ref(const char *dirname,
+   const struct string_list *extras,
+   const struct string_list *skip)
+{
+   int pos;
+   if (!extras)
+   return NULL;
+
+   /* Look for the place where dirname would be inserted in extras. */
+   for (pos = string_list_find_insert_index(extras, dirname, 0);
+pos < extras->nr; pos++) {
+   const char *extra_refname = extras->items[pos].string;
+
+   if (!starts_with(extra_refname, dirname))
+   break;
+
+   if (!skip || !string_list_has_string(skip, extra_refname))
+   return extra_refname;
+   }
+   return NULL;
+}
+
+int rename_ref_available(const char *oldname, const char *newname)
+{
+   struct string_list skip = STRING_LIST_INIT_NODUP;
+   struct strbuf err = STRBUF_INIT;
+   int ret;
+
+   string_list_insert(, oldname);
+   ret = !verify_refname_available(newname, NULL, , );
+   if (!ret)
+   error("%s", err.buf);
+
+   string_list_clear(, 0);
+   strbuf_release();
+   return ret;
+}
diff --git a/refs.h b/refs.h
index 7aed0a2..69fa4df 100644
--- a/refs.h
+++ b/refs.h
@@ -362,6 +362,8 @@ int 

[PATCH v4 23/26] initdb: move safe_create_dir into common code

[David Turner]

In a moment, we'll create initdb functions for ref backends, and code
from initdb that calls this function needs to move into the files
backend.  So this function needs to be public.

Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
Signed-off-by: Michael Haggerty 
---
 builtin/init-db.c | 12 
 cache.h   |  5 +
 path.c| 12 
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/builtin/init-db.c b/builtin/init-db.c
index f59f407..07229d6 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -24,18 +24,6 @@ static int init_shared_repository = -1;
 static const char *init_db_template_dir;
 static const char *git_link;
 
-static void safe_create_dir(const char *dir, int share)
-{
-   if (mkdir(dir, 0777) < 0) {
-   if (errno != EEXIST) {
-   perror(dir);
-   exit(1);
-   }
-   }
-   else if (share && adjust_shared_perm(dir))
-   die(_("Could not make %s writable by group"), dir);
-}
-
 static void copy_templates_1(struct strbuf *path, struct strbuf *template,
 DIR *dir)
 {
diff --git a/cache.h b/cache.h
index 9a905a8..1d8a051 100644
--- a/cache.h
+++ b/cache.h
@@ -1737,4 +1737,9 @@ void stat_validity_update(struct stat_validity *sv, int 
fd);
 int versioncmp(const char *s1, const char *s2);
 void sleep_millisec(int millisec);
 
+/*
+ * Create a directory and (if share is nonzero) adjust its permissions
+ * according to the shared_repository setting.
+ */
+void safe_create_dir(const char *dir, int share);
 #endif /* CACHE_H */
diff --git a/path.c b/path.c
index 212695a..9e0283c 100644
--- a/path.c
+++ b/path.c
@@ -723,6 +723,18 @@ int adjust_shared_perm(const char *path)
return 0;
 }
 
+void safe_create_dir(const char *dir, int share)
+{
+   if (mkdir(dir, 0777) < 0) {
+   if (errno != EEXIST) {
+   perror(dir);
+   exit(1);
+   }
+   }
+   else if (share && adjust_shared_perm(dir))
+   die(_("Could not make %s writable by group"), dir);
+}
+
 static int have_same_root(const char *path1, const char *path2)
 {
int is_abs1, is_abs2;
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 17/26] refs.c: move head_ref_namespaced to the common code

[David Turner]

From: Ronnie Sahlberg 

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 15 ---
 refs.c  | 15 +++
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 14319cb..ffc3813 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1939,21 +1939,6 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
   strlen(git_replace_ref_base), 0, cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
-{
-   struct strbuf buf = STRBUF_INIT;
-   int ret = 0;
-   struct object_id oid;
-   int flag;
-
-   strbuf_addf(, "%sHEAD", get_git_namespace());
-   if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, ))
-   ret = fn(buf.buf, , flag, cb_data);
-   strbuf_release();
-
-   return ret;
-}
-
 int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f7aa9b3..ae518a2 100644
--- a/refs.c
+++ b/refs.c
@@ -827,3 +827,18 @@ int for_each_remote_ref_submodule(const char *submodule, 
each_ref_fn fn, void *c
 {
return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, 
cb_data);
 }
+
+int head_ref_namespaced(each_ref_fn fn, void *cb_data)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int ret = 0;
+   struct object_id oid;
+   int flag;
+
+   strbuf_addf(, "%sHEAD", get_git_namespace());
+   if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, ))
+   ret = fn(buf.buf, , flag, cb_data);
+   strbuf_release();
+
+   return ret;
+}
-- 
2.4.2.644.g97b850b-twtrsrc

--
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 v4 09/26] refs.c: move dwim and friend functions to the common refs code

[David Turner]

From: Ronnie Sahlberg 

These functions do not contain any backend specific code so we move
them to the common code and share across all backends.

Signed-off-by: Ronnie Sahlberg 
Signed-off-by: David Turner 
Signed-off-by: Junio C Hamano 
---
 refs-be-files.c | 203 
 refs.c  | 203 
 2 files changed, 203 insertions(+), 203 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index dad23c7..3609fb7 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2247,30 +2247,6 @@ const char *prettify_refname(const char *name)
0);
 }
 
-static const char *ref_rev_parse_rules[] = {
-   "%.*s",
-   "refs/%.*s",
-   "refs/tags/%.*s",
-   "refs/heads/%.*s",
-   "refs/remotes/%.*s",
-   "refs/remotes/%.*s/HEAD",
-   NULL
-};
-
-int refname_match(const char *abbrev_name, const char *full_name)
-{
-   const char **p;
-   const int abbrev_name_len = strlen(abbrev_name);
-
-   for (p = ref_rev_parse_rules; *p; p++) {
-   if (!strcmp(full_name, mkpath(*p, abbrev_name_len, 
abbrev_name))) {
-   return 1;
-   }
-   }
-
-   return 0;
-}
-
 static void unlock_ref(struct ref_lock *lock)
 {
/* Do not free lock->lk -- atexit() still looks at them */
@@ -2323,92 +2299,6 @@ static int remove_empty_directories(struct strbuf *path)
 }
 
 /*
- * *string and *len will only be substituted, and *string returned (for
- * later free()ing) if the string passed in is a magic short-hand form
- * to name a branch.
- */
-static char *substitute_branch_name(const char **string, int *len)
-{
-   struct strbuf buf = STRBUF_INIT;
-   int ret = interpret_branch_name(*string, *len, );
-
-   if (ret == *len) {
-   size_t size;
-   *string = strbuf_detach(, );
-   *len = size;
-   return (char *)*string;
-   }
-
-   return NULL;
-}
-
-int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
-{
-   char *last_branch = substitute_branch_name(, );
-   const char **p, *r;
-   int refs_found = 0;
-
-   *ref = NULL;
-   for (p = ref_rev_parse_rules; *p; p++) {
-   char fullref[PATH_MAX];
-   unsigned char sha1_from_ref[20];
-   unsigned char *this_result;
-   int flag;
-
-   this_result = refs_found ? sha1_from_ref : sha1;
-   mksnpath(fullref, sizeof(fullref), *p, len, str);
-   r = resolve_ref_unsafe(fullref, RESOLVE_REF_READING,
-  this_result, );
-   if (r) {
-   if (!refs_found++)
-   *ref = xstrdup(r);
-   if (!warn_ambiguous_refs)
-   break;
-   } else if ((flag & REF_ISSYMREF) && strcmp(fullref, "HEAD")) {
-   warning("ignoring dangling symref %s.", fullref);
-   } else if ((flag & REF_ISBROKEN) && strchr(fullref, '/')) {
-   warning("ignoring broken ref %s.", fullref);
-   }
-   }
-   free(last_branch);
-   return refs_found;
-}
-
-int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
-{
-   char *last_branch = substitute_branch_name(, );
-   const char **p;
-   int logs_found = 0;
-
-   *log = NULL;
-   for (p = ref_rev_parse_rules; *p; p++) {
-   unsigned char hash[20];
-   char path[PATH_MAX];
-   const char *ref, *it;
-
-   mksnpath(path, sizeof(path), *p, len, str);
-   ref = resolve_ref_unsafe(path, RESOLVE_REF_READING,
-hash, NULL);
-   if (!ref)
-   continue;
-   if (reflog_exists(path))
-   it = path;
-   else if (strcmp(ref, path) && reflog_exists(ref))
-   it = ref;
-   else
-   continue;
-   if (!logs_found++) {
-   *log = xstrdup(it);
-   hashcpy(sha1, hash);
-   }
-   if (!warn_ambiguous_refs)
-   break;
-   }
-   free(last_branch);
-   return logs_found;
-}
-
-/*
  * Locks a ref returning the lock on success and NULL on failure.
  * On failure errno is set to something meaningful.
  */
@@ -4122,99 +4012,6 @@ cleanup:
return ret;
 }
 
-char *shorten_unambiguous_ref(const char *refname, int strict)
-{
-   int i;
-   static char **scanf_fmts;
-   static int nr_rules;
-   char *short_name;
-
-   if (!nr_rules) {
-   /*
-* Pre-generate scanf formats from ref_rev_parse_rules[].
-

Re: [PATCH v3] merge: fix cache_entry use-after-free

[David Turner]

On Thu, 2015-10-15 at 05:35 +0200, René Scharfe wrote:
> Am 15.10.2015 um 00:07 schrieb David Turner:
> > From: Keith McGuigan 
> >
> > During merges, we would previously free entries that we no longer need
> > in the destination index.  But those entries might also be stored in
> > the dir_entry cache, and when a later call to add_to_index found them,
> > they would be used after being freed.
> >
> > To prevent this, add a ref count for struct cache_entry.  Whenever
> > a cache entry is added to a data structure, the ref count is incremented;
> > when it is removed from the data structure, it is decremented.  When
> > it hits zero, the cache_entry is freed.
> >
> > Signed-off-by: Keith McGuigan 
> > Signed-off-by: David Turner 
> > ---
> >
> > Fix type of ref_count (from unsigned int to int).
> >
> >
> >   cache.h| 27 +++
> >   name-hash.c|  7 ++-
> >   read-cache.c   |  6 +-
> >   split-index.c  | 13 -
> >   unpack-trees.c |  6 --
> >   5 files changed, 50 insertions(+), 9 deletions(-)
> >
> > diff --git a/cache.h b/cache.h
> > index 752031e..7906026 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -149,6 +149,7 @@ struct stat_data {
> >
> >   struct cache_entry {
> > struct hashmap_entry ent;
> > +   int ref_count; /* count the number of refs to this in dir_hash */
> > struct stat_data ce_stat_data;
> > unsigned int ce_mode;
> > unsigned int ce_flags;
> > @@ -213,6 +214,32 @@ struct cache_entry {
> >   struct pathspec;
> >
> >   /*
> > + * Increment the cache_entry reference count.  Should be called
> > + * whenever a pointer to a cache_entry is retained in a data structure,
> > + * thus marking it as alive.
> > + */
> > +static inline void add_ce_ref(struct cache_entry *ce)
> > +{
> > +   assert(ce != NULL && ce->ref_count >= 0);
> > +   ce->ref_count++;
> > +}
> > +
> > +/*
> > + * Decrement the cache_entry reference count.  Should be called whenever
> > + * a pointer to a cache_entry is dropped.  Once the counter drops to 0
> > + * the cache_entry memory will be safely freed.
> > + */
> > +static inline void drop_ce_ref(struct cache_entry *ce)
> > +{
> > +   if (ce != NULL) {
> > +   assert(ce->ref_count >= 0);
> 
> Shouldn't this be "> 0" to prevent double frees?

No.  If the ref_count is 1, then there is still some reference to the
ce.  If it is 0, there is no reference to it, and the next check (< 1)
will succeed and the ce will get freed.  

> > +   if (--ce->ref_count < 1) {
> > +   free(ce);
> > +   }
> > +   }
> > +}
> 


--
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 2/3] stripspace: Implement --count-lines option

[Matthieu Moy]

Tobias Klauser  writes:

> + * comments are considered contents to be removed or not. Returns the
> + * number of lines in the resulting buffer.

We write comments at imperative tone, hence "Return", not "Returns".

Other than that, I agree with Junio:

* A preparatory patch introducing parse-options would make the actual
  patch much easier to review.

* Just running strbuf_stripspace and counting the number of lines in the
  result is much simpler. We use stripspace only on user-provided input
  which are never really big so maintainability is more important than
  performance.

Cheers,

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

Re: [PATCH 2/3] stripspace: Implement --count-lines option

[Junio C Hamano]

Tobias Klauser  writes:

> diff --git a/Documentation/git-stripspace.txt 
> b/Documentation/git-stripspace.txt
> index 60328d5..411e17c 100644
> --- a/Documentation/git-stripspace.txt
> +++ b/Documentation/git-stripspace.txt
> @@ -9,7 +9,7 @@ git-stripspace - Remove unnecessary whitespace
>  SYNOPSIS
>  
>  [verse]
> -'git stripspace' [-s | --strip-comments] < input
> +'git stripspace' [-s | --strip-comments] [-C | --count-lines] < input
>  'git stripspace' [-c | --comment-lines] < input
>  
>  DESCRIPTION
> @@ -44,6 +44,11 @@ OPTIONS
>   be terminated with a newline. On empty lines, only the comment character
>   will be prepended.
>  
> +-C::
> +--count-lines::
> + Output the number of resulting lines after stripping. This is equivalent
> + to calling 'git stripspace | wc -l'.

I agree with Matthieu that squatting on a short-and-sweet -C is
unwanted here.

> diff --git a/builtin/stripspace.c b/builtin/stripspace.c
> index f677093..7882edd 100644
> --- a/builtin/stripspace.c
> +++ b/builtin/stripspace.c
> @@ -1,5 +1,6 @@
>  #include "builtin.h"
>  #include "cache.h"
> +#include "parse-options.h"
>  #include "strbuf.h"
>  
>  static void comment_lines(struct strbuf *buf)
> @@ -12,45 +13,51 @@ static void comment_lines(struct strbuf *buf)
>   free(msg);
>  }
>  
> -static const char *usage_msg = "\n"
> -"  git stripspace [-s | --strip-comments] < input\n"
> -"  git stripspace [-c | --comment-lines] < input";
> +static const char * const usage_msg[] = {
> + N_("git stripspace [-s | --strip-comments] [-C | --count-lines] < 
> input"),
> + N_("git stripspace [-c | --comment-lines] < input"),
> + NULL
> +};
>  
>  int cmd_stripspace(int argc, const char **argv, const char *prefix)
>  {
>   struct strbuf buf = STRBUF_INIT;
> - int strip_comments = 0;
> - enum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = 
> STRIP_SPACE;
> -
> - if (argc == 2) {
> - if (!strcmp(argv[1], "-s") ||
> - !strcmp(argv[1], "--strip-comments")) {
> - strip_comments = 1;
> - } else if (!strcmp(argv[1], "-c") ||
> -!strcmp(argv[1], "--comment-lines")) {
> - mode = COMMENT_LINES;
> - } else {
> - mode = INVAL;
> - }
> - } else if (argc > 1) {
> - mode = INVAL;
> - }
> + int comment_mode = 0, count_lines = 0, strip_comments = 0;
> + size_t lines = 0;
> +
> + const struct option options[] = {
> + OPT_BOOL('s', "strip-comments", _comments,
> +  N_("skip and remove all lines starting with comment 
> character")),
> + OPT_BOOL('c', "comment-lines", _mode,
> +  N_("prepend comment character and blank to each 
> line")),
> + OPT_BOOL('C', "count-lines", _lines, N_("print line 
> count")),
> + OPT_END()
> + };

I think -s and -c are incompatible, so OPT_CMDMODE() might be a
better fit for them if you are going to switch the command line
parser to use parse-options.

Which makes me strongly suspect that this should be done in at least
two separate steps.  (1) Use parse-options to parse command line
without adding the counting at all, followed by (2) Add counting.

> + if (!count_lines)
> + write_or_die(1, buf.buf, buf.len);
> + else {
> + struct strbuf tmp = STRBUF_INIT;
> + strbuf_addf(, "%zu\n", lines);
> + write_or_die(1, tmp.buf, tmp.len);
> + }

So this is your output code, which gives only the number of lines
without the cleaned up result.

> @@ -797,15 +799,19 @@ void strbuf_stripspace(struct strbuf *sb, int 
> skip_comments)
>  
>   /* Not just an empty line? */
>   if (newlen) {
> - if (empties > 0 && j > 0)
> + if (empties > 0 && j > 0) {
>   sb->buf[j++] = '\n';
> + lines++;
> + }
>   empties = 0;
>   memmove(sb->buf + j, sb->buf + i, newlen);
>   sb->buf[newlen + j++] = '\n';
> + lines++;
>   } else {
>   empties++;
>   }
>   }

I cannot say that the above is one of the better possible
implementations of this feature I would think of.

 (1) If this is performance sensitive, then you do not want to do
 memmove() etc. to actually touch the contents of the *sb and
 only increment lines++, because you are not going to show that
 in the output anyway.

 (2) If this feature is not performance sensitive, then the best
 implementation would be not to touch strbuf_stripspace() at all
 to realize this change, and count the number of '\n' in the
 cmd_stripspace() just before you use printf("%d\n", lines).
 That is best from maintainability's