Re: [PATCH] make config --add behave correctly for empty and NULL values

2014-08-19 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 I just used

   #define CONFIG_REGEX_NONE ((void *)1)

 as my magic sentinel value, both for the string and compiled regex
 versions. Adding a bit to the store struct is a lot less disgusting and
 error-prone. So I won't share mine here. :)

Actually, I wrote something like that aloud but did not type it out
;-).  Great minds think alike.

We already have some code paths that use ((void *)1) as a special
pointer value, so in that sense I would say it is not the end of the
world if you added a new one.  At the end-user level (i.e. people
who write callers to set-multivar-in-file function), I actually like
your idea of inventing our own string syntax and parse it at the
place where we strip '!' out and remember that the pattern's match
status needs to be negated.  For example, instead of a^ (to which
I cannot say with confidence that no implementation would match the
not-at-the-beginning caret literally), I would not mind if we taught
set-multivar-in-file that we use !* as a mark to tell this
pattern never matches, and have it assign your never matches
mark, i.e. (void *)1, to store.value_regex.  Then matches() would
become

static int matches(const char *key, const char *value)
{
if (strcmp(key, store.key))
return 0; /* not ours */
if (!store.value_regex)
return 1; /* always matches */
if (store.value_regex == CONFIG_REGEX_NONE)
return 0; /* never matches */
return store.do_not_match ^
!regexec(store.value_regex, value, 0, NULL, 0);
}

or something like that, and the ugly magic will be localized,
which may make it more palatable.
--
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


Issuing warning when hook does not have execution permission

2014-08-19 Thread Babak M
Hi,

I saw that if a hook file is present in .git/hooks and it does not
have execution permissions it is silently ignored.

I thought it might be worthwhile issuing a warning such as Warning:
pre-commit hook exists but it cannot be executed due to insufficient
permissions.

Not sure if this has been discussed before. I searched the archive but
didn't see anything.

Thoughts, suggestions? Is there anything like that already?

Cheers,
Babak
--
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 profile issue on Git 2.1.0

2014-08-19 Thread Jeff King
On Sun, Aug 17, 2014 at 09:35:29PM -0500, Andrés Sicard-Ramírez wrote:

 I have the following issue on Git 2.1.0:
 
 $ make prefix=/some-directory profile
 ...
 make[2]: Entering directory `/home/asr/src/git/git-2.1.0/t/perf'
 rm -rf test-results
 ./run
 === Running 9 tests in this tree ===
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 error: No $GIT_PERF_REPO defined, and your build directory is not a repo
 cannot open test-results/p-perf-lib-sanity.subtests: No such file
 or directory at ./aggregate.perl line 77.
 make[2]: *** [perf] Error 2
 
 I hadn't issues running the same command on Git 2.0.2.

This is because v2.1.0 started using make perf to feed the profile
builds, but it doesn't work on tarballs.

Maybe we should do this:

-- 8 --
Subject: Makefile: make perf tests optional for profile build

The perf tests need a repository to operate on; if none is
defined, we fall back to the repository containing our build
directory.  That fails, though, for an exported tarball of
git.git, which has no repository.

Since 5d7fd6d we run the perf tests as part of make
profile. Therefore make profile fails out of the box on
released tarballs of v2.1.0.

We can fix this by making the perf tests optional; if they
are skipped, we still run the regular test suite, which
should give a lot of profile data (and is what we used to do
prior to 5d7fd6d anyway).

Signed-off-by: Jeff King p...@peff.net
---
As a side note, while testing this I noticed that the make perf run
goes a lot faster if you set GIT_PERF_REPEAT_COUNT=1. This is bad for
actually measuring things, but probably fine for profile feedback. I
don't use the profile builds myself, though, so I will leave it to
somebody who cares more to investigate whether such a change would be a
good idea.

 Makefile | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 2320de5..9f984a9 100644
--- a/Makefile
+++ b/Makefile
@@ -1659,7 +1659,11 @@ endif
 profile:: profile-clean
$(MAKE) PROFILE=GEN all
$(MAKE) PROFILE=GEN -j1 test
-   $(MAKE) PROFILE=GEN -j1 perf
+   @if test -n $$GIT_PERF_REPO || test -d .git; then \
+   $(MAKE) PROFILE=GEN -j1 perf; \
+   else \
+   echo Skipping profile of perf tests...; \
+   fi
$(MAKE) PROFILE=USE all
 
 profile-fast: profile-clean
-- 
2.1.0.346.ga0367b9

--
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] make config --add behave correctly for empty and NULL values

2014-08-19 Thread Jeff King
On Mon, Aug 18, 2014 at 11:03:51PM -0700, Junio C Hamano wrote:

 We already have some code paths that use ((void *)1) as a special
 pointer value, so in that sense I would say it is not the end of the
 world if you added a new one.

No, but if you use it to replace the regexp, you end up having to check
for it in the code paths that regfree() the result. I think a separate
bit is nicer for that reason.

 At the end-user level (i.e. people who write callers to
 set-multivar-in-file function), I actually like your idea of inventing
 our own string syntax and parse it at the place where we strip '!' out
 and remember that the pattern's match status needs to be negated.

I thought at first that ! by itself might be fine, but that is
actually a valid regex. And we may feed user input directly to the
function, so we have to be careful not to get too clever.

 For example, instead of a^ (to which
 I cannot say with confidence that no implementation would match the
 not-at-the-beginning caret literally), I would not mind if we taught
 set-multivar-in-file that we use !* as a mark to tell this
 pattern never matches,

That would work, but I think CONFIG_REGEX_NONE or similar is a bit less
cryptic, and not any harder to implement.

Here is the patch I wrote, for reference (I also think breaking the
matches function into a series of conditionals, as you showed, is way
more readable):

diff --git a/builtin/config.c b/builtin/config.c
index b9e7dce..7bba516 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -586,7 +586,8 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
check_argc(argc, 2, 2);
value = normalize_value(argv[0], argv[1]);
return git_config_set_multivar_in_file(given_config_source.file,
-  argv[0], value, a^, 0);
+  argv[0], value,
+  CONFIG_REGEX_NONE, 0);
}
else if (actions == ACTION_REPLACE_ALL) {
check_write();
diff --git a/cache.h b/cache.h
index fcb511d..dcf3a2a 100644
--- a/cache.h
+++ b/cache.h
@@ -1281,6 +1281,8 @@ extern int update_server_info(int);
 #define CONFIG_INVALID_PATTERN 6
 #define CONFIG_GENERIC_ERROR 7
 
+#define CONFIG_REGEX_NONE ((void *)1)
+
 struct git_config_source {
unsigned int use_stdin:1;
const char *file;
diff --git a/config.c b/config.c
index 67a7729..1199faf 100644
--- a/config.c
+++ b/config.c
@@ -1229,6 +1229,7 @@ static struct {
 static int matches(const char *key, const char *value)
 {
return !strcmp(key, store.key) 
+   store.value_regex != CONFIG_REGEX_NONE 
(store.value_regex == NULL ||
 (store.do_not_match ^
  (value  !regexec(store.value_regex, value, 0, NULL, 0;
@@ -1493,6 +1494,8 @@ out_free_ret_1:
 /*
  * If value==NULL, unset in (remove from) config,
  * if value_regex!=NULL, disregard key/value pairs where value does not match.
+ * if value_regex==CONFIG_REGEX_NONE, do not match any existing values
+ * (only add a new one)
  * if multi_replace==0, nothing, or only one matching key/value is replaced,
  * else all matching key/values (regardless how many) are removed,
  * before the new pair is written.
@@ -1576,6 +1579,8 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
 
if (value_regex == NULL)
store.value_regex = NULL;
+   else if (value_regex == CONFIG_REGEX_NONE)
+   store.value_regex = CONFIG_REGEX_NONE;
else {
if (value_regex[0] == '!') {
store.do_not_match = 1;
@@ -1607,7 +1612,8 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
if (git_config_from_file(store_aux, config_filename, NULL)) {
error(invalid config file %s, config_filename);
free(store.key);
-   if (store.value_regex != NULL) {
+   if (store.value_regex != NULL 
+   store.value_regex != CONFIG_REGEX_NONE) {
regfree(store.value_regex);
free(store.value_regex);
}
@@ -1616,7 +1622,8 @@ int git_config_set_multivar_in_file(const char 
*config_filename,
}
 
free(store.key);
-   if (store.value_regex != NULL) {
+   if (store.value_regex != NULL 
+   store.value_regex != CONFIG_REGEX_NONE) {
regfree(store.value_regex);
free(store.value_regex);
}
--
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  

Re: [PATCH v2 2/2] convert: Stream from fd to required clean filter instead of mmap

2014-08-19 Thread Jeff King
On Sun, Aug 17, 2014 at 06:33:54PM +0700, Duy Nguyen wrote:

  For files 2GB on a 32-bit system (e.g. msysgit), filtering with the
  previous code always failed.  Now it works.  I created the patch to
  change git from 'fundamentally doesn't handle this' to 'works as
  expected'.
 
 I deal with similar problem and added $GIT_ALLOC_LIMIT to test large
 blob code. Maybe we could add $GIT_MMAP_LIMIT? I don't suppose we call
 xmmap/xmalloc so often that the extra test would slow git down.

Yeah, I think I'd prefer that approach. We should mmap _way_ less than
we malloc, and I do not think the malloc check has caused any problems.

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


Re: Issuing warning when hook does not have execution permission

2014-08-19 Thread Jeff King
On Tue, Aug 19, 2014 at 04:05:21PM +1000, Babak M wrote:

 I saw that if a hook file is present in .git/hooks and it does not
 have execution permissions it is silently ignored.
 
 I thought it might be worthwhile issuing a warning such as Warning:
 pre-commit hook exists but it cannot be executed due to insufficient
 permissions.
 
 Not sure if this has been discussed before. I searched the archive but
 didn't see anything.
 
 Thoughts, suggestions? Is there anything like that already?

Once upon a time we shipped sample hooks with their execute bits turned
off, and such a warning would have been very bad.

These days we give them a .sample extension (because Windows installs
had trouble with the execute bit :) ), so I think it should be OK in
theory. Installing a new version of git on top of an old one with make
install does not clean up old files. So somebody who has continuously
upgraded their git via make install to the same directory would have
the old-style sample files. Under your proposal, they would get a lot of
warnings.

However, that change came in v1.6.0, just over 6 years ago. We can
probably discount that (and if it does happen, maybe it is time for that
someone to clean up the other leftover cruft from past git installs).

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


Re: Signinig a commit with multiple signatures

2014-08-19 Thread Jeff King
On Sun, Aug 17, 2014 at 09:30:47AM -0400, Jason Pyeron wrote:

 I am working on an open source project right now where we are looking
 to enforce a N of M audit approval process. It turns out that git
 supports verifying multiple signatures because gpg supports signature
 merging.

In the scheme you propose, the commit object is actually rewritten. So
whoever made and signed it first will then need to rebase on top of the
rewritten multi-signed version.

Is there a reason not to use detached signatures, and let each person
add them after the fact? You can store them in git-notes and then push
them along with the other commits (you can even check in a pre-receive
hook that the commits meet your N of M criteria, as long as everybody
has pushed up their signature notes).

 $ cat write-commit.ruby
 #!/usr/bin/irb
 require 'fileutils'
 file = File.open(ARGV[0], rb)
 content = file.read
 header = commit #{content.length}\0
 store = header + content
 require 'digest/sha1'
 sha1 = Digest::SHA1.hexdigest(store)
 require 'zlib'
 zlib_content = Zlib::Deflate.deflate(store)
 path = '.git/objects/' + sha1[0,2] + '/' + sha1[2,38]
 FileUtils.mkdir_p(File.dirname(path))
 File.open(path, 'w') { |f| f.write zlib_content }

I think this is just git hash-object -w -t commit file, isn't it?

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


Re: [PATCH v2 2/2] convert: Stream from fd to required clean filter instead of mmap

2014-08-19 Thread Steffen Prohaska

On Aug 19, 2014, at 9:53 AM, Jeff King p...@peff.net wrote:

 For files 2GB on a 32-bit system (e.g. msysgit), filtering with the
 previous code always failed.  Now it works.  I created the patch to
 change git from 'fundamentally doesn't handle this' to 'works as
 expected'.
 
 I deal with similar problem and added $GIT_ALLOC_LIMIT to test large
 blob code. Maybe we could add $GIT_MMAP_LIMIT? I don't suppose we call
 xmmap/xmalloc so often that the extra test would slow git down.
 
 Yeah, I think I'd prefer that approach. We should mmap _way_ less than
 we malloc, and I do not think the malloc check has caused any problems.

I am going to update the patch accordingly.

Steffen

--
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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

2014-08-19 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 A sentence --force has no effect under --preserve-merges mode does
 not tell the readers very much, either and leaves them wondering if
 it means --preserve-merges mode always rebases every time it is
 asked, never noticing 'ah, the history is already in a good shape
 and there is no need to do anything further' or --preserve-merges
 mode ignores --force and refuses to recreate the history if the
 history is in the shape the mode deems is already desirable.

 In fact there is no way to force rebase when --preserve-merges is given.
 Neither --force nor --no-ff has any effect.

 Maybe some clarification could be given in --preserve-merges
 description, provided it's not clear that has no effect for --force
 means that one can't force the rebase in this case.

 I am not sure if that is an intended behaviour or simply a bug (I
 rarely use preserve-merges myself, so I offhand do not know for
 certain).

It seems that there is some problem there anyway. Probably a slight
misfeature rather than a bug, as --preserve-merges always pretends it
does something, even when it does not:

$ git log --oneline --graph --decorate
*   6f25bd5 (HEAD, topic) M
|\  
| * c5a4a43 C
* | 3c6ab7a (master) N
* | 99ff328 B
|/  
* 130ae2d A
$ git rebase --preserve-merges master
Successfully rebased and updated refs/heads/topic.
$ git log --oneline --graph --decorate
*   6f25bd5 (HEAD, topic) M
|\  
| * c5a4a43 C
* | 3c6ab7a (master) N
* | 99ff328 B
|/  
* 130ae2d A
$

git reflog topic doesn't show any changes either.

-- 
Sergey.
--
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] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

2014-08-19 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:
 Junio C Hamano gits...@pobox.com writes:
[...]
 How about doing it this way, perhaps?

Could you please apply this your suggestion, as we seem not to agree
on anything better?

 -- 8 --
 From: Sergey Organov sorga...@gmail.com
 Date: Tue, 12 Aug 2014 00:22:48 +0400
 Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would 
 otherwise be a no-op

 Current branch is a descendant of the commit you are rebasing onto
 does not necessarily mean rebase requires --force.  For a plain
 vanilla history flattening rebase, the rebase can be done without
 forcing if there is a merge between the tip of the branch being
 rebased and the commit you are rebasing onto, even if the tip is
 descendant of the other.

 [jc: reworded both the text and the log description]

 Signed-off-by: Sergey Organov sorga...@gmail.com
 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  Documentation/git-rebase.txt | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

 diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
 index 2a93c64..f14100a 100644
 --- a/Documentation/git-rebase.txt
 +++ b/Documentation/git-rebase.txt
 @@ -316,11 +316,8 @@ which makes little sense.
  
  -f::
  --force-rebase::
 - Force the rebase even if the current branch is a descendant
 - of the commit you are rebasing onto.  Normally non-interactive rebase 
 will
 - exit with the message Current branch is up to date in such a
 - situation.
 - Incompatible with the --interactive option.
 + Force a rebase even if the current branch is up-to-date and
 + the command without `--force` would return without doing anything.
  +
  You may find this (or --no-ff with an interactive rebase) helpful after
  reverting a topic branch merge, as this option recreates the topic branch 
 with

--  
Sergey.
--
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: Relative submodule URLs

2014-08-19 Thread Heiko Voigt
On Mon, Aug 18, 2014 at 01:55:05PM -0700, Jonathan Nieder wrote:
 Robert Dailey wrote:
 
  The documentation wasn't 100% clear on this, but I'm assuming by
  remote origin, it says that the relative URL is relative to the
  actual remote *named* origin (and it is not using origin as just a
  general terminology).
 
 Thanks for reporting.  The remote used is the default remote that git
 fetch without further arguments would use:
 
   get_default_remote () {
   curr_branch=$(git symbolic-ref -q HEAD)
   curr_branch=${curr_branch#refs/heads/}
   origin=$(git config --get branch.$curr_branch.remote)
   echo ${origin:-origin}
   }
 
 The documentation is wrong.  git-fetch(1) doesn't provide a name for
 this thing.  Any ideas for wording?

How about 'upstream'? Like this[1]?

Lets step back a little is this really what we want in such situation? Is one
remote really the answer here? I suppose you have relative urls in your
.gitmodules file and two remotes in you superproject right?

What you want is that the remote names in the superproject are reflected in the
submodules when you initialise and update them?

Because at the moment what you get is always a remote 'origin' in the
submodule. Even if that remote was called 'fork' in the superproject.

Maybe in the relative URLs case we should teach the clone in submodule update
to use all remotes with their names from the superproject?

Would that solve your issue?

  Is there a way to specify (on a per-clone basis) which named remote
  will be used to calculate the URL for submodules?
 
 Currently there isn't, short of reconfiguring the remote used by
 default by git fetch.

Well currently it is either the tracked remote by the currently checked out
branch or if the branch has no tracked remote configured: 'origin'.

So by configuring (or checking out a branch with) a different remote you can
choose from remote submodule are cloned. No?

  Various co-workers use the remote named central instead of
  upstream and fork instead of origin (because that just makes
  more sense to them and it's perfectly valid).
 
  However if relative submodules require 'origin' to exist AND also
  represent the upstream repository (in triangle workflow), then this
  breaks on several levels.
 
 Can you explain further?  In a triangle workflow, git fetch will
 pull from the 'origin' remote by default and will push to the remote
 named in the '[remote] pushdefault' setting (see remote.pushdefault
 in git-config(1)).  So you can do
 
   [remote]
   pushDefault = whereishouldpush
 
 and then 'git fetch' and 'git fetch --recurse-submodules' will fetch
 from origin and 'git push' will push to the whereishouldpush remote.
 
 It might make sense to introduce a new
 
   [remote]
   default = whereishouldfetch
 
 setting to allow the name origin above to be replaced, too.  Is that
 what you mean?

I think the OP problem stems from him having a branch that does not have a
remote configured. Since they do not have 'origin' as a remote and

git submodule update --init --recursive path/to/submodule

fails. Right?

Cheers Heiko

[1]
From: Heiko Voigt hvo...@hvoigt.net
Subject: [PATCH] submodule: use 'upstream' instead of 'origin' in
 documentation

When talking about relative URL's it is ambiguous to use the term
'origin', since that might denote the default remote name 'origin'. Lets
use 'upstream' to make it more clear that the upstream repository is
meant.

Signed-off-by: Heiko Voigt hvo...@hvoigt.net
---
 Documentation/git-submodule.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 8e6af65..c6f82e6 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -80,15 +80,15 @@ to exist in the superproject. If path is not given, the
 The path is also used as the submodule's logical name in its
 configuration entries unless `--name` is used to specify a logical name.
 +
-repository is the URL of the new submodule's origin repository.
+repository is the URL of the new submodule's upstream repository.
 This may be either an absolute URL, or (if it begins with ./
-or ../), the location relative to the superproject's origin
+or ../), the location relative to the superproject's upstream
 repository (Please note that to specify a repository 'foo.git'
 which is located right next to a superproject 'bar.git', you'll
 have to use '../foo.git' instead of './foo.git' - as one might expect
 when following the rules for relative URLs - because the evaluation
 of relative URLs in Git is identical to that of relative directories).
-If the superproject doesn't have an origin configured
+If the superproject doesn't have any remote configured
 the superproject is its own authoritative upstream and the current
 working directory is used instead.
 +
-- 
2.1.0.rc0.52.gaa544bf

--
To unsubscribe from 

Fwd: Shallow clone

2014-08-19 Thread Steven Evergreen
Hi, everyone. I'm trying to perform a shallow clone with visibility of
all remote branches.

git clone REPO --depth 1 --no-single-branch

is consistently giving me

Cloning into 'REPONAME'...
fatal: (null) is unknown object
remote: Total 0 (delta 0), reused 0 (delta 0)
fatal: recursion detected in die handler
remote: aborting due to possible repository corruption on the remote side.
fatal: error in sideband demultiplexer

with 2.0.3 .

Is this command not supported? Have I hit a bug?

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/RFC] git-imap-send: use libcurl for implementation

2014-08-19 Thread Bernhard Reiter
Am 2014-08-17 um 20:42 schrieb Jeff King:
 [...]
 
 I'm not sure I understand this comment. Even if SSL is not in use,
 wouldn't we be passing a regular pipe to curl, which would break?

 Yeah, we can't do that, and thus would have to keep the handwritten IMAP
 implementation just for the tunnel case (allowing to drop only the
 OpenSSL specific stuff), see my other email:
 http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
 relevant part is pretty far down at the bottom).
 
 I'd really love it if we could make this work with tunnels and
 eventually get rid of the hand-written imap code entirely. I agree with
 Jonathan that we probably need to keep it around a bit for people on
 older curl, but dropping it is a good goal in the long run. That code
 was forked from the isync project, but mangled enough that we could not
 take bug fixes from upstream. As not many people use imap-send, I
 suspect it is largely unmaintained and the source of many lurking
 bugs[1]. Replacing it with curl's maintained implementation is probably
 a good step.

I'll work on this as soon as I find some time, but as that will include
changes to run-command.c (and possibly other files?), I'd like to cover
that in a commit of its own. Do you guys think the current patch [1] is
good enough for official submission already? If so, do I need some
sort of official review? Documentation/SubmittingPatches says I'm only
supposed to direct it to Junio after the list reaches consensus, so
I'm wondering how to get there... :-)

Bernhard
--
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 profile issue on Git 2.1.0

2014-08-19 Thread Andi Kleen
 Maybe we should do this:

Looks good to me.

 As a side note, while testing this I noticed that the make perf run
 goes a lot faster if you set GIT_PERF_REPEAT_COUNT=1. This is bad for
 actually measuring things, but probably fine for profile feedback. I
 don't use the profile builds myself, though, so I will leave it to
 somebody who cares more to investigate whether such a change would be a
 good idea.

Yes should be fine too.

Another way to speed it up would be also to run the tests (both 
tests and benchmarks) in parallel on multiple cores. The gcc feedback mechanism
won't mind, as it doesn't measure time.

-Andi
--
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: Shallow clone

2014-08-19 Thread Duy Nguyen
On Tue, Aug 19, 2014 at 6:11 PM, Steven Evergreen
i.stevenevergr...@gmail.com wrote:
 Hi, everyone. I'm trying to perform a shallow clone with visibility of
 all remote branches.

 git clone REPO --depth 1 --no-single-branch

 is consistently giving me

 Cloning into 'REPONAME'...
 fatal: (null) is unknown object
 remote: Total 0 (delta 0), reused 0 (delta 0)
 fatal: recursion detected in die handler
 remote: aborting due to possible repository corruption on the remote side.
 fatal: error in sideband demultiplexer

 with 2.0.3 .

 Is this command not supported? Have I hit a bug?

Yes it's supported and yes I think you hit a bug. Any chance you can
share this repository? If not, perhaps just SHA-1 so we know what the
commit DAG looks like?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Robin Rosenberg
Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like .tmp, which does not cause confusion.

Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
---
 Documentation/git-mergetool.txt |  7 +++
 git-mergetool.sh| 10 ++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..a586766 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable to 
`false`
 causes `git mergetool` to automatically remove the backup as files
 are successfully merged.
 
+`git mergetool` may also create other temporary files for the
+different versions involved in the merge. By default these files have
+the same filename suffix as the file being merged. This may confuse
+other tools in use during a long merge operation. The user can set
+`mergtool.tmpsuffix` to be used as an extra suffix, which will be
+appened to the temporary filenamame to lessen that problem.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9a046b7..d7cc76c 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -214,6 +214,8 @@ checkout_staged_file () {
 }
 
 merge_file () {
+   tmpsuffix=$(git config mergetool.tmpsuffix || true)
+
MERGED=$1
 
f=$(git ls-files -u -- $MERGED)
@@ -229,10 +231,10 @@ merge_file () {
fi
 
ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$')
-   BACKUP=./$MERGED.BACKUP.$ext
-   LOCAL=./$MERGED.LOCAL.$ext
-   REMOTE=./$MERGED.REMOTE.$ext
-   BASE=./$MERGED.BASE.$ext
+   BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
+   LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
+   REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
+   BASE=./$MERGED.BASE.$ext$tmpsuffix
 
base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print 
$1;}')
-- 
2.1.0.rc2.6.g39c33ff.dirty

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


Re: [PATCH] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Stefan Näwe
Am 19.08.2014 um 14:22 schrieb Robin Rosenberg:
 Using the original filename suffix for the temporary input files to
 the merge tool confuses IDEs like Eclipse. This patch introduces
 a configurtion option, mergetool.tmpsuffix, which get appended to
 the temporary file name. That way the user can choose to use a
 suffix like .tmp, which does not cause confusion.
 
 Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
 ---
  Documentation/git-mergetool.txt |  7 +++
  git-mergetool.sh| 10 ++
  2 files changed, 13 insertions(+), 4 deletions(-)
 
 diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
 index e846c2e..a586766 100644
 --- a/Documentation/git-mergetool.txt
 +++ b/Documentation/git-mergetool.txt
 @@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable 
 to `false`
  causes `git mergetool` to automatically remove the backup as files
  are successfully merged.
  
 +`git mergetool` may also create other temporary files for the
 +different versions involved in the merge. By default these files have
 +the same filename suffix as the file being merged. This may confuse
 +other tools in use during a long merge operation. The user can set
 +`mergtool.tmpsuffix` to be used as an extra suffix, which will be

s/mergtool/mergetool/

 +appened to the temporary filenamame to lessen that problem.

s/appened/appended/
s/filenamame/filename/

 +
  GIT
  ---
  Part of the linkgit:git[1] suite
 diff --git a/git-mergetool.sh b/git-mergetool.sh
 index 9a046b7..d7cc76c 100755
 --- a/git-mergetool.sh
 +++ b/git-mergetool.sh
 @@ -214,6 +214,8 @@ checkout_staged_file () {
  }
  
  merge_file () {
 + tmpsuffix=$(git config mergetool.tmpsuffix || true)
 +
   MERGED=$1
  
   f=$(git ls-files -u -- $MERGED)
 @@ -229,10 +231,10 @@ merge_file () {
   fi
  
   ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$')
 - BACKUP=./$MERGED.BACKUP.$ext
 - LOCAL=./$MERGED.LOCAL.$ext
 - REMOTE=./$MERGED.REMOTE.$ext
 - BASE=./$MERGED.BASE.$ext
 + BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
 + LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
 + REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
 + BASE=./$MERGED.BASE.$ext$tmpsuffix
  
   base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}')
   local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print 
 $1;}')
 

Stefan
-- 

/dev/random says: Confusion not only reigns, it pours.
python -c print 
'73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')
 
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9  9666 829B 49C5 9221 27AF
--
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: Signinig a commit with multiple signatures

2014-08-19 Thread Jason Pyeron
 -Original Message-
 From: Jeff King
 Sent: Tuesday, August 19, 2014 4:05
 
 On Sun, Aug 17, 2014 at 09:30:47AM -0400, Jason Pyeron wrote:
 
  I am working on an open source project right now where we 
 are looking
  to enforce a N of M audit approval process. It turns out that git
  supports verifying multiple signatures because gpg supports 
 signature
  merging.
 
 In the scheme you propose, the commit object is actually rewritten. So
 whoever made and signed it first will then need to rebase on 
 top of the
 rewritten multi-signed version.

Not exactly. A known and shared commit is used as the parent of an empty, no 
changes commit. The no changes commit object is taken and passed around 
before being added into the repository. There is no need for a rebase.

But my scheme uses out-of-band process to accomplish this. The idea of using 
git to distribute the conflict resolution seemed like a valid use case of 
sharing a working copy state for a distributed commit, just like this. [1][2]

 
 Is there a reason not to use detached signatures, and let each person

Yes. The embeded signatures provides the best user experience (UX) for 
verification.

 add them after the fact? You can store them in git-notes and then push
 them along with the other commits (you can even check in a pre-receive
 hook that the commits meet your N of M criteria, as long as everybody
 has pushed up their signature notes).
 
  $ cat write-commit.ruby
  #!/usr/bin/irb
  require 'fileutils'
  file = File.open(ARGV[0], rb)
  content = file.read
  header = commit #{content.length}\0
  store = header + content
  require 'digest/sha1'
  sha1 = Digest::SHA1.hexdigest(store)
  require 'zlib'
  zlib_content = Zlib::Deflate.deflate(store)
  path = '.git/objects/' + sha1[0,2] + '/' + sha1[2,38]
  FileUtils.mkdir_p(File.dirname(path))
  File.open(path, 'w') { |f| f.write zlib_content }
 
 I think this is just git hash-object -w -t commit file, isn't it?

Let me find the most complicated way of saying this, yes. I feel silly for that.

-Jason

[1]: 
http://git.661346.n2.nabble.com/Sharing-a-massive-distributed-merge-td6178696.html
[2]: 
http://git.661346.n2.nabble.com/Sharing-merge-conflict-resolution-between-multiple-developers-td7616700.html

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


[PATCH v2] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Robin Rosenberg
Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like .tmp, which does not cause confusion.

Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
---
 Documentation/git-mergetool.txt |  7 +++
 git-mergetool.sh| 10 ++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..80a0526 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable to 
`false`
 causes `git mergetool` to automatically remove the backup as files
 are successfully merged.
 
+`git mergetool` may also create other temporary files for the
+different versions involved in the merge. By default these files have
+the same filename suffix as the file being merged. This may confuse
+other tools in use during a long merge operation. The user can set
+`mergetool.tmpsuffix` to be used as an extra suffix, which will be
+appened to the temporary filename to lessen that problem.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9a046b7..d7cc76c 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -214,6 +214,8 @@ checkout_staged_file () {
 }
 
 merge_file () {
+   tmpsuffix=$(git config mergetool.tmpsuffix || true)
+
MERGED=$1
 
f=$(git ls-files -u -- $MERGED)
@@ -229,10 +231,10 @@ merge_file () {
fi
 
ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$')
-   BACKUP=./$MERGED.BACKUP.$ext
-   LOCAL=./$MERGED.LOCAL.$ext
-   REMOTE=./$MERGED.REMOTE.$ext
-   BASE=./$MERGED.BASE.$ext
+   BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
+   LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
+   REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
+   BASE=./$MERGED.BASE.$ext$tmpsuffix
 
base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print 
$1;}')
-- 
2.1.0.rc2.6.g39c33ff.dirty

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


Re: make profile issue on Git 2.1.0

2014-08-19 Thread Andrés Sicard-Ramírez
On 19 August 2014 01:12, Jeff King p...@peff.net wrote:
 This is because v2.1.0 started using make perf to feed the profile
 builds, but it doesn't work on tarballs.

Thanks for the explanation.

-- 
Andrés
--
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: Relative submodule URLs

2014-08-19 Thread Robert Dailey
On Mon, Aug 18, 2014 at 3:55 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Thanks for reporting.  The remote used is the default remote that git
 fetch without further arguments would use:

 get_default_remote () {
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch=${curr_branch#refs/heads/}
 origin=$(git config --get branch.$curr_branch.remote)
 echo ${origin:-origin}
 }

 The documentation is wrong.  git-fetch(1) doesn't provide a name for
 this thing.  Any ideas for wording?

I guess a good start would be to call it the tracked remote instead
of remote origin. The word tracked here makes it very obvious that
if I have a remote tracking branch setup, it will use the remote
portion of that configuration.

The real question is, how will `git submodule update` function if a
tracking remote is not configured? Will it fail with some useful error
message? I don't like the idea of it defaulting to self remote mode,
where it will be relative to my repo directory. That seems like it
would fail is subtle ways in a worst-case scenario (if I did by
happenstance have a bare repository cloned up one directory level for
other reasons).

 Currently there isn't, short of reconfiguring the remote used by
 default by git fetch.

I wish there was a way to specify the remote on a per-branch basis
separately from the tracking branch. I read a while back that someone
proposed some changes to git to support decentralized tracking
(concept of an upstream tracking branch and a separate one for origin,
i think). If that were implemented, then relative submodules could
utilize the 'upstream' remote by default for each branch, which would
provide more predictable default behavior. Right now most people on my
team would not be aware that if they tracked a branch on their fork,
they would subsequently need to fork the submodules to that same
remote.

 Various co-workers use the remote named central instead of
 upstream and fork instead of origin (because that just makes
 more sense to them and it's perfectly valid).

 However if relative submodules require 'origin' to exist AND also
 represent the upstream repository (in triangle workflow), then this
 breaks on several levels.

 Can you explain further?  In a triangle workflow, git fetch will
 pull from the 'origin' remote by default and will push to the remote
 named in the '[remote] pushdefault' setting (see remote.pushdefault
 in git-config(1)).  So you can do

 [remote]
 pushDefault = whereishouldpush

 and then 'git fetch' and 'git fetch --recurse-submodules' will fetch
 from origin and 'git push' will push to the whereishouldpush remote.

I didn't know about this option, seems useful.

A common workflow that we use on my team is to set the tracking branch
to 'upstream' for convenient pulls with rebase. This means a feature
branch of mine can track its parent branch on 'upstream', so that when
other pull requests get merged in on the remote repo branch, I can
just do `git pull` and my feature branch rebases onto the latest of
that parent branch.

Cases like these would work with relative submodules because
'upstream' is the tracked remote (and most of the time we don't want
to fork submodules). However sometimes I like to track the same pushed
branch on origin (my fork), especially when it is up for pull request.
In these cases, my submodule update will fail because I didn't fork my
submodules when I changed my tracking branch. Is this correct?

breaks on several levels was basically my way of saying that various
workflow choices will break when you introduce submodules. One of the
beautiful things about Git is that it allows everyone to choose their
own workflow. But submodules seem to prevent that to some degree. I
think addressing relative submodule usability issues is the best
approach for the long term as they feel more sustainable and scalable.
It's an absolute pain to move a submodule URL, I think we've all
experienced it. It's even harder for me because I'm the go-to at work
for help with Git. Most people that aren't advanced with Git will not
know what to do without a ton of reading  such.

 It might make sense to introduce a new

 [remote]
 default = whereishouldfetch

 setting to allow the name origin above to be replaced, too.  Is that
 what you mean?

I think you're on the right path. However I'd suggest something like
the following:

[submodule]
remote = remote_for_relative_submodules (e.g. `upstream`)

[branch.name]
submoduleRemote = remote_for_relative_submodule

Above, `submodule.remote` is the 'default' remote used by all relative
submodules on all branches. If unspecified, it defaults to
`branch.name.remote` as it currently behaves.

`branch.name.submoduleRemote` is an override for `submodule.remote`.

Basically if you consider this scenario:

[branch.myfoo]
   remote = origin
   submoduleRemote = upstream

I can track an 

Re: Relative submodule URLs

2014-08-19 Thread Robert Dailey
On Tue, Aug 19, 2014 at 5:24 AM, Heiko Voigt hvo...@hvoigt.net wrote:
 I think the OP problem stems from him having a branch that does not have a
 remote configured. Since they do not have 'origin' as a remote and

 git submodule update --init --recursive path/to/submodule

 fails. Right?

Not exactly. The issue is that there is a tug of war going on between
three specific commands (all of which utilize the tracked remote):

git fetch
git pull
git submodule update (for relative submodules)

The way I set up my remote tracking branch will be different for each
of these commands:

- git pull :: If I want convenient pulls (with rebase), I will track
my upstream branch. My pushes have to be more explicit as a tradeoff.
- git push :: If I want convenient pushes, track my origin branch.
Pulls become less convenient. My relative submodules will now need to
be forked.
- git submodule update :: I track upstream to avoid forking my
submodules. But pushes become more inconvenient.

As you can see, I feel like we're overusing the single remote setting.
Sure, we've added some global settings to set default push/pull
remotes and such, but I don't think that is a sustainable long term
solution. I like the idea of possibly introducing multiple tracking
remotes for various purposes. This adds some additional configuration
overhead (slightly), but git is already very config heavy so it might
be worth exploring. At least, this feels like a better thing for the
long term as I won't be constantly switching my tracking remote for
various purposes.

Could also explore the possibility of creating const remotes. If we
specify a remote MUST exist for relative submodules, git can create it
for us, and fail to operate without it. It's up to the user to map
fork to origin if needed (perhaps add a `git remote clone source
new remote` to assist with this)?

Various approaches we can take, but I don't do development on Git so
I'm not sure what makes the most sense.
--
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 2/5] refs.c: make add_packed_ref return an error instead of calling die

2014-08-19 Thread Ronnie Sahlberg
Change add_packed_ref to return an error instead of calling die().
Update all callers to check the return value of add_packed_ref.

We can also skip checking the refname format since this function is now
static and only called from the transaction code.
If we are updating a ref and the refname is bad then we fail the transaction
already in transaction_update_sha1().
For the ref deletion case the only caveat is that we would not want
to move the badly named ref into the packed refs file during transaction
commit. This again is not a problem since if the name is bad, then
resolve_ref_unsafe() will fail which protects us from calling add_packed_ref()
with the bad name.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/refs.c b/refs.c
index 65eee72..0aad8c8 100644
--- a/refs.c
+++ b/refs.c
@@ -1135,17 +1135,16 @@ static struct ref_dir *get_packed_refs(struct ref_cache 
*refs)
return get_packed_ref_dir(get_packed_ref_cache(refs));
 }
 
-static void add_packed_ref(const char *refname, const unsigned char *sha1)
+static int add_packed_ref(const char *refname, const unsigned char *sha1)
 {
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
 
if (!packed_ref_cache-lock)
-   die(internal error: packed refs not locked);
-   if (check_refname_format(refname, 
REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))
-   die(Reference has invalid format: '%s', refname);
+   return -1;
add_ref(get_packed_ref_dir(packed_ref_cache),
create_ref_entry(refname, sha1, REF_ISPACKED));
+   return 0;
 }
 
 /*
@@ -3666,7 +3665,13 @@ int transaction_commit(struct ref_transaction 
*transaction,
RESOLVE_REF_READING, NULL))
continue;
 
-   add_packed_ref(update-refname, sha1);
+   if (add_packed_ref(update-refname, sha1)) {
+   if (err)
+   strbuf_addf(err, Failed to add %s to packed 
+   refs, update-refname);
+   ret = -1;
+   goto cleanup;
+   }
need_repack = 1;
}
if (need_repack) {
@@ -3778,7 +3783,13 @@ int transaction_commit(struct ref_transaction 
*transaction,
 
packed = get_packed_refs(ref_cache);
remove_entry(packed, update-refname);
-   add_packed_ref(update-refname, update-new_sha1);
+   if (add_packed_ref(update-refname, update-new_sha1)) {
+   if (err)
+   strbuf_addf(err, Failed to add %s to packed 
+   refs, update-refname);
+   ret = -1;
+   goto cleanup;
+   }
need_repack = 1;
 
try_remove_empty_parents((char *)update-refname);
-- 
2.0.1.556.ge8f7cba.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 4/5] refs.c: add an err argument to commit_packed_refs

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 85 +++---
 1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/refs.c b/refs.c
index cfe1292..19e73f3 100644
--- a/refs.c
+++ b/refs.c
@@ -2232,8 +2232,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
  * Write an entry to the packed-refs file for the specified refname.
  * If peeled is non-NULL, write it as the entry's peeled value.
  */
-static void write_packed_entry(int fd, char *refname, unsigned char *sha1,
-  unsigned char *peeled)
+static int write_packed_entry(int fd, char *refname, unsigned char *sha1,
+ unsigned char *peeled, struct strbuf *err)
 {
char line[PATH_MAX + 100];
int len;
@@ -2241,33 +2241,50 @@ static void write_packed_entry(int fd, char *refname, 
unsigned char *sha1,
len = snprintf(line, sizeof(line), %s %s\n,
   sha1_to_hex(sha1), refname);
/* this should not happen but just being defensive */
-   if (len  sizeof(line))
-   die(too long a refname '%s', refname);
-   write_or_die(fd, line, len);
-
+   if (len  sizeof(line)) {
+   strbuf_addf(err, too long a refname '%s', refname);
+   return -1;
+   }
+   if (write_in_full(fd, line, len) != len) {
+   strbuf_addf(err, error writing to packed-refs. %s,
+   strerror(errno));
+   return -1;
+   }
if (peeled) {
if (snprintf(line, sizeof(line), ^%s\n,
 sha1_to_hex(peeled)) != PEELED_LINE_LENGTH)
die(internal error);
-   write_or_die(fd, line, PEELED_LINE_LENGTH);
+   len = PEELED_LINE_LENGTH;
+   if (write_in_full(fd, line, len) != len) {
+   strbuf_addf(err, error writing to packed-refs. %s,
+   strerror(errno));
+   return -1;
+   }
}
+   return 0;
 }
 
+struct write_packed_data {
+   int fd;
+   struct strbuf *err;
+};
+
 /*
  * An each_ref_entry_fn that writes the entry to a packed-refs file.
  */
 static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
 {
-   int *fd = cb_data;
+   struct write_packed_data *data = cb_data;
enum peel_status peel_status = peel_entry(entry, 0);
 
-   if (peel_status != PEEL_PEELED  peel_status != PEEL_NON_TAG)
-   error(internal error: %s is not a valid packed reference!,
- entry-name);
-   write_packed_entry(*fd, entry-name, entry-u.value.sha1,
-  peel_status == PEEL_PEELED ?
-  entry-u.value.peeled : NULL);
-   return 0;
+   if (peel_status != PEEL_PEELED  peel_status != PEEL_NON_TAG) {
+   strbuf_addf(data-err, internal error: %s is not a 
+   valid packed reference!, entry-name);
+   return -1;
+   }
+   return write_packed_entry(data-fd, entry-name, entry-u.value.sha1,
+ peel_status == PEEL_PEELED ?
+ entry-u.value.peeled : NULL, data-err);
 }
 
 static int lock_packed_refs(struct strbuf *err)
@@ -2296,30 +2313,34 @@ static int lock_packed_refs(struct strbuf *err)
 
 /*
  * Commit the packed refs changes.
- * On error we must make sure that errno contains a meaningful value.
  */
-static int commit_packed_refs(void)
+static int commit_packed_refs(struct strbuf *err)
 {
struct packed_ref_cache *packed_ref_cache =
get_packed_ref_cache(ref_cache);
int error = 0;
-   int save_errno = 0;
+   struct write_packed_data data;
 
if (!packed_ref_cache-lock)
die(internal error: packed-refs not locked);
-   write_or_die(packed_ref_cache-lock-fd,
-PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER));
+   if (write_in_full(packed_ref_cache-lock-fd,
+ PACKED_REFS_HEADER, strlen(PACKED_REFS_HEADER))  0) {
+   strbuf_addf(err, error writing packed-refs. %s,
+   strerror(errno));
+   return -1;
+   }
 
+   data.fd = packed_ref_cache-lock-fd;
+   data.err = err;
do_for_each_entry_in_dir(get_packed_ref_dir(packed_ref_cache),
-0, write_packed_entry_fn,
-packed_ref_cache-lock-fd);
+0, write_packed_entry_fn, data);
if (commit_lock_file(packed_ref_cache-lock)) {
-   save_errno = errno;
+   strbuf_addf(err, unable to overwrite old ref-pack 
+   file. %s, strerror(errno));
error = -1;
}
packed_ref_cache-lock = NULL;

[PATCH 0/5] ref-transactions-req-strbuf-err

2014-08-19 Thread Ronnie Sahlberg
List,

This is the next patch series in the ref transaction work.
This patch series is called ref-transactions-req-strbuf-err and builds ontop
of the series called ref-transactions-req-packed-refs which is origin/pu


This patch series mainly adds some nice strbuf arguments to some functions to
pass errors back to callers.
The only thing noteworthy is that we finally get to remove
-enum action_on_err {
-   UPDATE_REFS_MSG_ON_ERR,
-   UPDATE_REFS_DIE_ON_ERR,
-   UPDATE_REFS_QUIET_ON_ERR
-};

aside from that there is little/nothing much interesting in there.


Ronnie Sahlberg (5):
  refs.c: replace the onerr argument in update_ref with a strbuf err
  refs.c: make add_packed_ref return an error instead of calling die
  refs.c: make lock_packed_refs take an err argument
  refs.c: add an err argument to commit_packed_refs
  refs.c: add an err argument to pack_refs

 builtin/checkout.c   |   7 ++-
 builtin/clone.c  |  23 +---
 builtin/merge.c  |  20 ---
 builtin/notes.c  |  24 +
 builtin/pack-refs.c  |   8 ++-
 builtin/reset.c  |  12 +++--
 builtin/update-ref.c |   7 ++-
 notes-cache.c|   2 +-
 notes-utils.c|   5 +-
 refs.c   | 148 +--
 refs.h   |  13 ++---
 transport-helper.c   |   7 ++-
 transport.c  |   9 ++--
 13 files changed, 170 insertions(+), 115 deletions(-)

-- 
2.0.1.556.ge8f7cba.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 5/5] refs.c: add an err argument to pack_refs

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/pack-refs.c |  8 +++-
 refs.c  | 13 ++---
 refs.h  |  3 ++-
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/builtin/pack-refs.c b/builtin/pack-refs.c
index b20b1ec..299768e 100644
--- a/builtin/pack-refs.c
+++ b/builtin/pack-refs.c
@@ -10,6 +10,7 @@ static char const * const pack_refs_usage[] = {
 int cmd_pack_refs(int argc, const char **argv, const char *prefix)
 {
unsigned int flags = PACK_REFS_PRUNE;
+   struct strbuf err = STRBUF_INIT;
struct option opts[] = {
OPT_BIT(0, all,   flags, N_(pack everything), 
PACK_REFS_ALL),
OPT_BIT(0, prune, flags, N_(prune loose refs (default)), 
PACK_REFS_PRUNE),
@@ -17,5 +18,10 @@ int cmd_pack_refs(int argc, const char **argv, const char 
*prefix)
};
if (parse_options(argc, argv, prefix, opts, pack_refs_usage, 0))
usage_with_options(pack_refs_usage, opts);
-   return pack_refs(flags);
+   if (pack_refs(flags, err)) {
+   error(%s, err.buf);
+   strbuf_release(err);
+   return -1;
+   }
+   return 0;
 }
diff --git a/refs.c b/refs.c
index 19e73f3..5875c29 100644
--- a/refs.c
+++ b/refs.c
@@ -2328,7 +2328,7 @@ static int commit_packed_refs(struct strbuf *err)
strbuf_addf(err, error writing packed-refs. %s,
strerror(errno));
return -1;
-   }
+   }
 
data.fd = packed_ref_cache-lock-fd;
data.err = err;
@@ -2482,24 +2482,23 @@ static void prune_refs(struct ref_to_prune *r)
}
 }
 
-int pack_refs(unsigned int flags)
+int pack_refs(unsigned int flags, struct strbuf *err)
 {
struct pack_refs_cb_data cbdata;
-   struct strbuf err = STRBUF_INIT;
 
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
 
-   if (lock_packed_refs(err))
-   die(%s, err.buf);
+   if (lock_packed_refs(err))
+   return -1;
 
cbdata.packed_refs = get_packed_refs(ref_cache);
 
do_for_each_entry_in_dir(get_loose_refs(ref_cache), 0,
 pack_if_possible_fn, cbdata);
 
-   if (commit_packed_refs(err))
-   die(%s, err.buf);
+   if (commit_packed_refs(err))
+   return -1;
 
prune_refs(cbdata.ref_to_prune);
return 0;
diff --git a/refs.h b/refs.h
index dee9a98..1a98e27 100644
--- a/refs.h
+++ b/refs.h
@@ -122,8 +122,9 @@ extern void warn_dangling_symrefs(FILE *fp, const char 
*msg_fmt, const struct st
 /*
  * Write a packed-refs file for the current repository.
  * flags: Combination of the above PACK_REFS_* flags.
+ * Returns 0 on success and fills in err on failure.
  */
-int pack_refs(unsigned int flags);
+int pack_refs(unsigned int flags, struct strbuf *err);
 
 extern int ref_exists(const char *);
 
-- 
2.0.1.556.ge8f7cba.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 3/5] refs.c: make lock_packed_refs take an err argument

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 25 +
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index 0aad8c8..cfe1292 100644
--- a/refs.c
+++ b/refs.c
@@ -2270,13 +2270,17 @@ static int write_packed_entry_fn(struct ref_entry 
*entry, void *cb_data)
return 0;
 }
 
-/* This should return a meaningful errno on failure */
-static int lock_packed_refs(int flags)
+static int lock_packed_refs(struct strbuf *err)
 {
struct packed_ref_cache *packed_ref_cache;
 
-   if (hold_lock_file_for_update(packlock, git_path(packed-refs), 
flags)  0)
+   if (hold_lock_file_for_update(packlock, git_path(packed-refs),
+ 0)  0) {
+   if (err)
+   unable_to_lock_message(git_path(packed-refs),
+  errno, err);
return -1;
+   }
/*
 * Get the current packed-refs while holding the lock.  If the
 * packed-refs file has been modified since we last read it,
@@ -2460,11 +2464,14 @@ static void prune_refs(struct ref_to_prune *r)
 int pack_refs(unsigned int flags)
 {
struct pack_refs_cb_data cbdata;
+   struct strbuf err = STRBUF_INIT;
 
memset(cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
 
-   lock_packed_refs(LOCK_DIE_ON_ERROR);
+   if (lock_packed_refs(err))
+   die(%s, err.buf);
+
cbdata.packed_refs = get_packed_refs(ref_cache);
 
do_for_each_entry_in_dir(get_loose_refs(ref_cache), 0,
@@ -3626,10 +3633,7 @@ int transaction_commit(struct ref_transaction 
*transaction,
}
 
/* Lock packed refs during commit */
-   if (lock_packed_refs(0)) {
-   if (err)
-   unable_to_lock_message(git_path(packed-refs),
-  errno, err);
+   if (lock_packed_refs(err)) {
ret = -1;
goto cleanup;
}
@@ -3684,10 +3688,7 @@ int transaction_commit(struct ref_transaction 
*transaction,
goto cleanup;
}
/* lock the packed refs again so no one can change it */
-   if (lock_packed_refs(0)) {
-   if (err)
-   unable_to_lock_message(git_path(packed-refs),
-  errno, err);
+   if (lock_packed_refs(err)) {
ret = -1;
goto cleanup;
}
-- 
2.0.1.556.ge8f7cba.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 1/5] refs.c: replace the onerr argument in update_ref with a strbuf err

2014-08-19 Thread Ronnie Sahlberg
Get rid of the action_on_err enum and replace the action argument to
update_ref with a strbuf *err for error reporting.

Update all callers to the new api including two callers in transport*.c
which used the literal 0 instead of an enum.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/checkout.c   |  7 +--
 builtin/clone.c  | 23 +++
 builtin/merge.c  | 20 +---
 builtin/notes.c  | 24 ++--
 builtin/reset.c  | 12 
 builtin/update-ref.c |  7 +--
 notes-cache.c|  2 +-
 notes-utils.c|  5 +++--
 refs.c   | 14 +++---
 refs.h   | 10 ++
 transport-helper.c   |  7 ++-
 transport.c  |  9 ++---
 12 files changed, 81 insertions(+), 59 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 808c58f..a9ec5be 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -580,6 +580,8 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
 {
struct strbuf msg = STRBUF_INIT;
const char *old_desc, *reflog_msg;
+   struct strbuf err = STRBUF_INIT;
+
if (opts-new_branch) {
if (opts-new_orphan_branch) {
if (opts-new_branch_log  !log_all_ref_updates) {
@@ -617,8 +619,9 @@ static void update_refs_for_switch(const struct 
checkout_opts *opts,
if (!strcmp(new-name, HEAD)  !new-path  !opts-force_detach) {
/* Nothing to do. */
} else if (opts-force_detach || !new-path) {  /* No longer on any 
branch. */
-   update_ref(msg.buf, HEAD, new-commit-object.sha1, NULL,
-  REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+   if (update_ref(msg.buf, HEAD, new-commit-object.sha1, NULL,
+  REF_NODEREF, err))
+   die(%s, err.buf);
if (!opts-quiet) {
if (old-path  advice_detached_head)
detach_advice(new-name);
diff --git a/builtin/clone.c b/builtin/clone.c
index 7737e12..99e23cf 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -521,6 +521,7 @@ static void write_remote_refs(const struct ref *local_refs)
 static void write_followtags(const struct ref *refs, const char *msg)
 {
const struct ref *ref;
+   struct strbuf err = STRBUF_INIT;
for (ref = refs; ref; ref = ref-next) {
if (!starts_with(ref-name, refs/tags/))
continue;
@@ -528,8 +529,9 @@ static void write_followtags(const struct ref *refs, const 
char *msg)
continue;
if (!has_sha1_file(ref-old_sha1))
continue;
-   update_ref(msg, ref-name, ref-old_sha1,
-  NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+   if (update_ref(msg, ref-name, ref-old_sha1,
+  NULL, 0, err))
+   die(%s, err.buf);
}
 }
 
@@ -592,28 +594,33 @@ static void update_remote_refs(const struct ref *refs,
 static void update_head(const struct ref *our, const struct ref *remote,
const char *msg)
 {
+   struct strbuf err = STRBUF_INIT;
+
if (our  starts_with(our-name, refs/heads/)) {
/* Local default branch link */
create_symref(HEAD, our-name, NULL);
if (!option_bare) {
const char *head = skip_prefix(our-name, 
refs/heads/);
-   update_ref(msg, HEAD, our-old_sha1, NULL, 0,
-  UPDATE_REFS_DIE_ON_ERR);
+   if (update_ref(msg, HEAD, our-old_sha1, NULL, 0,
+  err))
+   die(%s, err.buf);
install_branch_config(0, head, option_origin, 
our-name);
}
} else if (our) {
struct commit *c = lookup_commit_reference(our-old_sha1);
/* --branch specifies a non-branch (i.e. tags), detach HEAD */
-   update_ref(msg, HEAD, c-object.sha1,
-  NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+   if (update_ref(msg, HEAD, c-object.sha1,
+  NULL, REF_NODEREF, err))
+   die(%s, err.buf);
} else if (remote) {
/*
 * We know remote HEAD points to a non-branch, or
 * HEAD points to a branch but we don't know which one.
 * Detach HEAD in all these cases.
 */
-   update_ref(msg, HEAD, remote-old_sha1,
-  NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR);
+ if (update_ref(msg, HEAD, remote-old_sha1,
+NULL, REF_NODEREF, err))
+   die(%s, err.buf);
}
 }
 
diff --git a/builtin/merge.c b/builtin/merge.c
index 

[PATCH 0/5] ref-transactions-send-pack

2014-08-19 Thread Ronnie Sahlberg
List,

This small patch series adds atomic-push support for pushes.
By default git will use the old style non-atomic updates for pushes,
as not to cause disruption in client scripts that may depend on that
behaviour.

Command line arguments are introduced to allow the client side to request/
negotiate atomic pushes if the remote repo supports it.
There is also a new configuration variable where a repo can set that it
wants all pushes to become atomic whether the client requests it or not.

This patch series is called ref-transactions-send-pack and depends on/is built
ontop of the series called ref-transactions-req-strbuf-err

Ronnie Sahlberg (5):
  receive-pack.c: add protocol support to negotiate atomic-push
  send-pack.c: add an --atomic-push command line argument
  receive-pack.c: use a single transaction when atomic-push is
negotiated
  receive-pack.c: add receive.atomicpush configuration option
  push.c: add an --atomic-push argument

 Documentation/config.txt  |  5 ++
 Documentation/git-push.txt|  7 ++-
 Documentation/git-send-pack.txt   |  7 ++-
 Documentation/technical/protocol-capabilities.txt |  7 +++
 builtin/push.c|  2 +
 builtin/receive-pack.c| 66 ++-
 builtin/send-pack.c   |  6 ++-
 send-pack.c   | 18 +--
 send-pack.h   |  1 +
 transport.c   |  1 +
 transport.h   |  1 +
 11 files changed, 103 insertions(+), 18 deletions(-)

-- 
2.0.1.556.ge8f7cba.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 5/5] push.c: add an --atomic-push argument

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Documentation/git-push.txt | 7 ++-
 builtin/push.c | 2 ++
 transport.c| 1 +
 transport.h| 1 +
 4 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21cd455..b80b0ac 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
 SYNOPSIS
 
 [verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] 
[--receive-pack=git-receive-pack]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n | 
--dry-run] [--receive-pack=git-receive-pack]
   [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u 
| --set-upstream]
   [--force-with-lease[=refname[:expect]]]
   [--no-verify] [repository [refspec...]]
@@ -129,6 +129,11 @@ already exists on the remote side.
from the remote but are pointing at commit-ish that are
reachable from the refs being pushed.
 
+--atomic-push::
+   Try using atomic push. If atomic push is negotiated with the server
+   then any push covering multiple refs will be atomic. Either all
+   refs are updated, or on error, no refs are updated.
+
 --receive-pack=git-receive-pack::
 --exec=git-receive-pack::
Path to the 'git-receive-pack' program on the remote
diff --git a/builtin/push.c b/builtin/push.c
index f8dfea4..f37390c 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -507,6 +507,8 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, no-verify, flags, N_(bypass pre-push hook), 
TRANSPORT_PUSH_NO_HOOK),
OPT_BIT(0, follow-tags, flags, N_(push missing but relevant 
tags),
TRANSPORT_PUSH_FOLLOW_TAGS),
+   OPT_BIT(0, atomic-push, flags, N_(use atomic push, if 
available),
+   TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
 
diff --git a/transport.c b/transport.c
index a3b7f48..ab5f553 100644
--- a/transport.c
+++ b/transport.c
@@ -837,6 +837,7 @@ static int git_transport_push(struct transport *transport, 
struct ref *remote_re
args.progress = transport-progress;
args.dry_run = !!(flags  TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags  TRANSPORT_PUSH_PORCELAIN);
+   args.use_atomic_push = !!(flags  TRANSPORT_ATOMIC_PUSH);
 
ret = send_pack(args, data-fd, data-conn, remote_refs,
data-extra_have);
diff --git a/transport.h b/transport.h
index 02ea248..407d641 100644
--- a/transport.h
+++ b/transport.h
@@ -123,6 +123,7 @@ struct transport {
 #define TRANSPORT_RECURSE_SUBMODULES_ON_DEMAND 256
 #define TRANSPORT_PUSH_NO_HOOK 512
 #define TRANSPORT_PUSH_FOLLOW_TAGS 1024
+#define TRANSPORT_ATOMIC_PUSH 2048
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - 
gettext_width(x)), (x)
-- 
2.0.1.556.ge8f7cba.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 4/5] receive-pack.c: add receive.atomicpush configuration option

2014-08-19 Thread Ronnie Sahlberg
Add a configuration argument to the receive side to force atomic pushes
for all pushes to the repo.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Documentation/config.txt | 5 +
 builtin/receive-pack.c   | 5 +
 2 files changed, 10 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1d718bd..75ce157 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2038,6 +2038,11 @@ rebase.autostash::
successful rebase might result in non-trivial conflicts.
Defaults to false.
 
+receive.atomicpush::
+   By default, git-receive-pack will only use atomic ref transactions
+   if the client negotiates it. When set to true, git-receive-pack
+   will force atomic ref updates for all client pushes.
+
 receive.autogc::
By default, git-receive-pack will run git-gc --auto after
receiving data from git-push and updating refs.  You can stop
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 47f778d..9fa637a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -132,6 +132,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, receive.atomicpush) == 0) {
+   use_atomic_push = git_config_bool(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
-- 
2.0.1.556.ge8f7cba.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 2/5] send-pack.c: add an --atomic-push command line argument

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Documentation/git-send-pack.txt | 7 ++-
 builtin/send-pack.c | 6 +-
 send-pack.c | 8 +++-
 send-pack.h | 1 +
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index dc3a568..4ee2ca1 100644
--- a/Documentation/git-send-pack.txt
+++ b/Documentation/git-send-pack.txt
@@ -9,7 +9,7 @@ git-send-pack - Push objects over Git protocol to another 
repository
 SYNOPSIS
 
 [verse]
-'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
[ref...]
+'git send-pack' [--all] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
[host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -52,6 +52,11 @@ OPTIONS
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
 
+--atomic-push::
+   With atomic-push all refs are updated in one single atomic transaction.
+   This means that if any of the refs fails then the entire push will
+   fail without changing any refs.
+
 host::
A remote host to house the repository.  When this
part is specified, 'git-receive-pack' is invoked via
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index f420b74..78e7d8f 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -13,7 +13,7 @@
 #include sha1-array.h
 
 static const char send_pack_usage[] =
-git send-pack [--all | --mirror] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [host:]directory 
[ref...]\n
+git send-pack [--all | --mirror] [--dry-run] [--force] 
[--receive-pack=git-receive-pack] [--verbose] [--thin] [--atomic-push] 
[host:]directory [ref...]\n
   --all and explicit ref specification are mutually exclusive.;
 
 static struct send_pack_args args;
@@ -165,6 +165,10 @@ int cmd_send_pack(int argc, const char **argv, const char 
*prefix)
args.use_thin_pack = 1;
continue;
}
+   if (!strcmp(arg, --atomic-push)) {
+   args.use_atomic_push = 1;
+   continue;
+   }
if (!strcmp(arg, --stateless-rpc)) {
args.stateless_rpc = 1;
continue;
diff --git a/send-pack.c b/send-pack.c
index f91b8d9..66f3724 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -228,6 +228,11 @@ int send_pack(struct send_pack_args *args,
if (server_supports(atomic-push))
atomic_push_supported = 1;
 
+   if (args-use_atomic_push  !atomic_push_supported) {
+   fprintf(stderr, Server does not support atomic-push.);
+   return -1;
+   }
+
if (!remote_refs) {
fprintf(stderr, No refs in common and none specified; doing 
nothing.\n
Perhaps you should specify a branch such as 
'master'.\n);
@@ -272,7 +277,8 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported  (args-quiet || 
!args-progress);
-   int atomic_push = atomic_push_supported;
+   int atomic_push = atomic_push_supported 
+   args-use_atomic_push;
 
if (!cmds_sent  (status_report || use_sideband ||
   quiet || agent_supported ||
diff --git a/send-pack.h b/send-pack.h
index 8e84392..0374ed8 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -10,6 +10,7 @@ struct send_pack_args {
force_update:1,
use_thin_pack:1,
use_ofs_delta:1,
+   use_atomic_push:1,
dry_run:1,
stateless_rpc:1;
 };
-- 
2.0.1.556.ge8f7cba.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 3/5] receive-pack.c: use a single transaction when atomic-push is negotiated

2014-08-19 Thread Ronnie Sahlberg
Update receive-pack to use an atomic transaction IFF the client negotiated
that it wanted atomic-push.
This leaves the default behaviour to be the old non-atomic one ref at a
time update. This is to cause as little disruption as possible to existing
clients. It is unknown if there are client scripts that depend on the old
non-atomic behaviour so we make it opt-in for now.

Later patch in this series also adds a configuration variable where you can
override the atomic push behaviour on the receiving repo and force it
to use atomic updates always.

If it turns out over time that there are no client scripts that depend on the
old behaviour we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Documentation/technical/protocol-capabilities.txt |  7 +++
 builtin/receive-pack.c| 55 ++-
 2 files changed, 51 insertions(+), 11 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index e174343..93d99c5 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -250,3 +250,10 @@ allow-tip-sha1-in-want
 If the upload-pack server advertises this capability, fetch-pack may
 send want lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
+
+atomic-push
+---
+
+If the receive-pack server advertises the 'atomic-push' capability, it means
+that it is capable of accepting atomic pushes. An atomic push is a push
+where the ref updates are done as a single atomic transaction.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f6b20cb..47f778d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -47,6 +47,8 @@ static void *head_name_to_free;
 static int sent_capabilities;
 static int shallow_update;
 static const char *alt_shallow_file;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -577,26 +579,38 @@ static char *update(struct command *cmd, struct 
shallow_info *si)
return NULL; /* good */
}
else {
-   struct strbuf err = STRBUF_INIT;
-   struct ref_transaction *transaction;
-
if (shallow_update  si-shallow_ref[cmd-index] 
update_shallow_ref(cmd, si))
return xstrdup(shallow error);
-
-   transaction = transaction_begin(err);
-   if (!transaction ||
-   transaction_update_sha1(transaction, namespaced_name,
+   if (!use_atomic_push) {
+   transaction = transaction_begin(err);
+   if (!transaction) {
+   char *str = xstrdup(err.buf);
+
+   strbuf_release(err);
+   transaction_free(transaction);
+   rp_error(%s, str);
+   return str;
+   }
+   }
+   if (transaction_update_sha1(transaction, namespaced_name,
new_sha1, old_sha1, 0, 1, push,
-   err) ||
-   transaction_commit(transaction, err)) {
-   char *str = strbuf_detach(err, NULL);
-   transaction_free(transaction);
+   err)) {
+   char *str = xstrdup(err.buf);
 
+   strbuf_release(err);
+   transaction_free(transaction);
rp_error(%s, str);
return str;
}
+   if (!use_atomic_push  transaction_commit(transaction, err)) {
+   char *str = xstrdup(err.buf);
 
+   strbuf_release(err);
+   transaction_free(transaction);
+   rp_error(%s, str);
+   return str;
+   }
transaction_free(transaction);
strbuf_release(err);
return NULL; /* good */
@@ -810,6 +824,16 @@ static void execute_commands(struct command *commands,
return;
}
 
+   if (use_atomic_push) {
+   transaction = transaction_begin(err);
+   if (!transaction) {
+   error(%s, err.buf);
+   strbuf_release(err);
+   for (cmd = commands; cmd; cmd = cmd-next)
+   cmd-error_string = transaction error;
+   return;
+   }
+   }
data.cmds = commands;
data.si = si;
if 

[PATCH 1/5] receive-pack.c: add protocol support to negotiate atomic-push

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 builtin/receive-pack.c |  6 +-
 send-pack.c| 12 +---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0565b94..f6b20cb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -36,6 +36,7 @@ static int transfer_unpack_limit = -1;
 static int unpack_limit = 100;
 static int report_status;
 static int use_sideband;
+static int use_atomic_push;
 static int quiet;
 static int prefer_ofs_delta = 1;
 static int auto_update_server_info;
@@ -142,7 +143,8 @@ static void show_ref(const char *path, const unsigned char 
*sha1)
else
packet_write(1, %s %s%c%s%s agent=%s\n,
 sha1_to_hex(sha1), path, 0,
- report-status delete-refs side-band-64k quiet,
+ report-status delete-refs side-band-64k quiet
+ atomic-push,
 prefer_ofs_delta ?  ofs-delta : ,
 git_user_agent_sanitized());
sent_capabilities = 1;
@@ -892,6 +894,8 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, quiet))
quiet = 1;
+   if (parse_feature_request(feature_list, atomic-push))
+   use_atomic_push = 1;
}
cmd = xcalloc(1, sizeof(struct command) + len - 80);
hashcpy(cmd-old_sha1, old_sha1);
diff --git a/send-pack.c b/send-pack.c
index 6129b0f..f91b8d9 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -205,6 +205,7 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+   int atomic_push_supported = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -224,6 +225,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports(no-thin))
args-use_thin_pack = 0;
+   if (server_supports(atomic-push))
+   atomic_push_supported = 1;
 
if (!remote_refs) {
fprintf(stderr, No refs in common and none specified; doing 
nothing.\n
@@ -269,17 +272,20 @@ int send_pack(struct send_pack_args *args,
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported  (args-quiet || 
!args-progress);
+   int atomic_push = atomic_push_supported;
 
if (!cmds_sent  (status_report || use_sideband ||
-  quiet || agent_supported)) {
+  quiet || agent_supported ||
+  atomic_push)) {
packet_buf_write(req_buf,
-%s %s %s%c%s%s%s%s%s,
+%s %s %s%c%s%s%s%s%s%s,
 old_hex, new_hex, ref-name, 0,
 status_report ?  
report-status : ,
 use_sideband ?  
side-band-64k : ,
 quiet ?  quiet : ,
 agent_supported ?  agent= : 
,
-agent_supported ? 
git_user_agent_sanitized() : 
+agent_supported ? 
git_user_agent_sanitized() : ,
+atomic_push ?  atomic-push : 

);
}
else
-- 
2.0.1.556.ge8f7cba.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 v3 05/23] refs.c: move delete_ref to the common code

2014-08-19 Thread Ronnie Sahlberg
This change moves delete_ref() to the refs.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 19 ---
 refs.c  | 19 +++
 2 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index eb66cf7..faf794c 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2622,25 +2622,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
-{
-   struct ref_transaction *transaction;
-   struct strbuf err = STRBUF_INIT;
-
-   transaction = transaction_begin(err);
-   if (!transaction ||
-   transaction_delete_sha1(transaction, refname, sha1, delopt,
-   sha1  !is_null_sha1(sha1), NULL, err) ||
-   transaction_commit(transaction, err)) {
-   error(%s, err.buf);
-   transaction_free(transaction);
-   strbuf_release(err);
-   return 1;
-   }
-   transaction_free(transaction);
-   return 0;
-}
-
 struct rename_reflog_cb {
struct ref_transaction *transaction;
const char *refname;
diff --git a/refs.c b/refs.c
index 5f6a1e7..70b12eb 100644
--- a/refs.c
+++ b/refs.c
@@ -26,3 +26,22 @@ int update_ref(const char *action, const char *refname,
}
return 0;
 }
+
+int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
+{
+   struct ref_transaction *transaction;
+   struct strbuf err = STRBUF_INIT;
+
+   transaction = transaction_begin(err);
+   if (!transaction ||
+   transaction_delete_sha1(transaction, refname, sha1, delopt,
+   sha1  !is_null_sha1(sha1), NULL, err) ||
+   transaction_commit(transaction, err)) {
+   error(%s, err.buf);
+   transaction_free(transaction);
+   strbuf_release(err);
+   return 1;
+   }
+   transaction_free(transaction);
+   return 0;
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 15/23] refs.c: move names_conflict to the common code

2014-08-19 Thread Ronnie Sahlberg
Move names_conflict to the common code and make it public.
We want to use the same name conflict checks across all backends
so we guarantee compatibility across backends.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 14 --
 refs.c  |  8 
 refs.h  |  9 +
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 70c034c..6542969 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -676,20 +676,6 @@ static void prime_ref_dir(struct ref_dir *dir)
prime_ref_dir(get_ref_dir(entry));
}
 }
-/*
- * Return true iff refname1 and refname2 conflict with each other.
- * Two reference names conflict if one of them exactly matches the
- * leading components of the other; e.g., foo/bar conflicts with
- * both foo and with foo/bar/baz but not with foo/bar or
- * foo/barbados.
- */
-static int names_conflict(const char *refname1, const char *refname2)
-{
-   for (; *refname1  *refname1 == *refname2; refname1++, refname2++)
-   ;
-   return (*refname1 == '\0'  *refname2 == '/')
-   || (*refname1 == '/'  *refname2 == '\0');
-}
 
 struct name_conflict_cb {
const char *refname;
diff --git a/refs.c b/refs.c
index 9bc0a31..177bed6 100644
--- a/refs.c
+++ b/refs.c
@@ -688,3 +688,11 @@ int is_branch(const char *refname)
 {
return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
 }
+
+int names_conflict(const char *refname1, const char *refname2)
+{
+   for (; *refname1  *refname1 == *refname2; refname1++, refname2++)
+   ;
+   return (*refname1 == '\0'  *refname2 == '/')
+   || (*refname1 == '/'  *refname2 == '\0');
+}
diff --git a/refs.h b/refs.h
index d526da0..a14fc5d 100644
--- a/refs.h
+++ b/refs.h
@@ -128,6 +128,15 @@ int pack_refs(unsigned int flags, struct strbuf *err);
 
 extern int ref_exists(const char *);
 
+/*
+ * Return true iff refname1 and refname2 conflict with each other.
+ * Two reference names conflict if one of them exactly matches the
+ * leading components of the other; e.g., foo/bar conflicts with
+ * both foo and with foo/bar/baz but not with foo/bar or
+ * foo/barbados.
+ */
+int names_conflict(const char *refname1, const char *refname2);
+
 extern int is_branch(const char *refname);
 
 /*
-- 
2.0.1.552.g1af257a

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


[PATCH v3 04/23] refs.c: move update_ref to refs.c

2014-08-19 Thread Ronnie Sahlberg
This change moves update_ref() to the refs.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 23 ---
 refs.c  | 25 +
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 4a22513..eb66cf7 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -3576,29 +3576,6 @@ int transaction_delete_sha1(struct ref_transaction 
*transaction,
  old_sha1, flags, have_old, msg, err);
 }
 
-int update_ref(const char *action, const char *refname,
-  const unsigned char *sha1, const unsigned char *oldval,
-  int flags, struct strbuf *e)
-{
-   struct ref_transaction *t;
-   struct strbuf err = STRBUF_INIT;
-
-   t = transaction_begin(err);
-   if (!t ||
-   transaction_update_sha1(t, refname, sha1, oldval, flags,
-   !!oldval, action, err) ||
-   transaction_commit(t, err)) {
-   const char *str = update_ref failed for ref '%s': %s;
-
-   transaction_free(t);
-   if (e)
-   strbuf_addf(e, str, refname, err.buf);
-   strbuf_release(err);
-   return 1;
-   }
-   return 0;
-}
-
 static int ref_update_compare(const void *r1, const void *r2)
 {
const struct ref_update * const *u1 = r1;
diff --git a/refs.c b/refs.c
index 77492ff..5f6a1e7 100644
--- a/refs.c
+++ b/refs.c
@@ -1,3 +1,28 @@
 /*
  * Common refs code for all backends.
  */
+#include cache.h
+#include refs.h
+
+int update_ref(const char *action, const char *refname,
+  const unsigned char *sha1, const unsigned char *oldval,
+  int flags, struct strbuf *e)
+{
+   struct ref_transaction *t;
+   struct strbuf err = STRBUF_INIT;
+
+   t = transaction_begin(err);
+   if (!t ||
+   transaction_update_sha1(t, refname, sha1, oldval, flags,
+   !!oldval, action, err) ||
+   transaction_commit(t, err)) {
+   const char *str = update_ref failed for ref '%s': %s;
+
+   transaction_free(t);
+   if (e)
+   strbuf_addf(e, str, refname, err.buf);
+   strbuf_release(err);
+   return 1;
+   }
+   return 0;
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 06/23] refs.c: move rename_ref to the common code

2014-08-19 Thread Ronnie Sahlberg
This change moves rename_ref() to the refs.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 92 -
 refs.c  | 92 +
 2 files changed, 92 insertions(+), 92 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index faf794c..7d579be 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2622,98 +2622,6 @@ static int delete_ref_loose(struct ref_lock *lock, int 
flag, struct strbuf *err)
return 0;
 }
 
-struct rename_reflog_cb {
-   struct ref_transaction *transaction;
-   const char *refname;
-   struct strbuf *err;
-};
-
-static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
-const char *id, unsigned long timestamp, int tz,
-const char *message, void *cb_data)
-{
-   struct rename_reflog_cb *cb = cb_data;
-   struct reflog_committer_info ci;
-
-   memset(ci, 0, sizeof(ci));
-   ci.id = id;
-   ci.timestamp = timestamp;
-   ci.tz = tz;
-   return transaction_update_reflog(cb-transaction, cb-refname,
-nsha1, osha1, ci, message, 0,
-cb-err);
-}
-
-int rename_ref(const char *oldrefname, const char *newrefname, const char 
*logmsg)
-{
-   unsigned char sha1[20];
-   int flag = 0, log;
-   struct ref_transaction *transaction = NULL;
-   struct strbuf err = STRBUF_INIT;
-   const char *symref = NULL;
-   struct rename_reflog_cb cb;
-   struct reflog_committer_info ci;
-
-   memset(ci, 0, sizeof(ci));
-   ci.committer_info = git_committer_info(0);
-
-   symref = resolve_ref_unsafe(oldrefname, sha1,
-   RESOLVE_REF_READING, flag);
-   if (flag  REF_ISSYMREF) {
-   error(refname %s is a symbolic ref, renaming it is not 
supported,
-   oldrefname);
-   return 1;
-   }
-   if (!symref) {
-   error(refname %s not found, oldrefname);
-   return 1;
-   }
-
-   if (!is_refname_available(newrefname, oldrefname, 1))
-   return 1;
-
-   log = reflog_exists(oldrefname);
-   transaction = transaction_begin(err);
-   if (!transaction)
-   goto fail;
-
-   if (strcmp(oldrefname, newrefname)) {
-   if (log  transaction_update_reflog(transaction, newrefname,
-sha1, sha1, ci, NULL,
-REFLOG_TRUNCATE, err))
-   goto fail;
-   cb.transaction = transaction;
-   cb.refname = newrefname;
-   cb.err = err;
-   if (log  for_each_reflog_ent(oldrefname, rename_reflog_ent,
-  cb))
-   goto fail;
-
-   if (transaction_delete_sha1(transaction, oldrefname, sha1,
-   REF_NODEREF,
-   1, NULL, err))
-   goto fail;
-   }
-   if (transaction_update_sha1(transaction, newrefname, sha1,
-   NULL, 0, 0, NULL, err))
-   goto fail;
-   if (log  transaction_update_reflog(transaction, newrefname, sha1,
-sha1, ci, logmsg,
-REFLOG_COMMITTER_INFO_IS_VALID,
-err))
-   goto fail;
-   if (transaction_commit(transaction, err))
-   goto fail;
-   transaction_free(transaction);
-   return 0;
-
- fail:
-   error(rename_ref failed: %s, err.buf);
-   strbuf_release(err);
-   transaction_free(transaction);
-   return 1;
-}
-
 static int close_ref(struct ref_lock *lock)
 {
if (close_lock_file(lock-lk))
diff --git a/refs.c b/refs.c
index 70b12eb..319eafa 100644
--- a/refs.c
+++ b/refs.c
@@ -45,3 +45,95 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
transaction_free(transaction);
return 0;
 }
+
+struct rename_reflog_cb {
+   struct ref_transaction *transaction;
+   const char *refname;
+   struct strbuf *err;
+};
+
+static int rename_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+const char *id, unsigned long timestamp, int tz,
+const char *message, void *cb_data)
+{
+   struct rename_reflog_cb *cb = cb_data;
+   struct reflog_committer_info ci;
+
+   memset(ci, 0, sizeof(ci));
+   ci.id = id;
+   ci.timestamp = timestamp;
+   ci.tz = tz;
+   return transaction_update_reflog(cb-transaction, cb-refname,
+

[PATCH v3 08/23] refs.c: move the hidden refs functions to the common code

2014-08-19 Thread Ronnie Sahlberg
This change moves the hidden refs functions to the refs.c file since
these functions do not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 43 ---
 refs.c  | 44 
 2 files changed, 44 insertions(+), 43 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 52ca0bb..6181edf 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -3796,46 +3796,3 @@ char *shorten_unambiguous_ref(const char *refname, int 
strict)
free(short_name);
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)
-{
-   struct string_list_item *item;
-
-   if (!hide_refs)
-   return 0;
-   for_each_string_list_item(item, hide_refs) {
-   int len;
-   if (!starts_with(refname, item-string))
-   continue;
-   len = strlen(item-string);
-   if (!refname[len] || refname[len] == '/')
-   return 1;
-   }
-   return 0;
-}
diff --git a/refs.c b/refs.c
index 072cd39..9e2059b 100644
--- a/refs.c
+++ b/refs.c
@@ -3,6 +3,7 @@
  */
 #include cache.h
 #include refs.h
+#include string-list.h
 
 int update_ref(const char *action, const char *refname,
   const unsigned char *sha1, const unsigned char *oldval,
@@ -251,3 +252,46 @@ int read_ref_at(const char *refname, unsigned long 
at_time, int cnt,
 
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)
+{
+   struct string_list_item *item;
+
+   if (!hide_refs)
+   return 0;
+   for_each_string_list_item(item, hide_refs) {
+   int len;
+   if (!starts_with(refname, item-string))
+   continue;
+   len = strlen(item-string);
+   if (!refname[len] || refname[len] == '/')
+   return 1;
+   }
+   return 0;
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 20/23] refs-be-files.c: add reflog backend methods

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 21 +++--
 refs.c  | 32 
 refs.h  | 16 
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 27eafd0..464d488 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2251,7 +2251,7 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* This function must set a meaningful errno on failure */
-int create_reflog(const char *refname)
+static int files_create_reflog(const char *refname)
 {
int logfd, oflags = O_APPEND | O_WRONLY;
char logfile[PATH_MAX];
@@ -2516,7 +2516,7 @@ int create_symref(const char *ref_target, const char 
*refs_heads_master,
return 0;
 }
 
-int reflog_exists(const char *refname)
+static int files_reflog_exists(const char *refname)
 {
struct stat st;
 
@@ -2524,7 +2524,7 @@ int reflog_exists(const char *refname)
S_ISREG(st.st_mode);
 }
 
-int delete_reflog(const char *refname)
+static int files_delete_reflog(const char *refname)
 {
return remove_path(git_path(logs/%s, refname));
 }
@@ -2568,7 +2568,9 @@ static char *find_beginning_of_line(char *bob, char *scan)
return scan;
 }
 
-int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, 
void *cb_data)
+static int files_for_each_reflog_ent_reverse(const char *refname,
+each_reflog_ent_fn fn,
+void *cb_data)
 {
struct strbuf sb = STRBUF_INIT;
FILE *logfp;
@@ -2645,7 +2647,8 @@ int for_each_reflog_ent_reverse(const char *refname, 
each_reflog_ent_fn fn, void
return ret;
 }
 
-int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void 
*cb_data)
+static int files_for_each_reflog_ent(const char *refname,
+each_reflog_ent_fn fn, void *cb_data)
 {
FILE *logfp;
struct strbuf sb = STRBUF_INIT;
@@ -2706,7 +2709,7 @@ static int do_for_each_reflog(struct strbuf *name, 
each_ref_fn fn, void *cb_data
return retval;
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+static int files_for_each_reflog(each_ref_fn fn, void *cb_data)
 {
int retval;
struct strbuf name;
@@ -3295,6 +3298,12 @@ struct ref_be refs_files = {
files_transaction_update_reflog,
files_transaction_commit,
files_transaction_free,
+   files_for_each_reflog_ent,
+   files_for_each_reflog_ent_reverse,
+   files_for_each_reflog,
+   files_reflog_exists,
+   files_create_reflog,
+   files_delete_reflog,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.c b/refs.c
index b8c942f..2db1a74 100644
--- a/refs.c
+++ b/refs.c
@@ -856,3 +856,35 @@ void transaction_free(struct ref_transaction *transaction)
 {
return refs-transaction_free(transaction);
 }
+
+int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return refs-for_each_reflog_ent_reverse(refname, fn, cb_data);
+}
+
+int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn,
+   void *cb_data)
+{
+   return refs-for_each_reflog_ent(refname, fn, cb_data);
+}
+
+int for_each_reflog(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_reflog(fn, cb_data);
+}
+
+int reflog_exists(const char *refname)
+{
+   return refs-reflog_exists(refname);
+}
+
+int create_reflog(const char *refname)
+{
+   return refs-create_reflog(refname);
+}
+
+int delete_reflog(const char *refname)
+{
+   return refs-delete_reflog(refname);
+}
diff --git a/refs.h b/refs.h
index 4b669f5..0a68986 100644
--- a/refs.h
+++ b/refs.h
@@ -372,6 +372,16 @@ typedef int (*transaction_update_reflog_fn)(
 typedef int (*transaction_commit_fn)(struct ref_transaction *transaction,
   struct strbuf *err);
 typedef void (*transaction_free_fn)(struct ref_transaction *transaction);
+typedef int (*for_each_reflog_ent_fn)(const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data);
+typedef int (*for_each_reflog_ent_reverse_fn)(const char *refname,
+ each_reflog_ent_fn fn,
+ void *cb_data);
+typedef int (*for_each_reflog_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*reflog_exists_fn)(const char *refname);
+typedef int (*create_reflog_fn)(const char *refname);
+typedef int (*delete_reflog_fn)(const char *refname);
 
 struct ref_be {
transaction_begin_fn transaction_begin;
@@ -381,6 +391,12 @@ struct ref_be {
transaction_update_reflog_fn transaction_update_reflog;
transaction_commit_fn transaction_commit;
transaction_free_fn transaction_free;
+   for_each_reflog_ent_fn 

[PATCH v3 14/23] refs.c: move is_branch to the common code

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 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 55bced9..70c034c 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2483,11 +2483,6 @@ static int log_ref_write(const char *refname, const 
unsigned char *old_sha1,
return 0;
 }
 
-int is_branch(const char *refname)
-{
-   return !strcmp(refname, HEAD) || starts_with(refname, refs/heads/);
-}
-
 static int write_sha1_update_reflog(struct ref_lock *lock,
const unsigned char *sha1, const char *logmsg)
 {
diff --git a/refs.c b/refs.c
index ea5f276..9bc0a31 100644
--- a/refs.c
+++ b/refs.c
@@ -683,3 +683,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.0.1.552.g1af257a

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


[PATCH v3 18/23] refs.c: move head_ref_namespaced to the common code

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 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 9aa88ef..e58a7e1 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1635,21 +1635,6 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
-{
-   struct strbuf buf = STRBUF_INIT;
-   int ret = 0;
-   unsigned char sha1[20];
-   int flag;
-
-   strbuf_addf(buf, %sHEAD, get_git_namespace());
-   if (!read_ref_full(buf.buf, sha1, RESOLVE_REF_READING, flag))
-   ret = fn(buf.buf, sha1, flag, cb_data);
-   strbuf_release(buf);
-
-   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 964a513..6b434ad 100644
--- a/refs.c
+++ b/refs.c
@@ -786,3 +786,18 @@ 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 head_ref_namespaced(each_ref_fn fn, void *cb_data)
+{
+   struct strbuf buf = STRBUF_INIT;
+   int ret = 0;
+   unsigned char sha1[20];
+   int flag;
+
+   strbuf_addf(buf, %sHEAD, get_git_namespace());
+   if (!read_ref_full(buf.buf, sha1, RESOLVE_REF_READING, flag))
+   ret = fn(buf.buf, sha1, flag, cb_data);
+   strbuf_release(buf);
+
+   return ret;
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 19/23] refs-be-files.c: add a backend method structure with transaction functions

2014-08-19 Thread Ronnie Sahlberg
Add a ref structure for backend methods. Start by adding method pointers
for the transaction functions.

Rename the existing transaction functions to files_* and make them static.
Add new transaction functions that just pass through to the appropriate
methods for the backend.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 68 ++---
 refs.c  | 55 ++
 refs.h  | 35 +
 3 files changed, 131 insertions(+), 27 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index e58a7e1..27eafd0 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2777,12 +2777,12 @@ struct ref_transaction {
enum ref_transaction_state state;
 };
 
-struct ref_transaction *transaction_begin(struct strbuf *err)
+static struct ref_transaction *files_transaction_begin(struct strbuf *err)
 {
return xcalloc(1, sizeof(struct ref_transaction));
 }
 
-void transaction_free(struct ref_transaction *transaction)
+static void files_transaction_free(struct ref_transaction *transaction)
 {
int i;
 
@@ -2812,13 +2812,13 @@ static struct ref_update *add_update(struct 
ref_transaction *transaction,
return update;
 }
 
-int transaction_update_reflog(struct ref_transaction *transaction,
- const char *refname,
- const unsigned char *new_sha1,
- const unsigned char *old_sha1,
- struct reflog_committer_info *ci,
- const char *msg, int flags,
- struct strbuf *err)
+static int files_transaction_update_reflog(struct ref_transaction *transaction,
+  const char *refname,
+  const unsigned char *new_sha1,
+  const unsigned char *old_sha1,
+  struct reflog_committer_info *ci,
+  const char *msg, int flags,
+  struct strbuf *err)
 {
struct ref_update *update;
int i;
@@ -2865,12 +2865,13 @@ int transaction_update_reflog(struct ref_transaction 
*transaction,
return 0;
 }
 
-int transaction_update_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *new_sha1,
-   const unsigned char *old_sha1,
-   int flags, int have_old, const char *msg,
-   struct strbuf *err)
+static int files_transaction_update_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+const unsigned char *old_sha1,
+int flags, int have_old,
+const char *msg,
+struct strbuf *err)
 {
struct ref_update *update;
 
@@ -2897,11 +2898,11 @@ int transaction_update_sha1(struct ref_transaction 
*transaction,
return 0;
 }
 
-int transaction_create_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *new_sha1,
-   int flags, const char *msg,
-   struct strbuf *err)
+static int files_transaction_create_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *new_sha1,
+int flags, const char *msg,
+struct strbuf *err)
 {
if (transaction-state != REF_TRANSACTION_OPEN)
die(BUG: create called for transaction that is not open);
@@ -2913,11 +2914,12 @@ int transaction_create_sha1(struct ref_transaction 
*transaction,
   null_sha1, flags, 1, msg, err);
 }
 
-int transaction_delete_sha1(struct ref_transaction *transaction,
-   const char *refname,
-   const unsigned char *old_sha1,
-   int flags, int have_old, const char *msg,
-   struct strbuf *err)
+static int files_transaction_delete_sha1(struct ref_transaction *transaction,
+const char *refname,
+const unsigned char *old_sha1,
+int flags, int have_old,
+const char *msg,
+struct strbuf *err)
 {
if (transaction-state != 

[PATCH v3 07/23] refs.c: move read_ref_at to the common refs file

2014-08-19 Thread Ronnie Sahlberg
This change moves read_ref_at() to the refs.c file since this function
does not contain any backend specific code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 114 
 refs.c  | 114 
 2 files changed, 114 insertions(+), 114 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 7d579be..52ca0bb 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -2935,120 +2935,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 *id, 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_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_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 *id, 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 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(cb, 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, cb);
-
-   if (!cb.reccnt)
-   die(Log for %s is empty., refname);
-   if (cb.found_it)
-   return 0;
-
-   for_each_reflog_ent(refname, read_ref_at_ent_oldest, cb);
-
-   return 1;
-}
-
 int reflog_exists(const char *refname)
 {
struct stat st;
diff --git a/refs.c b/refs.c
index 319eafa..072cd39 100644
--- a/refs.c
+++ b/refs.c
@@ -137,3 +137,117 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
transaction_free(transaction);
return 1;
 }
+
+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 

[PATCH v3 03/23] refs.c: add a new refs.c file to hold all common refs code

2014-08-19 Thread Ronnie Sahlberg
Create a new erfs.c file that will be used to hold all the refs
code that is backend agnostic and will be shared across all backends.

The reason we renamed everything to refs-be-files.c in the previous patch
and now start moving the common code back to the new refs.c file
instead of the other way around is the etive volumes of code.

With the ref_cache, packed refs and loose ref handling that are all
part of the files based implementation the backend specific part
of the old refs.c file is several times larger than the backend agnostic
part. Therefore it makes more sense to first rename everything to be
part of the files based backend and then move the parts that can be used
as common code back to refs.c.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 Makefile | 1 +
 refs.c   | 3 +++
 2 files changed, 4 insertions(+)
 create mode 100644 refs.c

diff --git a/Makefile b/Makefile
index e010ad1..937d22a 100644
--- a/Makefile
+++ b/Makefile
@@ -857,6 +857,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 += 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.0.1.552.g1af257a

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


[PATCH v3 17/23] refs.c: move ref iterators to the common code

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 81 -
 refs.c  | 81 +
 2 files changed, 81 insertions(+), 81 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index fb9c614..9aa88ef 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1377,22 +1377,6 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int fla
}
 }
 
-/* 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 unsigned char *sha1, 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, sha1, flags, filter-cb_data);
-}
-
 enum peel_status {
/* object was peeled successfully: */
PEEL_PEELED = 0,
@@ -1646,36 +1630,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(ref_cache, refs/replace/, fn, 13, 0, cb_data);
@@ -1706,41 +1660,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(real_pattern, refs/);
-   else if (prefix)
-   strbuf_addstr(real_pattern, prefix);
-   strbuf_addstr(real_pattern, pattern);
-
-   if (!has_glob_specials(pattern)) {
-   /* Append implied '/' '*' if not present. */
-   if (real_pattern.buf[real_pattern.len - 1] != '/')
-   strbuf_addch(real_pattern, '/');
-   /* No need to check for '*', there is none. */
-   strbuf_addch(real_pattern, '*');
-   }
-
-   filter.pattern = real_pattern.buf;
-   filter.fn = fn;
-   filter.cb_data = cb_data;
-   ret = for_each_ref(filter_refs, filter);
-
-   strbuf_release(real_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(ref_cache, , fn, 0,
diff --git a/refs.c b/refs.c
index d163e61..964a513 100644
--- a/refs.c
+++ b/refs.c
@@ -705,3 +705,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 unsigned char *sha1, 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, sha1, flags, filter-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 

[PATCH v3 13/23] refs.c: move check_refname_component to the common code

2014-08-19 Thread Ronnie Sahlberg
This function does not contain any backend specific code so we
can move it to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 110 
 refs.c  | 110 
 2 files changed, 110 insertions(+), 110 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index ed7bc61..55bced9 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -6,25 +6,6 @@
 #include string-list.h
 
 /*
- * 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, ~, ^, : or SP
- */
-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, 4, 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
-};
-
-/*
  * Used as a flag to transaction_delete_sha1 when a loose ref is being
  * pruned.
  */
@@ -35,97 +16,6 @@ static unsigned char refname_disposition[256] = {
  */
 #define UPDATE_REFLOG_NOLOCK 0x0200
 
-/*
- * 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 character, ~, ^, : or SP, anywhere, or
- * - it ends with a /.
- * - it ends with .lock
- * - it contains a \ (backslash)
- */
-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;
-   }
-   last = ch;
-   }
-out:
-   if (cp == refname)
-   return 0; /* Component has zero length. */
-   if (refname[0] == '.') {
-   if (!(flags  REFNAME_DOT_COMPONENT))
-   return -1; /* Component starts with '.'. */
-   /*
-* Even if leading dots are allowed, don't allow .
-* as a component (.. is prevented by a rule above).
-*/
-   if (refname[1] == '\0')
-   return -1; /* Component equals .. */
-   }
-   if (cp - refname = 5  !memcmp(cp - 5, .lock, 5))
-   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, flags);
-   if (component_len = 0) {
-   if ((flags  REFNAME_REFSPEC_PATTERN) 
-   refname[0] == '*' 
-   (refname[1] == '\0' || refname[1] == 
'/')) {
-   /* Accept one wildcard as a full refname 
component. */
-   flags = ~REFNAME_REFSPEC_PATTERN;
-   component_len = 1;
-   } else {
-   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 one component. */
-   return 0;
-}
-
 struct ref_entry;
 
 /*
diff --git a/refs.c b/refs.c
index 

[PATCH v3 22/23] refs-be-files.c: add methods for head_ref*

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c |  7 +--
 refs.c  | 10 ++
 refs.h  |  5 +
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index b09f0fc..910663b 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1603,12 +1603,13 @@ static int do_head_ref(const char *submodule, 
each_ref_fn fn, void *cb_data)
return 0;
 }
 
-int head_ref(each_ref_fn fn, void *cb_data)
+static int files_head_ref(each_ref_fn fn, void *cb_data)
 {
return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+static int files_head_ref_submodule(const char *submodule, each_ref_fn fn,
+   void *cb_data)
 {
return do_head_ref(submodule, fn, cb_data);
 }
@@ -3315,6 +3316,8 @@ struct ref_be refs_files = {
files_peel_ref,
files_create_symref,
files_resolve_gitlink_ref,
+   files_head_ref,
+   files_head_ref_submodule,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.c b/refs.c
index 60b6241..841d905 100644
--- a/refs.c
+++ b/refs.c
@@ -921,3 +921,13 @@ int resolve_gitlink_ref(const char *path, const char 
*refname,
 {
return refs-resolve_gitlink_ref(path, refname, sha1);
 }
+
+int head_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-head_ref(fn, cb_data);
+}
+
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+{
+   return refs-head_ref_submodule(submodule, fn, cb_data);
+}
diff --git a/refs.h b/refs.h
index 5257437..92f8f44 100644
--- a/refs.h
+++ b/refs.h
@@ -394,6 +394,9 @@ typedef int (*create_symref_fn)(const char *ref_target,
const char *logmsg);
 typedef int (*resolve_gitlink_ref_fn)(const char *path, const char *refname,
  unsigned char *sha1);
+typedef int (*head_ref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*head_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
+void *cb_data);
 
 struct ref_be {
transaction_begin_fn transaction_begin;
@@ -415,6 +418,8 @@ struct ref_be {
peel_ref_fn peel_ref;
create_symref_fn create_symref;
resolve_gitlink_ref_fn resolve_gitlink_ref;
+   head_ref_fn head_ref;
+   head_ref_submodule_fn head_ref_submodule;
 };
 
 extern struct ref_be *refs;
-- 
2.0.1.552.g1af257a

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


[PATCH v3 21/23] refs-be-files.c: add methods for misc ref operations

2014-08-19 Thread Ronnie Sahlberg
Add ref backend methods for:
resolve_ref_unsafe, is_refname_available, pack_refs, peel_ref,
create_symref, resolve_gitlink_ref.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 25 ++---
 refs.c  | 33 +
 refs.h  | 18 ++
 3 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 464d488..b09f0fc 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1114,7 +1114,8 @@ static struct ref_dir *get_loose_refs(struct ref_cache 
*refs)
return get_ref_dir(refs-loose);
 }
 
-int is_refname_available(const char *refname, const char **skip, int skipnum)
+static int files_is_refname_available(const char *refname, const char **skip,
+ int skipnum)
 {
if (!is_refname_available_dir(refname, get_packed_refs(ref_cache),
  skip, skipnum))
@@ -1188,7 +1189,8 @@ static int resolve_gitlink_ref_recursive(struct ref_cache 
*refs,
return resolve_gitlink_ref_recursive(refs, p, sha1, recursion+1);
 }
 
-int resolve_gitlink_ref(const char *path, const char *refname, unsigned char 
*sha1)
+static int files_resolve_gitlink_ref(const char *path, const char *refname,
+unsigned char *sha1)
 {
int len = strlen(path), retval;
char *submodule;
@@ -1247,7 +1249,9 @@ static const char *handle_missing_loose_ref(const char 
*refname,
 }
 
 /* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int 
flags, int *ref_flag)
+static const char *files_resolve_ref_unsafe(const char *refname,
+   unsigned char *sha1, int flags,
+   int *ref_flag)
 {
int depth = MAXDEPTH;
ssize_t len;
@@ -1466,7 +1470,7 @@ static enum peel_status peel_entry(struct ref_entry 
*entry, int repeel)
return status;
 }
 
-int peel_ref(const char *refname, unsigned char *sha1)
+static int files_peel_ref(const char *refname, unsigned char *sha1)
 {
int flag;
unsigned char base[20];
@@ -2080,7 +2084,7 @@ static void prune_refs(struct ref_to_prune *r)
}
 }
 
-int pack_refs(unsigned int flags, struct strbuf *err)
+static int files_pack_refs(unsigned int flags, struct strbuf *err)
 {
struct pack_refs_cb_data cbdata;
 
@@ -2453,8 +2457,9 @@ static int write_ref_sha1(struct ref_lock *lock,
return 0;
 }
 
-int create_symref(const char *ref_target, const char *refs_heads_master,
- const char *logmsg)
+static int files_create_symref(const char *ref_target,
+  const char *refs_heads_master,
+  const char *logmsg)
 {
const char *lockpath;
char ref[1000];
@@ -3304,6 +3309,12 @@ struct ref_be refs_files = {
files_reflog_exists,
files_create_reflog,
files_delete_reflog,
+   files_resolve_ref_unsafe,
+   files_is_refname_available,
+   files_pack_refs,
+   files_peel_ref,
+   files_create_symref,
+   files_resolve_gitlink_ref,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.c b/refs.c
index 2db1a74..60b6241 100644
--- a/refs.c
+++ b/refs.c
@@ -888,3 +888,36 @@ int delete_reflog(const char *refname)
 {
return refs-delete_reflog(refname);
 }
+
+const char *resolve_ref_unsafe(const char *ref, unsigned char *sha1,
+  int reading, int *flag)
+{
+   return refs-resolve_ref_unsafe(ref, sha1, reading, flag);
+}
+
+int is_refname_available(const char *refname, const char **skip, int skipnum)
+{
+   return refs-is_refname_available(refname, skip, skipnum);
+}
+
+int pack_refs(unsigned int flags, struct strbuf *err)
+{
+   return refs-pack_refs(flags, err);
+}
+
+int peel_ref(const char *refname, unsigned char *sha1)
+{
+   return refs-peel_ref(refname, sha1);
+}
+
+int create_symref(const char *ref_target, const char *refs_heads_master,
+ const char *logmsg)
+{
+   return refs-create_symref(ref_target, refs_heads_master, logmsg);
+}
+
+int resolve_gitlink_ref(const char *path, const char *refname,
+   unsigned char *sha1)
+{
+   return refs-resolve_gitlink_ref(path, refname, sha1);
+}
diff --git a/refs.h b/refs.h
index 0a68986..5257437 100644
--- a/refs.h
+++ b/refs.h
@@ -382,6 +382,18 @@ typedef int (*for_each_reflog_fn)(each_ref_fn fn, void 
*cb_data);
 typedef int (*reflog_exists_fn)(const char *refname);
 typedef int (*create_reflog_fn)(const char *refname);
 typedef int (*delete_reflog_fn)(const char *refname);
+typedef const char *(*resolve_ref_unsafe_fn)(const char *ref,
+   unsigned char *sha1, int reading, int *flag);
+
+typedef int (*is_refname_available_fn)(const char *refname, const char **skip,
+

[PATCH v3 23/23] refs-be-files.c: add methods for the ref iterators

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 21 ++---
 refs.c  | 36 
 refs.h  | 18 ++
 3 files changed, 68 insertions(+), 7 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 910663b..7c0ab25 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1614,33 +1614,33 @@ static int files_head_ref_submodule(const char 
*submodule, each_ref_fn fn,
return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+static int files_for_each_ref_submodule(const char *submodule, each_ref_fn fn, 
void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), , fn, 0, 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+static int files_for_each_ref_in(const char *prefix, each_ref_fn fn, void 
*cb_data)
 {
return do_for_each_ref(ref_cache, prefix, fn, strlen(prefix), 0, 
cb_data);
 }
 
-int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+static int files_for_each_ref_in_submodule(const char *submodule, const char 
*prefix,
each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(get_ref_cache(submodule), prefix, fn, 
strlen(prefix), 0, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, refs/replace/, fn, 13, 0, cb_data);
 }
 
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+static int files_for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 {
struct strbuf buf = STRBUF_INIT;
int ret;
@@ -1650,7 +1650,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
return ret;
 }
 
-int for_each_rawref(each_ref_fn fn, void *cb_data)
+static int files_for_each_rawref(each_ref_fn fn, void *cb_data)
 {
return do_for_each_ref(ref_cache, , fn, 0,
   DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -3318,6 +3318,13 @@ struct ref_be refs_files = {
files_resolve_gitlink_ref,
files_head_ref,
files_head_ref_submodule,
+   files_for_each_ref,
+   files_for_each_ref_submodule,
+   files_for_each_ref_in,
+   files_for_each_ref_in_submodule,
+   files_for_each_rawref,
+   files_for_each_namespaced_ref,
+   files_for_each_replace_ref,
 };
 
 struct ref_be *refs = refs_files;
diff --git a/refs.c b/refs.c
index 841d905..ceee979 100644
--- a/refs.c
+++ b/refs.c
@@ -931,3 +931,39 @@ int head_ref_submodule(const char *submodule, each_ref_fn 
fn, void *cb_data)
 {
return refs-head_ref_submodule(submodule, fn, cb_data);
 }
+
+int for_each_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref(fn, cb_data);
+}
+
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void 
*cb_data)
+{
+   return refs-for_each_ref_submodule(submodule, fn, cb_data);
+}
+
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref_in(prefix, fn, cb_data);
+}
+
+int for_each_ref_in_submodule(const char *submodule, const char *prefix,
+ each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_ref_in_submodule(submodule, prefix, fn, cb_data);
+}
+
+int for_each_rawref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_rawref(fn, cb_data);
+}
+
+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_namespaced_ref(fn, cb_data);
+}
+
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+{
+   return refs-for_each_replace_ref(fn, cb_data);
+}
diff --git a/refs.h b/refs.h
index 92f8f44..bd3a0d4 100644
--- a/refs.h
+++ b/refs.h
@@ -397,6 +397,17 @@ typedef int (*resolve_gitlink_ref_fn)(const char *path, 
const char *refname,
 typedef int (*head_ref_fn)(each_ref_fn fn, void *cb_data);
 typedef int (*head_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
 void *cb_data);
+typedef int (*for_each_ref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*for_each_ref_submodule_fn)(const char *submodule, each_ref_fn fn,
+void *cb_data);
+typedef int (*for_each_ref_in_fn)(const char *prefix, each_ref_fn fn,
+ void *cb_data);
+typedef int (*for_each_ref_in_submodule_fn)(const char *submodule,
+   const char *prefix,
+   each_ref_fn fn, void *cb_data);
+typedef int (*for_each_rawref_fn)(each_ref_fn fn, void *cb_data);
+typedef int (*for_each_namespaced_ref_fn)(each_ref_fn fn, void 

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

2014-08-19 Thread Ronnie Sahlberg
These functions do not depend on the backend implementation so we
can move them to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 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 40c329b..a94378e 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1514,24 +1514,6 @@ struct ref_filter {
void *cb_data;
 };
 
-int read_ref_full(const char *refname, unsigned char *sha1, int flags, int 
*ref_flag)
-{
-   if (resolve_ref_unsafe(refname, sha1, flags, ref_flag))
-   return 0;
-   return -1;
-}
-
-int read_ref(const char *refname, unsigned char *sha1)
-{
-   return read_ref_full(refname, sha1, RESOLVE_REF_READING, NULL);
-}
-
-int ref_exists(const char *refname)
-{
-   unsigned char sha1[20];
-   return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
-}
-
 static int filter_refs(const char *refname, const unsigned char *sha1, int 
flags,
   void *data)
 {
diff --git a/refs.c b/refs.c
index 3011452..a0e6d81 100644
--- a/refs.c
+++ b/refs.c
@@ -549,3 +549,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, data);
 }
+
+int read_ref_full(const char *refname, unsigned char *sha1, int flags, int 
*ref_flag)
+{
+   if (resolve_ref_unsafe(refname, sha1, flags, ref_flag))
+   return 0;
+   return -1;
+}
+
+int read_ref(const char *refname, unsigned char *sha1)
+{
+   return read_ref_full(refname, sha1, RESOLVE_REF_READING, NULL);
+}
+
+int ref_exists(const char *refname)
+{
+   unsigned char sha1[20];
+   return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 10/23] refs.c: move warn_if_dangling_symref* to the common code

2014-08-19 Thread Ronnie Sahlberg
These functions do not use any backend specific code so we can move
them to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 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 56e146f..40c329b 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1667,58 +1667,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 unsigned char 
*sha1,
-  int flags, void *cb_data)
-{
-   struct warn_if_dangling_data *d = cb_data;
-   const char *resolves_to;
-   unsigned char junk[20];
-
-   if (!(flags  REF_ISSYMREF))
-   return 0;
-
-   resolves_to = resolve_ref_unsafe(refname, junk, 0, 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, data);
-}
-
-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, data);
-}
-
 /*
  * 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 adf0c29..3011452 100644
--- a/refs.c
+++ b/refs.c
@@ -497,3 +497,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 unsigned char 
*sha1,
+  int flags, void *cb_data)
+{
+   struct warn_if_dangling_data *d = cb_data;
+   const char *resolves_to;
+   unsigned char junk[20];
+
+   if (!(flags  REF_ISSYMREF))
+   return 0;
+
+   resolves_to = resolve_ref_unsafe(refname, junk, 0, 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, data);
+}
+
+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, data);
+}
-- 
2.0.1.552.g1af257a

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


[PATCH v3 01/23] refs.c: create a public function for is_refname_available

2014-08-19 Thread Ronnie Sahlberg
Export a generic is_refname_available() function. We will need this
as a public shared function later when we add additional refs backends
since we want to keep using the same rules for ref naming across
all backends.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs.c | 29 ++---
 refs.h |  6 ++
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/refs.c b/refs.c
index 7e13c0f..4a22513 100644
--- a/refs.c
+++ b/refs.c
@@ -830,9 +830,9 @@ static int name_conflict_fn(struct ref_entry *entry, void 
*cb_data)
  * operation). skip contains a list of refs we want to skip checking for
  * conflicts with.
  */
-static int is_refname_available(const char *refname,
-   struct ref_dir *dir,
-   const char **skip, int skipnum)
+static int is_refname_available_dir(const char *refname,
+   struct ref_dir *dir,
+   const char **skip, int skipnum)
 {
struct name_conflict_cb data;
data.refname = refname;
@@ -1238,6 +1238,18 @@ static struct ref_dir *get_loose_refs(struct ref_cache 
*refs)
return get_ref_dir(refs-loose);
 }
 
+int is_refname_available(const char *refname, const char **skip, int skipnum)
+{
+   if (!is_refname_available_dir(refname, get_packed_refs(ref_cache),
+ skip, skipnum))
+   return 0;
+
+   if (!is_refname_available_dir(refname, get_loose_refs(ref_cache),
+ skip, skipnum))
+   return 0;
+   return 1;
+}
+
 /* We allow recursive symbolic refs. Only within reason, though */
 #define MAXDEPTH 5
 #define MAXREFLEN (1024)
@@ -2168,8 +2180,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char 
*refname,
 * name is a proper prefix of our refname.
 */
if (missing 
-!is_refname_available(refname, get_packed_refs(ref_cache),
-  skip, skipnum)) {
+!is_refname_available_dir(refname, get_packed_refs(ref_cache),
+  skip, skipnum)) {
last_errno = ENOTDIR;
goto error_return;
}
@@ -2676,12 +2688,7 @@ int rename_ref(const char *oldrefname, const char 
*newrefname, const char *logms
return 1;
}
 
-   if (!is_refname_available(newrefname, get_packed_refs(ref_cache),
- oldrefname, 1))
-   return 1;
-
-   if (!is_refname_available(newrefname, get_loose_refs(ref_cache),
- oldrefname, 1))
+   if (!is_refname_available(newrefname, oldrefname, 1))
return 1;
 
log = reflog_exists(oldrefname);
diff --git a/refs.h b/refs.h
index f44b5c8..d526da0 100644
--- a/refs.h
+++ b/refs.h
@@ -131,6 +131,12 @@ extern int ref_exists(const char *);
 extern int is_branch(const char *refname);
 
 /*
+ * Check that a particular refname is available for creation. skip contains
+ * a list of refnames to exclude from the refname collision tests.
+ */
+int is_refname_available(const char *refname, const char **skip, int skipnum);
+
+/*
  * If refname is a non-symbolic reference that refers to a tag object,
  * and the tag can be (recursively) dereferenced to a non-tag object,
  * store the SHA1 of the referred-to object to sha1 and return 0.  If
-- 
2.0.1.552.g1af257a

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


[PATCH v3 09/23] refs.c: move dwim and friend functions to the common refs code

2014-08-19 Thread Ronnie Sahlberg
These functions do not contain any backend specific code so we can move
them to the common code and share across all backends.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 202 
 refs.c  | 202 
 2 files changed, 202 insertions(+), 202 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index 6181edf..56e146f 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1956,30 +1956,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 */
@@ -2033,91 +2009,6 @@ static int remove_empty_directories(const char *file)
return result;
 }
 
-/*
- * *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, buf);
-
-   if (ret == *len) {
-   size_t size;
-   *string = strbuf_detach(buf, size);
-   *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(str, len);
-   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, this_result,
-  RESOLVE_REF_READING, flag);
-   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(str, len);
-   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, hash, RESOLVE_REF_READING, 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;
-}
-
 /* This function should make sure errno is meaningful on error */
 static struct ref_lock *lock_ref_sha1_basic(const char *refname,
const unsigned char *old_sha1,
@@ -3703,96 +3594,3 @@ cleanup:
ret = -2;
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[].
-* Generate a format suitable for scanf from a
-  

[PATCH v3 16/23] refs.c: move prettify_refname to the common code

2014-08-19 Thread Ronnie Sahlberg
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 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 6542969..fb9c614 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1747,15 +1747,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 177bed6..d163e61 100644
--- a/refs.c
+++ b/refs.c
@@ -696,3 +696,12 @@ int names_conflict(const char *refname1, const char 
*refname2)
return (*refname1 == '\0'  *refname2 == '/')
|| (*refname1 == '/'  *refname2 == '\0');
 }
+
+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.0.1.552.g1af257a

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


[PATCH v3 12/23] refs.c: move resolve_refdup to common

2014-08-19 Thread Ronnie Sahlberg
This function can be shared across all refs backends so move it
to the common code.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
---
 refs-be-files.c | 6 --
 refs.c  | 6 ++
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/refs-be-files.c b/refs-be-files.c
index a94378e..ed7bc61 100644
--- a/refs-be-files.c
+++ b/refs-be-files.c
@@ -1501,12 +1501,6 @@ const char *resolve_ref_unsafe(const char *refname, 
unsigned char *sha1, int fla
}
 }
 
-char *resolve_refdup(const char *ref, unsigned char *sha1, int flags, int 
*ref_flag)
-{
-   const char *ret = resolve_ref_unsafe(ref, sha1, flags, ref_flag);
-   return ret ? xstrdup(ret) : NULL;
-}
-
 /* The argument to filter_refs */
 struct ref_filter {
const char *pattern;
diff --git a/refs.c b/refs.c
index a0e6d81..b8582f8 100644
--- a/refs.c
+++ b/refs.c
@@ -567,3 +567,9 @@ int ref_exists(const char *refname)
unsigned char sha1[20];
return !!resolve_ref_unsafe(refname, sha1, RESOLVE_REF_READING, NULL);
 }
+
+char *resolve_refdup(const char *ref, unsigned char *sha1, int flags, int 
*ref_flag)
+{
+   const char *ret = resolve_ref_unsafe(ref, sha1, flags, ref_flag);
+   return ret ? xstrdup(ret) : NULL;
+}
-- 
2.0.1.552.g1af257a

--
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: Relative submodule URLs

2014-08-19 Thread Junio C Hamano
Robert Dailey rcdailey.li...@gmail.com writes:

 The way I set up my remote tracking branch will be different for each
 of these commands:

 - git pull :: If I want convenient pulls (with rebase), I will track
 my upstream branch. My pushes have to be more explicit as a tradeoff.

Keeping 'origin' pointing at the repository where you cloned from,
without doing anything funky (i.e. set up my remote) would give
you convenient pulls.

 - git push :: If I want convenient pushes, track my origin branch.
 Pulls become less convenient. My relative submodules will now need to
 be forked.

You need to configure your pushes to go to a different place, if you
want them to go to a different place ;-).

Long time ago, it used to be that you have to affect the URL used in
both direction, making pulls less conveninent, but hasn't this been
made an non-issue for triangular workflows with the introduction of
remote.pushdefault long time ago?

 - git submodule update :: I track upstream to avoid forking my
 submodules. But pushes become more inconvenient.

If 'submodule update' follows the same place as 'pull' goes by
default, I would imagine that there is no issue here, no?  Am I
oversimplifying the issue by guessing that the root cause of is that
you are not using remote.pushdefault from your configuration
toolchest and instead setting the 'origin' to a wrong (i.e. where
push goes) place?
--
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: Relative submodule URLs

2014-08-19 Thread Robert Dailey
On Tue, Aug 19, 2014 at 11:39 AM, Junio C Hamano gits...@pobox.com wrote:
 Robert Dailey rcdailey.li...@gmail.com writes:

 The way I set up my remote tracking branch will be different for each
 of these commands:

 - git pull :: If I want convenient pulls (with rebase), I will track
 my upstream branch. My pushes have to be more explicit as a tradeoff.

 Keeping 'origin' pointing at the repository where you cloned from,
 without doing anything funky (i.e. set up my remote) would give
 you convenient pulls.

 - git push :: If I want convenient pushes, track my origin branch.
 Pulls become less convenient. My relative submodules will now need to
 be forked.

 You need to configure your pushes to go to a different place, if you
 want them to go to a different place ;-).

 Long time ago, it used to be that you have to affect the URL used in
 both direction, making pulls less conveninent, but hasn't this been
 made an non-issue for triangular workflows with the introduction of
 remote.pushdefault long time ago?

 - git submodule update :: I track upstream to avoid forking my
 submodules. But pushes become more inconvenient.

 If 'submodule update' follows the same place as 'pull' goes by
 default, I would imagine that there is no issue here, no?  Am I
 oversimplifying the issue by guessing that the root cause of is that
 you are not using remote.pushdefault from your configuration
 toolchest and instead setting the 'origin' to a wrong (i.e. where
 push goes) place?


Maybe I'm misunderstanding something here and you can help me out.

All the reading I've done (mostly github) says that 'upstream' points
to the authoritative repository that you forked from but do not have
permissions to write to. 'origin' points to the place you push your
changes for pull requests (the fork).

Basically the workflow I use is:

- Use 'upstream' to PULL changes (latest code is obtained from the
authoritative repository)
- Use 'origin' to push your branches. Since I never modify the
branches that exist in 'upstream' on my 'origin' (I do everything
through separate personal branches).

That means if I have a branch off of 'master' named 'topic', I will
track 'upstream/master' and get latest with 'git pull'. When I'm ready
for a pull request, I do 'git push origin' (I use push.default =
simple).

According to my understanding, relative submodules work here. But not
everyone on my team uses this workflow. Sometimes they track
origin/topic (if we use my example again). Won't the submodule try
to find itself on the fork now?

Basically it seems like what you're advocating is that I need to
enforce a policy of always track upstream and never track origin
and always set remote.pushdefault. Seems a bit error prone...
--
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: Issuing warning when hook does not have execution permission

2014-08-19 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Tue, Aug 19, 2014 at 04:05:21PM +1000, Babak M wrote:

 I saw that if a hook file is present in .git/hooks and it does not
 have execution permissions it is silently ignored.
 
 I thought it might be worthwhile issuing a warning such as Warning:
 pre-commit hook exists but it cannot be executed due to insufficient
 permissions.
 
 Not sure if this has been discussed before. I searched the archive but
 didn't see anything.
 
 Thoughts, suggestions? Is there anything like that already?

 Once upon a time we shipped sample hooks with their execute bits turned
 off, and such a warning would have been very bad.

 These days we give them a .sample extension (because Windows installs
 had trouble with the execute bit :) ), so I think it should be OK in
 theory. Installing a new version of git on top of an old one with make
 install does not clean up old files. So somebody who has continuously
 upgraded their git via make install to the same directory would have
 the old-style sample files. Under your proposal, they would get a lot of
 warnings.

 However, that change came in v1.6.0, just over 6 years ago. We can
 probably discount that (and if it does happen, maybe it is time for that
 someone to clean up the other leftover cruft from past git installs).

The above all sounds very sensible.

We have another code path that looks for an executable, finds one
with no execute permission and decides not to execute it, and I
wonder if we should use the same criteria to decide to give (or not
give) a warning as the one used in the other code path (i.e. looking
up git-foo executable when the user runs git foo).
--
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] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Junio C Hamano
Robin Rosenberg robin.rosenb...@dewire.com writes:

 Using the original filename suffix for the temporary input files to
 the merge tool confuses IDEs like Eclipse. This patch introduces
 a configurtion option, mergetool.tmpsuffix, which get appended to
 the temporary file name. That way the user can choose to use a
 suffix like .tmp, which does not cause confusion.

 Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
 ---
  Documentation/git-mergetool.txt |  7 +++
  git-mergetool.sh| 10 ++
  2 files changed, 13 insertions(+), 4 deletions(-)

No updates to Documentation/config.txt to describe the new variable?


 diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
 index e846c2e..80a0526 100644
 --- a/Documentation/git-mergetool.txt
 +++ b/Documentation/git-mergetool.txt
 @@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable 
 to `false`
  causes `git mergetool` to automatically remove the backup as files
  are successfully merged.
  
 +`git mergetool` may also create other temporary files for the
 +different versions involved in the merge. By default these files have
 +the same filename suffix as the file being merged. This may confuse
 +other tools in use during a long merge operation. The user can set
 +`mergetool.tmpsuffix` to be used as an extra suffix, which will be
 +appened to the temporary filename to lessen that problem.
 +
  GIT
  ---
  Part of the linkgit:git[1] suite
 diff --git a/git-mergetool.sh b/git-mergetool.sh
 index 9a046b7..d7cc76c 100755
 --- a/git-mergetool.sh
 +++ b/git-mergetool.sh
 @@ -214,6 +214,8 @@ checkout_staged_file () {
  }
  
  merge_file () {
 + tmpsuffix=$(git config mergetool.tmpsuffix || true)
 +
   MERGED=$1
  
   f=$(git ls-files -u -- $MERGED)
 @@ -229,10 +231,10 @@ merge_file () {
   fi
  
   ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$')
 - BACKUP=./$MERGED.BACKUP.$ext
 - LOCAL=./$MERGED.LOCAL.$ext
 - REMOTE=./$MERGED.REMOTE.$ext
 - BASE=./$MERGED.BASE.$ext
 + BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
 + LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
 + REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
 + BASE=./$MERGED.BASE.$ext$tmpsuffix
  
   base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}')
   local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print 
 $1;}')
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-imap-send: use libcurl for implementation

2014-08-19 Thread Junio C Hamano
Bernhard Reiter ock...@raz.or.at writes:

 Am 2014-08-17 um 20:42 schrieb Jeff King:
 [...]
 
 I'm not sure I understand this comment. Even if SSL is not in use,
 wouldn't we be passing a regular pipe to curl, which would break?

 Yeah, we can't do that, and thus would have to keep the handwritten IMAP
 implementation just for the tunnel case (allowing to drop only the
 OpenSSL specific stuff), see my other email:
 http://www.mail-archive.com/git@vger.kernel.org/msg56791.html (the
 relevant part is pretty far down at the bottom).
 
 I'd really love it if we could make this work with tunnels and
 eventually get rid of the hand-written imap code entirely. I agree with
 Jonathan that we probably need to keep it around a bit for people on
 older curl, but dropping it is a good goal in the long run. That code
 was forked from the isync project, but mangled enough that we could not
 take bug fixes from upstream. As not many people use imap-send, I
 suspect it is largely unmaintained and the source of many lurking
 bugs[1]. Replacing it with curl's maintained implementation is probably
 a good step.

I would agree with s/a good step/a good goal/ ;-)

 I'll work on this as soon as I find some time, but as that will include
 changes to run-command.c (and possibly other files?), I'd like to cover
 that in a commit of its own. Do you guys think the current patch [1] is
 good enough for official submission already?

My impression from reading the discussion in this thread has been
that the patch that started this thread would break the tunneling
code, i.e. is not there yet.  Or did you mean some other patch?

The other patch $gmane/255403 from you looked good and I think I
already have a copy queued on 'pu' as f9dc5d65 (git-imap-send:
simplify tunnel construction, 2014-08-13).

Thanks.


[References]

*$gmane/255403*
http://thread.gmane.org/gmane.comp.version-control.git/255220/focus=255403
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v3] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Robin Rosenberg
Using the original filename suffix for the temporary input files to
the merge tool confuses IDEs like Eclipse. This patch introduces
a configurtion option, mergetool.tmpsuffix, which get appended to
the temporary file name. That way the user can choose to use a
suffix like .tmp, which does not cause confusion.

Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
---
 Documentation/config.txt|  5 +
 Documentation/git-mergetool.txt |  7 +++
 git-mergetool.sh| 10 ++
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c55c22a..0e15800 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1778,6 +1778,11 @@ notes.displayRef::
several times.  A warning will be issued for refs that do not
exist, but a glob that does not match any refs is silently
ignored.
+
+mergetool.tmpsuffix::
+   A string to append the names of the temporary files mergetool
+   creates in the worktree as input to a custom merge tool. The
+   primary use is to avoid confusion in IDEs during merge.
 +
 This setting can be overridden with the `GIT_NOTES_DISPLAY_REF`
 environment variable, which must be a colon separated list of refs or
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index e846c2e..80a0526 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable to 
`false`
 causes `git mergetool` to automatically remove the backup as files
 are successfully merged.
 
+`git mergetool` may also create other temporary files for the
+different versions involved in the merge. By default these files have
+the same filename suffix as the file being merged. This may confuse
+other tools in use during a long merge operation. The user can set
+`mergetool.tmpsuffix` to be used as an extra suffix, which will be
+appened to the temporary filename to lessen that problem.
+
 GIT
 ---
 Part of the linkgit:git[1] suite
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 9a046b7..d7cc76c 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -214,6 +214,8 @@ checkout_staged_file () {
 }
 
 merge_file () {
+   tmpsuffix=$(git config mergetool.tmpsuffix || true)
+
MERGED=$1
 
f=$(git ls-files -u -- $MERGED)
@@ -229,10 +231,10 @@ merge_file () {
fi
 
ext=$$$(expr $MERGED : '.*\(\.[^/]*\)$')
-   BACKUP=./$MERGED.BACKUP.$ext
-   LOCAL=./$MERGED.LOCAL.$ext
-   REMOTE=./$MERGED.REMOTE.$ext
-   BASE=./$MERGED.BASE.$ext
+   BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
+   LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
+   REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
+   BASE=./$MERGED.BASE.$ext$tmpsuffix
 
base_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==1) print $1;}')
local_mode=$(git ls-files -u -- $MERGED | awk '{if ($3==2) print 
$1;}')
-- 
2.1.0.rc2.6.g39c33ff.dirty

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


Re: [PATCH/RFC] git-imap-send: use libcurl for implementation

2014-08-19 Thread Junio C Hamano
Bernhard Reiter ock...@raz.or.at writes:

 Use libcurl's high-level API functions to implement git-imap-send
 instead of the previous low-level OpenSSL-based functions.

 Since version 7.30.0, libcurl's API has been able to communicate with
 IMAP servers. Using those high-level functions instead of the current
 ones would reduce imap-send.c by some 1200 lines of code. For now,
 the old ones are wrapped in #ifdefs, and the new functions are enabled
 by make if curl's version is = 7.35.0, from which version on curl's
 CURLOPT_LOGIN_OPTIONS (enabling IMAP authentication) parameter has been
 available.

 As I don't have access to that many IMAP servers, I haven't been able to
 test the new code with a wide variety of parameter combinations. I did
 test both secure and insecure (imaps:// and imap://) connections and
 values of PLAIN and LOGIN for the authMethod.

 Signed-off-by: Bernhard Reiter ock...@raz.or.at
 ---

I think people have missed this one, partly because it was hidden as
an attachment.

  Documentation/git-imap-send.txt |   3 +-
  INSTALL |  15 +++---
  Makefile|  16 +-
  git.spec.in |   5 +-
  imap-send.c | 109
 +---
  5 files changed, 132 insertions(+), 16 deletions(-)

I smell a line-wrapped patch but it probably is at my receiving end,
forcing the attachment into a flattened form inside my MUA.

Nice to see git.spec.in updated; I like it when I see that the
author paid attention to details.

 diff --git a/imap-send.c b/imap-send.c
 index fb01a9c..a45570d 100644
 --- a/imap-send.c
 +++ b/imap-send.c
 @@ -22,6 +22,10 @@
   *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
   */
  
 +#ifdef USE_CURL_FOR_IMAP_SEND
 +#define NO_OPENSSL
 +#endif
 +

This looks strange and stands out like a sore thumb.  Do any of our
other sources do this kind of macro tweaking inside C source before
including git-compat-util.h (or its equivalent like cache.h)?

I think what you are trying to do is to change the meaning of
NO_OPENSSL, which merely means we do not have OpenSSL library and
do not want to link with it, locally to we may or may not have and
use OpenSSL library elsewhere in Git, but in the code below we do
not want to use the code written to work on top of OpenSSL and
instead use libcurl.  Because of that, you define NO_OPENSSL before
including any of our headers to cause us not to include the headers,
and typedef away SSL, for example.

  #include cache.h
  #include credential.h
  #include exec_cmd.h
 @@ -29,6 +33,9 @@
  #ifdef NO_OPENSSL
  typedef void *SSL;
  #endif
 +#ifdef USE_CURL_FOR_IMAP_SEND
 +#include http.h
 +#endif

But does it have to be that way?  For one thing, doing it this way,
the user has to make a compilation-time choice, but if you separate
these compilation time macro into two, one for can we even link
with and use OpenSSL? (which is what NO_OPENSSL is about) and
another for do we want an ability to talk to imap via libcurl?,
wouldn't it make it possible for you to switch between them at
runtime (e.g. you might want to go over the direct connection when
tunneling, while letting libcurl do the heavy lifting in
non-tunneled operation)?

 @@ -44,9 +51,7 @@ __attribute__((format (printf, 1, 2)))
  static void imap_info(const char *, ...);
  __attribute__((format (printf, 1, 2)))
  static void imap_warn(const char *, ...);
 -
  static char *next_arg(char **);
 -
  __attribute__((format (printf, 3, 4)))
  static int nfsnprintf(char *buf, int blen, const char *fmt, ...);
  
 @@ -69,6 +74,7 @@ struct imap_server_conf {
   char *tunnel;
   char *host;
   int port;
 + char *folder;
   char *user;
   char *pass;
   int use_ssl;
 @@ -82,6 +88,7 @@ static struct imap_server_conf server = {
   NULL,   /* tunnel */
   NULL,   /* host */
   0,  /* port */
 + NULL,   /* folder */
   NULL,   /* user */
   NULL,   /* pass */
   0,  /* use_ssl */
 @@ -1323,7 +1330,54 @@ static int split_msg(struct strbuf *all_msgs, struct 
 strbuf *msg, int *ofs)
   return 1;
  }
  
 -static char *imap_folder;

All of the above seem to have meant well, but these changes are not
about talking with IMAP servers via libcurl.  We'd prefer to see
changes like these as preliminary clean-up before the main change as
separate patch(es).

 @@ -1417,18 +1476,48 @@ int main(int argc, char **argv)
   }
  
   /* write it to the imap server */
 +
 +#ifdef USE_CURL_FOR_IMAP_SEND
 + if (!server.tunnel) {
 + curl = setup_curl(server);
 + curl_easy_setopt(curl, CURLOPT_READDATA, msgbuf);
 + } else {
 +#endif
   ctx = imap_open_store(server);
   if (!ctx) {
   fprintf(stderr, failed to open store\n);
   return 1;
   }
 + ctx-name = server.folder;
 +#ifdef USE_CURL_FOR_IMAP_SEND
 + }
 +#endif
  
   fprintf(stderr, sending %d 

Re: [PATCH v3] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Junio C Hamano
Robin Rosenberg robin.rosenb...@dewire.com writes:

 Using the original filename suffix for the temporary input files to
 the merge tool confuses IDEs like Eclipse. This patch introduces
 a configurtion option, mergetool.tmpsuffix, which get appended to
 the temporary file name. That way the user can choose to use a
 suffix like .tmp, which does not cause confusion.

 Signed-off-by: Robin Rosenberg robin.rosenb...@dewire.com
 ---
  Documentation/config.txt|  5 +
  Documentation/git-mergetool.txt |  7 +++
  git-mergetool.sh| 10 ++
  3 files changed, 18 insertions(+), 4 deletions(-)

Thanks for a quick turn-around.


 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index c55c22a..0e15800 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1778,6 +1778,11 @@ notes.displayRef::
   several times.  A warning will be issued for refs that do not
   exist, but a glob that does not match any refs is silently
   ignored.
 +
 +mergetool.tmpsuffix::
 + A string to append the names of the temporary files mergetool
 + creates in the worktree as input to a custom merge tool. The
 + primary use is to avoid confusion in IDEs during merge.
  +
  This setting can be overridden with the `GIT_NOTES_DISPLAY_REF`
  environment variable, which must be a colon separated list of refs or

I smell that the new paragraph is inserted at a wrong place.  What
does notes-display-ref environment have anything to do with this
variable?

Also, could you phrase this in a way to hint that the users are
likely to want to begin the value for this variable with a dot (or
some other special character)?  Your 'suffix like .tmp' in the
proposed log message does it very nicely [*1*], and I'd like to see the
same done for the end users who do not have access to our log
message but do have access to our documentation pages.

Same comment applies to the new paragraph in the documentation of
git-mergetool itself.

 diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
 index e846c2e..80a0526 100644
 --- a/Documentation/git-mergetool.txt
 +++ b/Documentation/git-mergetool.txt
 @@ -89,6 +89,13 @@ Setting the `mergetool.keepBackup` configuration variable 
 to `false`
  causes `git mergetool` to automatically remove the backup as files
  are successfully merged.
  
 +`git mergetool` may also create other temporary files for the
 +different versions involved in the merge. By default these files have
 +the same filename suffix as the file being merged. This may confuse
 +other tools in use during a long merge operation. The user can set
 +`mergetool.tmpsuffix` to be used as an extra suffix, which will be
 +appened to the temporary filename to lessen that problem.
 +

[Footnote]

*1* To anybody remotely intelligent (like me), it hints that a
temporary file would have a name like hello.rbtmp if it is set to
tmp, to let them make a natural inference that they are better off
using something like .tmp, ~tmp, +tmp, etc.
--
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] Push the NATIVE_CRLF Makefile variable to C and added a test for native.

2014-08-19 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 Commit 95f31e9a correctly points out that the NATIVE_CRLF setting is
 incorrectly set on Mingw git. However, the Makefile variable is not
 propagated to the C preprocessor and results in no change. This patch
 pushes the definition to the C code and adds a test to validate that
 when core.eol as native is crlf, we actually normalize text files to this
 line ending convention when core.autocrlf is false.

 Signed-off-by: Pat Thoyts pattho...@users.sourceforge.net
 Signed-off-by: Stepan Kasal ka...@ucw.cz
 Signed-off-by: Torsten Bögershausen tbo...@web.de
 ---

 (This is from MINGW, and some part of my brain thougth that this was send
 upstream, but it wasn't. Only 95f31e9a is in git.git)

  Makefile  |  3 +++
  t/t0026-eol-config.sh | 18 ++
  2 files changed, 21 insertions(+)

 diff --git a/Makefile b/Makefile
 index 63a210d..13311d2 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1481,6 +1481,9 @@ ifdef NO_REGEX
   COMPAT_CFLAGS += -Icompat/regex
   COMPAT_OBJS += compat/regex/regex.o
  endif
 +ifdef NATIVE_CRLF
 + BASIC_CFLAGS += -DNATIVE_CRLF
 +endif
  
  ifdef USE_NED_ALLOCATOR
 COMPAT_CFLAGS += -Icompat/nedmalloc
 diff --git a/t/t0026-eol-config.sh b/t/t0026-eol-config.sh
 index 4807b0f..43a580a 100755
 --- a/t/t0026-eol-config.sh
 +++ b/t/t0026-eol-config.sh
 @@ -80,4 +80,22 @@ test_expect_success 'autocrlf=true overrides unset eol' '
   test -z $onediff  test -z $twodiff
  '
  
 +test_expect_success NATIVE_CRLF 'eol native is crlf' '

Who defines this test prerequisite?

 +
 + rm -rf native_eol  mkdir native_eol 
 + ( cd native_eol 
 + printf *.txt text\n  .gitattributes
 + printf one\r\ntwo\r\nthree\r\n  filedos.txt
 + printf one\ntwo\nthree\n  fileunix.txt

Style and nits:

 - No SP between a redirection operator and its target.
 - Broken  chain.
 - Not indented.

i.e.

rm -rf native_eol 
mkdir native_eol 
(
cd native_eol 
printf ... .gitattributes 
...
has_cr filedos.txt 
has_cr fileunix.txt
)

 + git init 
 + git config core.autocrlf false 
 + git config core.eol native 
 + git add filedos.txt fileunix.txt 
 + git commit -m first 
 + rm file*.txt 
 + git reset --hard HEAD 
 + has_cr filedos.txt  has_cr fileunix.txt
 + )
 +'
 +
  test_done
--
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 v2 1/4] run-command: introduce CHILD_PROCESS_INIT

2014-08-19 Thread René Scharfe
Most struct child_process variables are cleared using memset first after
declaration.  Provide a macro, CHILD_PROCESS_INIT, that can be used to
initialize them statically instead.  That's shorter, doesn't require a
function call and is slightly more readable (especially given that we
already have STRBUF_INIT, ARGV_ARRAY_INIT etc.).

Helped-by: Johannes Sixt j...@kdbg.org
Signed-off-by: Rene Scharfe l@web.de
---
Now with ARGV_ARRAY_INIT and more conversions.

 Documentation/technical/api-run-command.txt |  4 ++--
 archive-tar.c   |  3 +--
 builtin/add.c   |  3 +--
 builtin/commit.c|  3 +--
 builtin/help.c  |  3 +--
 builtin/merge.c |  3 +--
 builtin/notes.c |  3 +--
 builtin/receive-pack.c  | 12 
 builtin/remote-ext.c|  3 +--
 builtin/repack.c|  3 +--
 builtin/replace.c   |  4 ++--
 builtin/verify-pack.c   |  3 +--
 bundle.c|  6 ++
 column.c|  2 +-
 connect.c   |  2 +-
 connected.c |  3 +--
 convert.c   |  3 +--
 credential-cache.c  |  3 +--
 credential.c|  3 +--
 daemon.c|  8 +++-
 diff.c  |  3 +--
 editor.c|  3 +--
 fetch-pack.c|  3 +--
 gpg-interface.c |  6 ++
 http-backend.c  |  3 +--
 http.c  |  3 +--
 imap-send.c |  2 +-
 pager.c |  2 +-
 prompt.c|  3 +--
 remote-curl.c   |  3 +--
 remote-testsvn.c|  3 +--
 run-command.c   |  3 +--
 run-command.h   |  2 ++
 send-pack.c |  3 +--
 submodule.c | 21 +++--
 test-run-command.c  |  4 +---
 test-subprocess.c   |  3 +--
 transport.c | 12 
 upload-pack.c   |  5 ++---
 wt-status.c |  3 +--
 40 files changed, 60 insertions(+), 107 deletions(-)

diff --git a/Documentation/technical/api-run-command.txt 
b/Documentation/technical/api-run-command.txt
index 69510ae..ca066bf 100644
--- a/Documentation/technical/api-run-command.txt
+++ b/Documentation/technical/api-run-command.txt
@@ -96,8 +96,8 @@ command to run in a sub-process.
 
 The caller:
 
-1. allocates and clears (memset(chld, 0, sizeof(chld));) a
-   struct child_process variable;
+1. allocates and clears (memset(chld, 0, sizeof(chld)); or
+   using CHILD_PROCESS_INIT) a struct child_process variable;
 2. initializes the members;
 3. calls start_command();
 4. processes the data;
diff --git a/archive-tar.c b/archive-tar.c
index 719b629..0d1e6bd 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -395,7 +395,7 @@ static int write_tar_filter_archive(const struct archiver 
*ar,
struct archiver_args *args)
 {
struct strbuf cmd = STRBUF_INIT;
-   struct child_process filter;
+   struct child_process filter = CHILD_PROCESS_INIT;
const char *argv[2];
int r;
 
@@ -406,7 +406,6 @@ static int write_tar_filter_archive(const struct archiver 
*ar,
if (args-compression_level = 0)
strbuf_addf(cmd,  -%d, args-compression_level);
 
-   memset(filter, 0, sizeof(filter));
argv[0] = cmd.buf;
argv[1] = NULL;
filter.argv = argv;
diff --git a/builtin/add.c b/builtin/add.c
index 4baf3a5..352b85e 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -180,7 +180,7 @@ static int edit_patch(int argc, const char **argv, const 
char *prefix)
char *file = git_pathdup(ADD_EDIT.patch);
const char *apply_argv[] = { apply, --recount, --cached,
NULL, NULL };
-   struct child_process child;
+   struct child_process child = CHILD_PROCESS_INIT;
struct rev_info rev;
int out;
struct stat st;
@@ -214,7 +214,6 @@ static int edit_patch(int argc, const char **argv, const 
char *prefix)
if (!st.st_size)
die(_(Empty patch. Aborted.));
 
-   memset(child, 0, sizeof(child));
child.git_cmd = 1;
child.argv = apply_argv;
if (run_command(child))
diff --git a/builtin/commit.c b/builtin/commit.c
index 5ed6036..b8b8663 100644
--- 

[PATCH v2 3/4] run-command: call run_command_v_opt_cd_env() instead of duplicating it

2014-08-19 Thread René Scharfe
Signed-off-by: Rene Scharfe l@web.de
---
 run-command.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/run-command.c b/run-command.c
index 47ab21b..9196ee0 100644
--- a/run-command.c
+++ b/run-command.c
@@ -577,9 +577,7 @@ static void prepare_run_command_v_opt(struct child_process 
*cmd,
 
 int run_command_v_opt(const char **argv, int opt)
 {
-   struct child_process cmd;
-   prepare_run_command_v_opt(cmd, argv, opt);
-   return run_command(cmd);
+   return run_command_v_opt_cd_env(argv, opt, NULL, NULL);
 }
 
 int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, 
const char *const *env)
-- 
2.1.0

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


[PATCH v2 4/4] run-command: inline prepare_run_command_v_opt()

2014-08-19 Thread René Scharfe
Merge prepare_run_command_v_opt() and its only caller.  This removes a
pointer indirection and allows to initialize the struct child_process
using CHILD_PROCESS_INIT.

Signed-off-by: Rene Scharfe l@web.de
---
 run-command.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/run-command.c b/run-command.c
index 9196ee0..761f0fd 100644
--- a/run-command.c
+++ b/run-command.c
@@ -561,20 +561,6 @@ int run_command(struct child_process *cmd)
return finish_command(cmd);
 }
 
-static void prepare_run_command_v_opt(struct child_process *cmd,
- const char **argv,
- int opt)
-{
-   memset(cmd, 0, sizeof(*cmd));
-   cmd-argv = argv;
-   cmd-no_stdin = opt  RUN_COMMAND_NO_STDIN ? 1 : 0;
-   cmd-git_cmd = opt  RUN_GIT_CMD ? 1 : 0;
-   cmd-stdout_to_stderr = opt  RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
-   cmd-silent_exec_failure = opt  RUN_SILENT_EXEC_FAILURE ? 1 : 0;
-   cmd-use_shell = opt  RUN_USING_SHELL ? 1 : 0;
-   cmd-clean_on_exit = opt  RUN_CLEAN_ON_EXIT ? 1 : 0;
-}
-
 int run_command_v_opt(const char **argv, int opt)
 {
return run_command_v_opt_cd_env(argv, opt, NULL, NULL);
@@ -582,8 +568,14 @@ int run_command_v_opt(const char **argv, int opt)
 
 int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, 
const char *const *env)
 {
-   struct child_process cmd;
-   prepare_run_command_v_opt(cmd, argv, opt);
+   struct child_process cmd = CHILD_PROCESS_INIT;
+   cmd.argv = argv;
+   cmd.no_stdin = opt  RUN_COMMAND_NO_STDIN ? 1 : 0;
+   cmd.git_cmd = opt  RUN_GIT_CMD ? 1 : 0;
+   cmd.stdout_to_stderr = opt  RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
+   cmd.silent_exec_failure = opt  RUN_SILENT_EXEC_FAILURE ? 1 : 0;
+   cmd.use_shell = opt  RUN_USING_SHELL ? 1 : 0;
+   cmd.clean_on_exit = opt  RUN_CLEAN_ON_EXIT ? 1 : 0;
cmd.dir = dir;
cmd.env = env;
return run_command(cmd);
-- 
2.1.0

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


Re: Relative submodule URLs

2014-08-19 Thread Junio C Hamano
Robert Dailey rcdailey.li...@gmail.com writes:

 Maybe I'm misunderstanding something here and you can help me out.

 All the reading I've done (mostly github) says that 'upstream' points
 to the authoritative repository that you forked from but do not have
 permissions to write to. 'origin' points to the place you push your
 changes for pull requests (the fork).

I do not know if that is how GitHub teaches people, but I would have
to say that these are strange phrasing.  I suspect that part of
their documentation was written long time ago, back when nobody on
the GitHub side were involved in design (let alone implementation)
of Git, and I would take it with a grain of salt.

Having said that, here is a summary of the current support for
referring to different repositories in Git:

   The word 'origin' refers to where things originate from; a place
   you push to is where things go, so it makes no sense to use that
   word to refer to the repository where you publish your work
   result.  The 'origin' may or may not be where you can push (or
   you would want to push) to.  It is where you 'pull' from to
   synchronize with the 'upstream'.

   The 'upstream' in SCM context refers to those who control a
   logically more authoritative history than you, whose work you
   derive your work from, i.e. synonymous to 'origin'.

   For people like Linus (i.e. he may pull from others but that is
   to take in changes made as derived work; he does not pull to grab
   more authoritative work), there is no need to say 'upstream'; or
   you can consider he is his own 'upstream'.

   For those who use CVS-style central repository model (i.e. they
   would pull from that single central shared repository, and push
   their work back to the same repository), 'origin' are writable to
   them and they push to them.  For people with CVS-style central
   shared repository model, their central repository is their
   'upstream' with respect to their local copy.

   Since these two classes of people need just one other repository
   to refer to, we just used 'origin' when we did the very initial
   version of git clone, and these users can keep using that name
   to refer to that single other repository they interact with.

   The support for the triangular workflow in which you pull from
   one place and push the result of work to another, which the
   configuration variable 'remote.pushdefault' is a part of, is
   relatively a more recent development in Git.  I am not sure we
   have added an official term to our glossary to refer to the
   repository you push your work result to, but in the discussions
   we have seen phrases like 'publishing repository' used, I think.
   It must be writable by you, of course, and it may or may not be
   the same as the 'origin' repository.

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


Re: Re: Relative submodule URLs

2014-08-19 Thread Heiko Voigt
On Tue, Aug 19, 2014 at 11:50:08AM -0500, Robert Dailey wrote:
 Maybe I'm misunderstanding something here and you can help me out.
 
 All the reading I've done (mostly github) says that 'upstream' points
 to the authoritative repository that you forked from but do not have
 permissions to write to. 'origin' points to the place you push your
 changes for pull requests (the fork).
 
 Basically the workflow I use is:
 
 - Use 'upstream' to PULL changes (latest code is obtained from the
 authoritative repository)
 - Use 'origin' to push your branches. Since I never modify the
 branches that exist in 'upstream' on my 'origin' (I do everything
 through separate personal branches).
 
 That means if I have a branch off of 'master' named 'topic', I will
 track 'upstream/master' and get latest with 'git pull'. When I'm ready
 for a pull request, I do 'git push origin' (I use push.default =
 simple).
 
 According to my understanding, relative submodules work here. But not
 everyone on my team uses this workflow. Sometimes they track
 origin/topic (if we use my example again). Won't the submodule try
 to find itself on the fork now?

Well the remote for the submodule is currently only calculated once,
when you do the initial

git submodule update --init

that clones the submodule. Afterwards the fixed url is configured under
the name 'origin' in the submodule like in a normal git repository that
you have freshly cloned. Which remote is used for cloning depends on the
configured remote for the current branch or 'origin'.

When you do a fetch or push with --recurse-submodules it only executes a
'git fetch' or 'git push' without any specific remote. For fetch the
same commandline options (but only the options) are passed on.

Here it might make sense to guess the remote in the submodule somehow
and not do what fetch without remotes would do.

For the triangular workflow not much work has been done in regards to
submodule support.

But since a submodule behaves like a normal git repository maybe there
is not much work needed and we can just point to the workflow without
submodules most times. We still have to figure that out properly.

Cheers Heiko
--
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: Transaction patch series overview

2014-08-19 Thread Ronnie Sahlberg
List, please see here an overview and ordering of the ref transaction
patch series.
These series build on each other and needs to be applied in the order
listed below.

This is an update.




rs/ref-transaction-0
---
Early part of the ref transaction topic.

* rs/ref-transaction-0:
  refs.c: change ref_transaction_update() to do error checking and
return status
  refs.c: remove the onerr argument to ref_transaction_commit
  update-ref: use err argument to get error from ref_transaction_commit
  refs.c: make update_ref_write update a strbuf on failure
  refs.c: make ref_update_reject_duplicates take a strbuf argument
for errors
  refs.c: log_ref_write should try to return meaningful errno
  refs.c: make resolve_ref_unsafe set errno to something meaningful on error
  refs.c: commit_packed_refs to return a meaningful errno on failure
  refs.c: make remove_empty_directories always set errno to something sane
  refs.c: verify_lock should set errno to something meaningful
  refs.c: make sure log_ref_setup returns a meaningful errno
  refs.c: add an err argument to repack_without_refs
  lockfile.c: make lock_file return a meaningful errno on failurei
  lockfile.c: add a new public function unable_to_lock_message
  refs.c: add a strbuf argument to ref_transaction_commit for error logging
  refs.c: allow passing NULL to ref_transaction_free
  refs.c: constify the sha arguments for
ref_transaction_create|delete|update
  refs.c: ref_transaction_commit should not free the transaction
  refs.c: remove ref_transaction_rollback

Has been merged into next.



ref-transaction-1 (2014-07-16) 20 commits
-
Second batch of ref transactions

 - refs.c: make delete_ref use a transaction
 - refs.c: make prune_ref use a transaction to delete the ref
 - refs.c: remove lock_ref_sha1
 - refs.c: remove the update_ref_write function
 - refs.c: remove the update_ref_lock function
 - refs.c: make lock_ref_sha1 static
 - walker.c: use ref transaction for ref updates
 - fast-import.c: use a ref transaction when dumping tags
 - receive-pack.c: use a reference transaction for updating the refs
 - refs.c: change update_ref to use a transaction
 - branch.c: use ref transaction for all ref updates
 - fast-import.c: change update_branch to use ref transactions
 - sequencer.c: use ref transactions for all ref updates
 - commit.c: use ref transactions for updates
 - replace.c: use the ref transaction functions for updates
 - tag.c: use ref transactions when doing updates
 - refs.c: add transaction.status and track OPEN/CLOSED/ERROR
 - refs.c: make ref_transaction_begin take an err argument
 - refs.c: update ref_transaction_delete to check for error and return status
 - refs.c: change ref_transaction_create to do error checking and return status
 (this branch is used by rs/ref-transaction, rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename.)

 The second batch of the transactional ref update series.

Has been merged into pu



rs/ref-transaction (2014-07-17) 12 commits
-
 - refs.c: fix handling of badly named refs
 - refs.c: make write_ref_sha1 static
 - fetch.c: change s_update_ref to use a ref transaction
 - refs.c: propagate any errno==ENOTDIR from _commit back to the callers
 - refs.c: pass a skip list to name_conflict_fn
 - refs.c: call lock_ref_sha1_basic directly from commit
 - refs.c: move the check for valid refname to lock_ref_sha1_basic
 - refs.c: pass NULL as *flags to read_ref_full
 - refs.c: pass the ref log message to _create/delete/update instead of _commit
 - refs.c: add an err argument to delete_ref_loose
 - wrapper.c: add a new function unlink_or_msg
 - wrapper.c: simplify warn_if_unremovable
 (this branch is used by rs/ref-transaction-multi,
rs/ref-transaction-reflog and rs/ref-transaction-rename; uses
rs/ref-transaction-1.)

The third and final part of the basic ref-transaction work.

Has been merged into pu.




rs/ref-transaction-reflog (2014-07-23) 15 commits
---
 - refs.c: allow deleting refs with a broken sha1
 - refs.c: remove lock_any_ref_for_update
 - refs.c: make unlock_ref/close_ref/commit_ref static
 - refs.c: rename log_ref_setup to create_reflog
 - reflog.c: use a reflog transaction when writing during expire
 - refs.c: allow multiple reflog updates during a single transaction
 - refs.c: only write reflog update if msg is non-NULL
 - refs.c: add a flag to allow reflog updates to truncate the log
 - refs.c: add a transaction function to append a reflog entry
 - lockfile.c: make hold_lock_file_for_append preserve meaningful errno
 - refs.c: add a function to append a reflog entry to a fd
 - refs.c: add a new update_type field to ref_update
 - refs.c: rename the transaction functions
 - 

Re: Relative submodule URLs

2014-08-19 Thread Robert Dailey
On Tue, Aug 19, 2014 at 2:19 PM, Junio C Hamano gits...@pobox.com wrote:

 I do not know if that is how GitHub teaches people, but I would have
 to say that these are strange phrasing.  I suspect that part of
 their documentation was written long time ago, back when nobody on
 the GitHub side were involved in design (let alone implementation)
 of Git, and I would take it with a grain of salt.

 Having said that, here is a summary of the current support for
 referring to different repositories in Git:

 snip

Wow, that was a very good read. Thank you for that. I definitely have
been using the wrong terms. upstream  origin are interchangeable, yet
I was using them to represent two distinct repositories.

I think going forward my central repository will be named 'origin' and
for the name of the second, nothing has really jumped out at me yet
but it'll either be fork or proxy... surrogate would be nice too
if it wasn't such a long word in comparison.

I'm sure you guys will find a name for it in good time. I wonder what
Linus would suggest.
--
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: Re: Relative submodule URLs

2014-08-19 Thread Robert Dailey
On Tue, Aug 19, 2014 at 2:30 PM, Heiko Voigt hvo...@hvoigt.net wrote:
 Well the remote for the submodule is currently only calculated once,
 when you do the initial

 git submodule update --init

 that clones the submodule. Afterwards the fixed url is configured under
 the name 'origin' in the submodule like in a normal git repository that
 you have freshly cloned. Which remote is used for cloning depends on the
 configured remote for the current branch or 'origin'.

 When you do a fetch or push with --recurse-submodules it only executes a
 'git fetch' or 'git push' without any specific remote. For fetch the
 same commandline options (but only the options) are passed on.

 Here it might make sense to guess the remote in the submodule somehow
 and not do what fetch without remotes would do.

 For the triangular workflow not much work has been done in regards to
 submodule support.

 But since a submodule behaves like a normal git repository maybe there
 is not much work needed and we can just point to the workflow without
 submodules most times. We still have to figure that out properly.

Maybe then the only thing we need is a --with-remote option for git
submodule? ::

git submodule update --init --with-remote myremote

The --with-remote option would be a NOOP if it's already initialized,
as you say. But I could create an alias for this as needed to make
sure it is always specified.

That way, just in case someone cloned with their fork (in which case
'origin' would not be pointing in the right place), they could tell it
to use `myremote`. This is really the only strange case to handle
right now (people that clone their forks instead of the actual
upstream/central repository).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Johannes Sixt
Am 19.08.2014 19:15, schrieb Robin Rosenberg:
 Using the original filename suffix for the temporary input files to
 the merge tool confuses IDEs like Eclipse. This patch introduces
 a configurtion option, mergetool.tmpsuffix, which get appended to
 the temporary file name. That way the user can choose to use a
 suffix like .tmp, which does not cause confusion.

I have a merge tool that does syntax highlighting based on the file
extension. Given this:

 + BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
 + LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
 + REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
 + BASE=./$MERGED.BASE.$ext$tmpsuffix

I guess I lose syntax highlighting if I were to use mergetool.tmpsuffix;
but then I don't use Eclipse. Could it be that this is really just a
band-aid for Eclipse users, not IDEs in general as you are hinting in
the Documentation of the new variable?

-- Hannes

--
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: Re: Re: Relative submodule URLs

2014-08-19 Thread Heiko Voigt
On Tue, Aug 19, 2014 at 03:23:36PM -0500, Robert Dailey wrote:
 On Tue, Aug 19, 2014 at 2:30 PM, Heiko Voigt hvo...@hvoigt.net wrote:
  Well the remote for the submodule is currently only calculated once,
  when you do the initial
 
  git submodule update --init
 
  that clones the submodule. Afterwards the fixed url is configured under
  the name 'origin' in the submodule like in a normal git repository that
  you have freshly cloned. Which remote is used for cloning depends on the
  configured remote for the current branch or 'origin'.
 
  When you do a fetch or push with --recurse-submodules it only executes a
  'git fetch' or 'git push' without any specific remote. For fetch the
  same commandline options (but only the options) are passed on.
 
  Here it might make sense to guess the remote in the submodule somehow
  and not do what fetch without remotes would do.
 
  For the triangular workflow not much work has been done in regards to
  submodule support.
 
  But since a submodule behaves like a normal git repository maybe there
  is not much work needed and we can just point to the workflow without
  submodules most times. We still have to figure that out properly.
 
 Maybe then the only thing we need is a --with-remote option for git
 submodule? ::
 
 git submodule update --init --with-remote myremote
 
 The --with-remote option would be a NOOP if it's already initialized,
 as you say. But I could create an alias for this as needed to make
 sure it is always specified.

I would actually error out when specified in already cloned state.
Because otherwise the user might expect the remote to be updated.

Since we are currently busy implementing recursive fetch and checkout I have
added that to our ideas list[1] so we do not forget about it.

In the meantime you can either use the branch.name.remote
configuration to define a remote to use or just use 'origin'.

Cheers Heiko

[1] 
https://github.com/jlehmann/git-submod-enhancements/wiki#add-with-remote--switch-to-submodule-update
--
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] imap-send.c: imap_folder - imap_server_conf.folder

2014-08-19 Thread Bernhard Reiter
Rename the imap_folder variable to folder and make it a member
of struct imap_server_conf.

Signed-off-by: Bernhard Reiter ock...@raz.or.at
---
As discussed in
http://www.mail-archive.com/git@vger.kernel.org/msg57019.html

Bernhard

 imap-send.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index fb01a9c..05a02b5 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -69,6 +69,7 @@ struct imap_server_conf {
char *tunnel;
char *host;
int port;
+   char *folder;
char *user;
char *pass;
int use_ssl;
@@ -82,6 +83,7 @@ static struct imap_server_conf server = {
NULL,   /* tunnel */
NULL,   /* host */
0,  /* port */
+   NULL,   /* folder */
NULL,   /* user */
NULL,   /* pass */
0,  /* use_ssl */
@@ -1323,8 +1325,6 @@ static int split_msg(struct strbuf *all_msgs,
struct strbuf *msg, int *ofs)
return 1;
 }
 -static char *imap_folder;
-
 static int git_imap_config(const char *key, const char *val, void *cb)
 {
if (!skip_prefix(key, imap., key))
@@ -1339,7 +1339,7 @@ static int git_imap_config(const char *key, const
char *val, void *cb)
return config_error_nonbool(key);
if (!strcmp(folder, key)) {
-   imap_folder = xstrdup(val);
+   server.folder = xstrdup(val);
} else if (!strcmp(host, key)) {
if (starts_with(val, imap:))
val += 5;
@@ -1387,7 +1387,7 @@ int main(int argc, char **argv)
if (!server.port)
server.port = server.use_ssl ? 993 : 143;
 -  if (!imap_folder) {
+   if (!server.folder) {
fprintf(stderr, no imap store specified\n);
return 1;
}
@@ -1424,7 +1424,7 @@ int main(int argc, char **argv)
}
fprintf(stderr, sending %d message%s\n, total, (total != 1) ? s :
);
-   ctx-name = imap_folder;
+   ctx-name = server.folder;
while (1) {
unsigned percent = n * 100 / total;
 -- 2.1.0.3.g63c96dd


--
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 00/18] Signed push

2014-08-19 Thread Junio C Hamano
While signed tags and commits assert that the objects thusly signed
came from you, who signed these objects, there is not a good way to
assert that you wanted to have a particular object at the tip of a
particular branch.  My signing v2.0.1 tag only means I want to call
the version v2.0.1, and it does not mean I want to push it out to my
'master' branch---it is likely that I only want it in 'maint', so
the signature on the object alone is insufficient.

The only assurance to you that 'maint' points at what I wanted to
place there comes from your trust on the hosting site and my
authentication with it, which cannot easily audited later.

This series introduces a cryptographic assurance for ref updates
done by git push by introducing a mechanism that allows you to
sign a push certificate (for the lack of better name) every time
you push.  Think of it as working on an axis orthogonal to the
traditional signed tags.

The most interesting part starts at 15/18; everything that precedes
that patch are preparatory clean-ups.

[PATCH 15/18] the beginning of the signed push

This step presents the basic idea.  If you remember the
underlying git push protocol exchange, it goes like this:

- The server advertises the existing refs and where they
  point at, and the capabilities the server supports;

- The git push client sends update commands (one or more
  old-sha1 new-sha1 refname) followed by the pack data;

- The server unpacks and updates the refname to point at
  new-sha1.

We introduce push-cert capability, and allow the client to
sign the update commands it will send to the server and
send this signature using the new push-cert command.

This certificate is exported to the pre/post-receive hooks
of the server, so that the pre-receive hook can GPG validate
(and possibly reject a bad push); post-receive hook can log
the certificate.

[PATCH 16/18] receive-pack: GPG-validate push certificates

This step builds a native GPG validation into the server to
help the pre-receive hook.  The signature is verified
against the GPG keychain the server uses (it is likely that
you would want to set and export GNUPGHOME when starting
your server), and verification result is given to the
pre/post-receive hook.

[PATCH 17/18] send-pack: send feature request on push-cert packet
[PATCH 18/18] signed push: final protocol update

With the protocol introduced in 15/18, the update commands
and the push certificate record the same information twice;
the protocol was kept inefficient to make it easier to
review the changes.

These two steps updates the protocol to the final version,
which does not to send the update commands when a push
certificate is in use.

If the server's GPG keychain and pre-receive hook are properly set
up, a git push --signed over an unauthenticated and unencrypted
communication channel (aka git daemon) can be made as secure as,
and even more secure than, the authenticated git push ssh://.

With the signed push certificate, together with the connectivity
check done when the server accepts the packed data, we are assured
that the trusted user vouches for the history leading to the
proposed tips of refs (aka new-sha1s), and a man-in-the-middle
would not be able to make the server accept an update altered in
transit.


Junio C Hamano (18):
  receive-pack: do not overallocate command structure
  receive-pack: parse feature request a bit earlier
  receive-pack: do not reuse old_sha1[] to other things
  receive-pack: factor out queueing of command
  send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher
  send-pack: refactor decision to send update per ref
  send-pack: always send capabilities
  send-pack: factor out capability string generation
  send-pack: rename new_refs to need_pack_data
  send-pack: refactor inspecting and resetting status and sending commands
  send-pack: clarify that cmds_sent is a boolean
  gpg-interface: move parse_gpg_output() to where it should be
  gpg-interface: move parse_signature() to where it should be
  pack-protocol doc: typofix for PKT-LINE
  the beginning of the signed push
  receive-pack: GPG-validate push certificates
  send-pack: send feature request on push-cert packet
  signed push: final protocol update

 Documentation/git-push.txt|   9 +-
 Documentation/git-receive-pack.txt|  30 +++-
 Documentation/technical/pack-protocol.txt |  24 ++-
 Documentation/technical/protocol-capabilities.txt |  12 +-
 builtin/push.c|   1 +
 builtin/receive-pack.c| 161 +++---
 commit.c  |  36 -
 gpg-interface.c   |  57 +++
 gpg-interface.h 

[PATCH 01/18] receive-pack: do not overallocate command structure

2014-08-19 Thread Junio C Hamano
An update command in the protocol exchange consists of 40-hex old
object name, SP, 40-hex new object name, SP, and a refname, but the
first instance is further followed by a NUL with feature requests.

The command structure, which has a flex-array member that stores the
refname at the end, was allocated based on the whole length of the
update command, without excluding the trailing feature requests.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f93ac45..1663beb 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -872,10 +872,11 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
if (parse_feature_request(feature_list, quiet))
quiet = 1;
}
-   cmd = xcalloc(1, sizeof(struct command) + len - 80);
+   cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
hashcpy(cmd-old_sha1, old_sha1);
hashcpy(cmd-new_sha1, new_sha1);
-   memcpy(cmd-ref_name, line + 82, len - 81);
+   memcpy(cmd-ref_name, refname, reflen);
+   cmd-ref_name[reflen] = '\0';
*p = cmd;
p = cmd-next;
}
-- 
2.1.0-301-g54593e2

--
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/18] receive-pack: do not reuse old_sha1[] to other things

2014-08-19 Thread Junio C Hamano
This piece of code reads object names of shallow boundaries,
not old_sha1[].

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 43f35c4..ee855b4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -847,9 +847,11 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
break;
 
if (len == 48  starts_with(line, shallow )) {
-   if (get_sha1_hex(line + 8, old_sha1))
-   die(protocol error: expected shallow sha, got 
'%s', line + 8);
-   sha1_array_append(shallow, old_sha1);
+   unsigned char sha1[20];
+   if (get_sha1_hex(line + 8, sha1))
+   die(protocol error: expected shallow sha, got 
'%s',
+   line + 8);
+   sha1_array_append(shallow, sha1);
continue;
}
 
-- 
2.1.0-301-g54593e2

--
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/18] receive-pack: parse feature request a bit earlier

2014-08-19 Thread Junio C Hamano
Ideally, we should have also allowed the first shallow to carry
the feature request trailer, but that is water under the bridge
now.  This makes the next step to factor out the queuing of commands
easier to review.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 24 +---
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1663beb..43f35c4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -840,7 +840,7 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
unsigned char old_sha1[20], new_sha1[20];
struct command *cmd;
char *refname;
-   int len, reflen;
+   int len, reflen, linelen;
 
line = packet_read_line(0, len);
if (!line)
@@ -853,7 +853,18 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
continue;
}
 
-   if (len  83 ||
+   linelen = strlen(line);
+   if (linelen  len) {
+   const char *feature_list = line + linelen + 1;
+   if (parse_feature_request(feature_list, 
report-status))
+   report_status = 1;
+   if (parse_feature_request(feature_list, 
side-band-64k))
+   use_sideband = LARGE_PACKET_MAX;
+   if (parse_feature_request(feature_list, quiet))
+   quiet = 1;
+   }
+
+   if (linelen  83 ||
line[40] != ' ' ||
line[81] != ' ' ||
get_sha1_hex(line, old_sha1) ||
@@ -863,15 +874,6 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
 
refname = line + 82;
reflen = strlen(refname);
-   if (reflen + 82  len) {
-   const char *feature_list = refname + reflen + 1;
-   if (parse_feature_request(feature_list, 
report-status))
-   report_status = 1;
-   if (parse_feature_request(feature_list, 
side-band-64k))
-   use_sideband = LARGE_PACKET_MAX;
-   if (parse_feature_request(feature_list, quiet))
-   quiet = 1;
-   }
cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
hashcpy(cmd-old_sha1, old_sha1);
hashcpy(cmd-new_sha1, new_sha1);
-- 
2.1.0-301-g54593e2

--
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/18] receive-pack: factor out queueing of command

2014-08-19 Thread Junio C Hamano
Make a helper function to accept a line of a protocol message and
queue an update command out of the code from read_head_info().

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 50 +-
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index ee855b4..341bb46 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -831,16 +831,40 @@ static void execute_commands(struct command *commands,
  the reported refs above);
 }
 
+static struct command **queue_command(struct command **p,
+ const char *line,
+ int linelen)
+{
+   unsigned char old_sha1[20], new_sha1[20];
+   struct command *cmd;
+   const char *refname;
+   int reflen;
+
+   if (linelen  83 ||
+   line[40] != ' ' ||
+   line[81] != ' ' ||
+   get_sha1_hex(line, old_sha1) ||
+   get_sha1_hex(line + 41, new_sha1))
+   die(protocol error: expected old/new/ref, got '%s', line);
+
+   refname = line + 82;
+   reflen = linelen - 82;
+   cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
+   hashcpy(cmd-old_sha1, old_sha1);
+   hashcpy(cmd-new_sha1, new_sha1);
+   memcpy(cmd-ref_name, refname, reflen);
+   cmd-ref_name[reflen] = '\0';
+   *p = cmd;
+   return cmd-next;
+}
+
 static struct command *read_head_info(struct sha1_array *shallow)
 {
struct command *commands = NULL;
struct command **p = commands;
for (;;) {
char *line;
-   unsigned char old_sha1[20], new_sha1[20];
-   struct command *cmd;
-   char *refname;
-   int len, reflen, linelen;
+   int len, linelen;
 
line = packet_read_line(0, len);
if (!line)
@@ -866,23 +890,7 @@ static struct command *read_head_info(struct sha1_array 
*shallow)
quiet = 1;
}
 
-   if (linelen  83 ||
-   line[40] != ' ' ||
-   line[81] != ' ' ||
-   get_sha1_hex(line, old_sha1) ||
-   get_sha1_hex(line + 41, new_sha1))
-   die(protocol error: expected old/new/ref, got '%s',
-   line);
-
-   refname = line + 82;
-   reflen = strlen(refname);
-   cmd = xcalloc(1, sizeof(struct command) + reflen + 1);
-   hashcpy(cmd-old_sha1, old_sha1);
-   hashcpy(cmd-new_sha1, new_sha1);
-   memcpy(cmd-ref_name, refname, reflen);
-   cmd-ref_name[reflen] = '\0';
-   *p = cmd;
-   p = cmd-next;
+   p = queue_command(p, line, linelen);
}
return commands;
 }
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: move REF_STATUS_REJECT_NODELETE logic a bit higher

2014-08-19 Thread Junio C Hamano
20e8b465 (refactor ref status logic for pushing, 2010-01-08)
restructured the code to set status for each ref to be pushed, but
did not quite go far enough.  We inspect the status set earlier by
set_refs_status_for_push() and then perform yet another update to
the status of a ref with an otherwise OK status to be deleted to
mark it with REF_STATUS_REJECT_NODELETE when the protocol tells us
never to delete.

Split the latter into a separate loop that comes before we enter the
per-ref loop.  This way we would have one less condition to check in
the main loop.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 6129b0f..7428ae6 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -231,6 +231,15 @@ int send_pack(struct send_pack_args *args,
return 0;
}
 
+   /*
+* NEEDSWORK: why is delete-refs so specific to send-pack
+* machinery that set_ref_status_for_push() cannot set this
+* bit for us???
+*/
+   for (ref = remote_refs; ref; ref = ref-next)
+   if (ref-deletion  !allow_deleting_refs)
+   ref-status = REF_STATUS_REJECT_NODELETE;
+
if (!args-dry_run)
advertise_shallow_grafts_buf(req_buf);
 
@@ -249,17 +258,13 @@ int send_pack(struct send_pack_args *args,
case REF_STATUS_REJECT_FETCH_FIRST:
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
+   case REF_STATUS_REJECT_NODELETE:
case REF_STATUS_UPTODATE:
continue;
default:
; /* do nothing */
}
 
-   if (ref-deletion  !allow_deleting_refs) {
-   ref-status = REF_STATUS_REJECT_NODELETE;
-   continue;
-   }
-
if (!ref-deletion)
new_refs++;
 
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: always send capabilities

2014-08-19 Thread Junio C Hamano
We tried to avoid sending one extra byte, NUL and nothing behind it
to signal there is no protocol capabilities being sent, on the first
command packet on the wire, but it just made the code look ugly.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index f3c5ebe..2fa6c34 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -281,8 +281,7 @@ int send_pack(struct send_pack_args *args,
char *new_hex = sha1_to_hex(ref-new_sha1);
int quiet = quiet_supported  (args-quiet || 
!args-progress);
 
-   if (!cmds_sent  (status_report || use_sideband ||
-  quiet || agent_supported)) {
+   if (!cmds_sent)
packet_buf_write(req_buf,
 %s %s %s%c%s%s%s%s%s,
 old_hex, new_hex, ref-name, 0,
@@ -292,7 +291,6 @@ int send_pack(struct send_pack_args *args,
 agent_supported ?  agent= : 
,
 agent_supported ? 
git_user_agent_sanitized() : 
);
-   }
else
packet_buf_write(req_buf, %s %s %s,
 old_hex, new_hex, ref-name);
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: refactor decision to send update per ref

2014-08-19 Thread Junio C Hamano
A new helper function ref_update_to_be_sent() decides for each ref
if the update is to be sent based on the status previously set by
set_ref_status_for_push() and also if this is a mirrored push.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 36 +---
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 7428ae6..f3c5ebe 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,6 +190,26 @@ static void advertise_shallow_grafts_buf(struct strbuf *sb)
for_each_commit_graft(advertise_shallow_grafts_cb, sb);
 }
 
+static int ref_update_to_be_sent(const struct ref *ref, const struct 
send_pack_args *args)
+{
+   if (!ref-peer_ref  !args-send_mirror)
+   return 0;
+
+   /* Check for statuses set by set_ref_status_for_push() */
+   switch (ref-status) {
+   case REF_STATUS_REJECT_NONFASTFORWARD:
+   case REF_STATUS_REJECT_ALREADY_EXISTS:
+   case REF_STATUS_REJECT_FETCH_FIRST:
+   case REF_STATUS_REJECT_NEEDS_FORCE:
+   case REF_STATUS_REJECT_STALE:
+   case REF_STATUS_REJECT_NODELETE:
+   case REF_STATUS_UPTODATE:
+   return 0;
+   default:
+   return 1;
+   }
+}
+
 int send_pack(struct send_pack_args *args,
  int fd[], struct child_process *conn,
  struct ref *remote_refs,
@@ -248,23 +268,9 @@ int send_pack(struct send_pack_args *args,
 */
new_refs = 0;
for (ref = remote_refs; ref; ref = ref-next) {
-   if (!ref-peer_ref  !args-send_mirror)
+   if (!ref_update_to_be_sent(ref, args))
continue;
 
-   /* Check for statuses set by set_ref_status_for_push() */
-   switch (ref-status) {
-   case REF_STATUS_REJECT_NONFASTFORWARD:
-   case REF_STATUS_REJECT_ALREADY_EXISTS:
-   case REF_STATUS_REJECT_FETCH_FIRST:
-   case REF_STATUS_REJECT_NEEDS_FORCE:
-   case REF_STATUS_REJECT_STALE:
-   case REF_STATUS_REJECT_NODELETE:
-   case REF_STATUS_UPTODATE:
-   continue;
-   default:
-   ; /* do nothing */
-   }
-
if (!ref-deletion)
new_refs++;
 
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: factor out capability string generation

2014-08-19 Thread Junio C Hamano
A run of 'var ?  var : ' fed to a long printf string in a deeply
nested block was hard to read.  Move it outside the loop and format
it into a strbuf.

As an added bonus, the trick to add agent=agent-name by using
two conditionals is replaced by a more readable version.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 2fa6c34..c1c64ee 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -218,6 +218,7 @@ int send_pack(struct send_pack_args *args,
int in = fd[0];
int out = fd[1];
struct strbuf req_buf = STRBUF_INIT;
+   struct strbuf cap_buf = STRBUF_INIT;
struct ref *ref;
int new_refs;
int allow_deleting_refs = 0;
@@ -251,6 +252,15 @@ int send_pack(struct send_pack_args *args,
return 0;
}
 
+   if (status_report)
+   strbuf_addstr(cap_buf,  report-status);
+   if (use_sideband)
+   strbuf_addstr(cap_buf,  side-band-64k);
+   if (quiet_supported  (args-quiet || !args-progress))
+   strbuf_addstr(cap_buf,  quiet);
+   if (agent_supported)
+   strbuf_addf(cap_buf,  agent=%s, git_user_agent_sanitized());
+
/*
 * NEEDSWORK: why is delete-refs so specific to send-pack
 * machinery that set_ref_status_for_push() cannot set this
@@ -279,18 +289,12 @@ int send_pack(struct send_pack_args *args,
} else {
char *old_hex = sha1_to_hex(ref-old_sha1);
char *new_hex = sha1_to_hex(ref-new_sha1);
-   int quiet = quiet_supported  (args-quiet || 
!args-progress);
 
if (!cmds_sent)
packet_buf_write(req_buf,
-%s %s %s%c%s%s%s%s%s,
+%s %s %s%c%s,
 old_hex, new_hex, ref-name, 0,
-status_report ?  
report-status : ,
-use_sideband ?  
side-band-64k : ,
-quiet ?  quiet : ,
-agent_supported ?  agent= : 
,
-agent_supported ? 
git_user_agent_sanitized() : 
-   );
+cap_buf.buf);
else
packet_buf_write(req_buf, %s %s %s,
 old_hex, new_hex, ref-name);
@@ -311,6 +315,7 @@ int send_pack(struct send_pack_args *args,
packet_flush(out);
}
strbuf_release(req_buf);
+   strbuf_release(cap_buf);
 
if (use_sideband  cmds_sent) {
memset(demux, 0, sizeof(demux));
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: rename new_refs to need_pack_data

2014-08-19 Thread Junio C Hamano
The variable counts how many non-deleting command is being sent, but
is only checked with 0-ness to decide if we need to send the pack
data.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index c1c64ee..590eb0a 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -220,7 +220,7 @@ int send_pack(struct send_pack_args *args,
struct strbuf req_buf = STRBUF_INIT;
struct strbuf cap_buf = STRBUF_INIT;
struct ref *ref;
-   int new_refs;
+   int need_pack_data = 0;
int allow_deleting_refs = 0;
int status_report = 0;
int use_sideband = 0;
@@ -276,13 +276,12 @@ int send_pack(struct send_pack_args *args,
/*
 * Finally, tell the other end!
 */
-   new_refs = 0;
for (ref = remote_refs; ref; ref = ref-next) {
if (!ref_update_to_be_sent(ref, args))
continue;
 
if (!ref-deletion)
-   new_refs++;
+   need_pack_data = 1;
 
if (args-dry_run) {
ref-status = REF_STATUS_OK;
@@ -327,7 +326,7 @@ int send_pack(struct send_pack_args *args,
in = demux.out;
}
 
-   if (new_refs  cmds_sent) {
+   if (need_pack_data  cmds_sent) {
if (pack_objects(out, remote_refs, extra_have, args)  0) {
for (ref = remote_refs; ref; ref = ref-next)
ref-status = REF_STATUS_NONE;
-- 
2.1.0-301-g54593e2

--
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 16/18] receive-pack: GPG-validate push certificates

2014-08-19 Thread Junio C Hamano
Reusing the GPG signature check helpers we already have, verify
the signature in receive-pack and give the results to the hooks
via GIT_PUSH_CERT_{SIGNER,KEY,STATUS} environment variables.

Policy decisions, such as accepting or rejecting a good signature by
a key that is not fully trusted, is left to the hook and kept
outside of the core.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-receive-pack.txt | 22 ++
 builtin/receive-pack.c | 29 +
 t/t5534-push-signed.sh | 18 --
 3 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-receive-pack.txt 
b/Documentation/git-receive-pack.txt
index 6c458af..a66884c 100644
--- a/Documentation/git-receive-pack.txt
+++ b/Documentation/git-receive-pack.txt
@@ -56,7 +56,21 @@ sha1-old and sha1-new should be valid objects in the 
repository.
 When accepting a signed push (see linkgit:git-push[1]), the signed
 push certificate is stored in a blob and an environment variable
 `GIT_PUSH_CERT` can be consulted for its object name.  See the
-description of `post-receive` hook for an example.
+description of `post-receive` hook for an example.  In addition, the
+certificate is verified using GPG and the result is exported with
+the following environment variables:
+
+GIT_PUSH_CERT_SIGNER::
+   The name and the e-mail address of the owner of the key that
+   signed the push certificate.
+
+GIT_PUSH_CERT_KEY::
+   The GPG key ID of the key that signed the push certificate.
+
+GIT_PUSH_CERT_STATUS::
+   The status of GPG verification of the push certificate,
+   using the same mnemonic as used in `%G?` format of `git log`
+   family of commands (see linkgit:git-log[1]).
 
 This hook is called before any refname is updated and before any
 fast-forward checks are performed.
@@ -106,13 +120,13 @@ the update.  Refs that were created will have sha1-old 
equal to
 0\{40}, otherwise sha1-old and sha1-new should be valid objects in
 the repository.
 
-The `GIT_PUSH_CERT` environment variable can be inspected, just as
+The `GIT_PUSH_CERT*` environment variables can be inspected, just as
 in `pre-receive` hook, after accepting a signed push.
 
 Using this hook, it is easy to generate mails describing the updates
 to the repository.  This example script sends one mail message per
 ref listing the commits pushed to the repository, and logs the push
-certificates of signed pushes to a file:
+certificates of signed pushes with good signatures to a file:
 
#!/bin/sh
# mail out commit update information.
@@ -129,7 +143,7 @@ certificates of signed pushes to a file:
mail -s Changes to ref $ref commit-list@mydomain
done
# log signed push certificate, if any
-   if test -n ${GIT_PUSH_CERT-}
+   if test -n ${GIT_PUSH_CERT-}  test ${GIT_PUSH_CERT_STATUS} = G
then
git cat-file blob ${GIT_PUSH_CERT} /var/log/push-log
fi
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index f30df8a..abdc296 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -15,6 +15,8 @@
 #include connected.h
 #include argv-array.h
 #include version.h
+#include tag.h
+#include gpg-interface.h
 
 static const char receive_pack_usage[] = git receive-pack git-dir;
 
@@ -48,6 +50,7 @@ static int shallow_update;
 static const char *alt_shallow_file;
 static struct strbuf push_cert = STRBUF_INIT;
 static unsigned char push_cert_sha1[20];
+static struct signature_check sigcheck;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -260,12 +263,38 @@ static void prepare_push_cert_sha1(struct child_process 
*proc)
struct argv_array env = ARGV_ARRAY_INIT;
 
if (!already_done) {
+   struct strbuf gpg_output = STRBUF_INIT;
+   struct strbuf gpg_status = STRBUF_INIT;
+   int bogs /* beginning_of_gpg_sig */;
+
already_done = 1;
if (write_sha1_file(push_cert.buf, push_cert.len, blob, 
push_cert_sha1))
hashclr(push_cert_sha1);
+
+   memset(sigcheck, '\0', sizeof(sigcheck));
+   sigcheck.result = 'N';
+
+   bogs = parse_signature(push_cert.buf, push_cert.len);
+   if (verify_signed_buffer(push_cert.buf, bogs,
+push_cert.buf + bogs, push_cert.len - 
bogs,
+gpg_output, gpg_status)  0) {
+   ; /* error running gpg */
+   } else {
+   sigcheck.payload = push_cert.buf;
+   sigcheck.gpg_output = gpg_output.buf;
+   sigcheck.gpg_status = gpg_status.buf;
+   parse_gpg_output(sigcheck);
+   }
+
+   strbuf_release(gpg_output);
+   strbuf_release(gpg_status);
}
if 

[PATCH 14/18] pack-protocol doc: typofix for PKT-LINE

2014-08-19 Thread Junio C Hamano
Everywhere else we use PKT-LINE to denote the pkt-line formatted
data, but shallow/deepen messages are described with PKT_LINE().

Fix them.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/technical/pack-protocol.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index 18dea8d..a845d51 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -212,9 +212,9 @@ out of what the server said it could do with the first 
'want' line.
   want-list =  first-want
   *additional-want
 
-  shallow-line  =  PKT_LINE(shallow SP obj-id)
+  shallow-line  =  PKT-LINE(shallow SP obj-id)
 
-  depth-request =  PKT_LINE(deepen SP depth)
+  depth-request =  PKT-LINE(deepen SP depth)
 
   first-want=  PKT-LINE(want SP obj-id SP capability-list LF)
   additional-want   =  PKT-LINE(want SP obj-id LF)
-- 
2.1.0-301-g54593e2

--
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/18] send-pack: refactor inspecting and resetting status and sending commands

2014-08-19 Thread Junio C Hamano
The main loop over remote_refs list inspects the ref status
to see if we need to generate pack data (i.e. a delete-only push
does not need to send any additional data), resets it to expecting
the status report state, and formats the actual update commands
to be sent.

Split the former two out of the main loop, as it will become
conditional in later steps.

Besides, we should have code that does real thing here, before the
Finally, tell the other end! part ;-)

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 49 ++---
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 590eb0a..f3262f2 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -274,7 +274,8 @@ int send_pack(struct send_pack_args *args,
advertise_shallow_grafts_buf(req_buf);
 
/*
-* Finally, tell the other end!
+* Clear the status for each ref and see if we need to send
+* the pack data.
 */
for (ref = remote_refs; ref; ref = ref-next) {
if (!ref_update_to_be_sent(ref, args))
@@ -283,25 +284,35 @@ int send_pack(struct send_pack_args *args,
if (!ref-deletion)
need_pack_data = 1;
 
-   if (args-dry_run) {
+   if (args-dry_run || !status_report)
ref-status = REF_STATUS_OK;
-   } else {
-   char *old_hex = sha1_to_hex(ref-old_sha1);
-   char *new_hex = sha1_to_hex(ref-new_sha1);
-
-   if (!cmds_sent)
-   packet_buf_write(req_buf,
-%s %s %s%c%s,
-old_hex, new_hex, ref-name, 0,
-cap_buf.buf);
-   else
-   packet_buf_write(req_buf, %s %s %s,
-old_hex, new_hex, ref-name);
-   ref-status = status_report ?
-   REF_STATUS_EXPECTING_REPORT :
-   REF_STATUS_OK;
-   cmds_sent++;
-   }
+   else
+   ref-status = REF_STATUS_EXPECTING_REPORT;
+   }
+
+   /*
+* Finally, tell the other end!
+*/
+   for (ref = remote_refs; ref; ref = ref-next) {
+   char *old_hex, *new_hex;
+
+   if (args-dry_run)
+   continue;
+
+   if (!ref_update_to_be_sent(ref, args))
+   continue;
+
+   old_hex = sha1_to_hex(ref-old_sha1);
+   new_hex = sha1_to_hex(ref-new_sha1);
+   if (!cmds_sent)
+   packet_buf_write(req_buf,
+%s %s %s%c%s,
+old_hex, new_hex, ref-name, 0,
+cap_buf.buf);
+   else
+   packet_buf_write(req_buf, %s %s %s,
+old_hex, new_hex, ref-name);
+   cmds_sent++;
}
 
if (args-stateless_rpc) {
-- 
2.1.0-301-g54593e2

--
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 13/18] gpg-interface: move parse_signature() to where it should be

2014-08-19 Thread Junio C Hamano
Our signed-tag objects set the standard format used by Git to store
GPG-signed payload (i.e. the payload followed by its detached
signature), and it made sense to have a helper to find the boundary
between the payload and its signature in tag.c back then.

Newer code added later to parse other kinds of objects that learned
to use the same format to store GPG-signed payload (e.g. signed
commits), however, kept using the helper from the same location.

Move it to gpg-interface; the helper is no longer about signed tag,
but it is how our code and data interact with GPG.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 gpg-interface.c | 21 +
 gpg-interface.h |  1 +
 tag.c   | 20 
 tag.h   |  1 -
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/gpg-interface.c b/gpg-interface.c
index 3c9624c..0dd11ea 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -7,6 +7,9 @@
 static char *configured_signing_key;
 static const char *gpg_program = gpg;
 
+#define PGP_SIGNATURE -BEGIN PGP SIGNATURE-
+#define PGP_MESSAGE -BEGIN PGP MESSAGE-
+
 void signature_check_clear(struct signature_check *sigc)
 {
free(sigc-payload);
@@ -57,6 +60,24 @@ void parse_gpg_output(struct signature_check *sigc)
}
 }
 
+/*
+ * Look at GPG signed content (e.g. a signed tag object), whose
+ * payload is followed by a detached signature on it.  Return the
+ * offset where the embedded detached signature begins, or the end of
+ * the data when there is no such signature.
+ */
+size_t parse_signature(const char *buf, unsigned long size)
+{
+   char *eol;
+   size_t len = 0;
+   while (len  size  !starts_with(buf + len, PGP_SIGNATURE) 
+   !starts_with(buf + len, PGP_MESSAGE)) {
+   eol = memchr(buf + len, '\n', size - len);
+   len += eol ? eol - (buf + len) + 1 : size - len;
+   }
+   return len;
+}
+
 void set_signing_key(const char *key)
 {
free(configured_signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index 8d677cc..93c76c2 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -20,6 +20,7 @@ struct signature_check {
 };
 
 extern void signature_check_clear(struct signature_check *sigc);
+extern size_t parse_signature(const char *buf, unsigned long size);
 extern void parse_gpg_output(struct signature_check *);
 
 extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const 
char *signing_key);
diff --git a/tag.c b/tag.c
index 82d841b..5b0ac62 100644
--- a/tag.c
+++ b/tag.c
@@ -4,9 +4,6 @@
 #include tree.h
 #include blob.h
 
-#define PGP_SIGNATURE -BEGIN PGP SIGNATURE-
-#define PGP_MESSAGE -BEGIN PGP MESSAGE-
-
 const char *tag_type = tag;
 
 struct object *deref_tag(struct object *o, const char *warn, int warnlen)
@@ -143,20 +140,3 @@ int parse_tag(struct tag *item)
free(data);
return ret;
 }
-
-/*
- * Look at a signed tag object, and return the offset where
- * the embedded detached signature begins, or the end of the
- * data when there is no such signature.
- */
-size_t parse_signature(const char *buf, unsigned long size)
-{
-   char *eol;
-   size_t len = 0;
-   while (len  size  !starts_with(buf + len, PGP_SIGNATURE) 
-   !starts_with(buf + len, PGP_MESSAGE)) {
-   eol = memchr(buf + len, '\n', size - len);
-   len += eol ? eol - (buf + len) + 1 : size - len;
-   }
-   return len;
-}
diff --git a/tag.h b/tag.h
index bc8a1e4..f4580ae 100644
--- a/tag.h
+++ b/tag.h
@@ -17,6 +17,5 @@ extern int parse_tag_buffer(struct tag *item, const void 
*data, unsigned long si
 extern int parse_tag(struct tag *item);
 extern struct object *deref_tag(struct object *, const char *, int);
 extern struct object *deref_tag_noverify(struct object *);
-extern size_t parse_signature(const char *buf, unsigned long size);
 
 #endif /* TAG_H */
-- 
2.1.0-301-g54593e2

--
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/18] gpg-interface: move parse_gpg_output() to where it should be

2014-08-19 Thread Junio C Hamano
Earlier, ffb6d7d5 (Move commit GPG signature verification to
commit.c, 2013-03-31) moved this helper that used to be in pretty.c
(i.e. the output code path) to commit.c for better reusability.

It was a good first step in the right direction, but still suffers a
myopic view that commits will be the only thing we would ever want
to sign---we would actually want to be able to reuse it even wider.

The function interprets what GPG said; gpg-interface is obviously a
better place.  Move it there.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 commit.c| 36 
 gpg-interface.c | 36 
 gpg-interface.h | 17 -
 3 files changed, 48 insertions(+), 41 deletions(-)

diff --git a/commit.c b/commit.c
index ae7f2b1..01cdad2 100644
--- a/commit.c
+++ b/commit.c
@@ -1220,42 +1220,6 @@ free_return:
free(buf);
 }
 
-static struct {
-   char result;
-   const char *check;
-} sigcheck_gpg_status[] = {
-   { 'G', \n[GNUPG:] GOODSIG  },
-   { 'B', \n[GNUPG:] BADSIG  },
-   { 'U', \n[GNUPG:] TRUST_NEVER },
-   { 'U', \n[GNUPG:] TRUST_UNDEFINED },
-};
-
-static void parse_gpg_output(struct signature_check *sigc)
-{
-   const char *buf = sigc-gpg_status;
-   int i;
-
-   /* Iterate over all search strings */
-   for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
-   const char *found, *next;
-
-   if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, 
found)) {
-   found = strstr(buf, sigcheck_gpg_status[i].check);
-   if (!found)
-   continue;
-   found += strlen(sigcheck_gpg_status[i].check);
-   }
-   sigc-result = sigcheck_gpg_status[i].result;
-   /* The trust messages are not followed by key/signer 
information */
-   if (sigc-result != 'U') {
-   sigc-key = xmemdupz(found, 16);
-   found += 17;
-   next = strchrnul(found, '\n');
-   sigc-signer = xmemdupz(found, next - found);
-   }
-   }
-}
-
 void check_commit_signature(const struct commit* commit, struct 
signature_check *sigc)
 {
struct strbuf payload = STRBUF_INIT;
diff --git a/gpg-interface.c b/gpg-interface.c
index ff07012..3c9624c 100644
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -21,6 +21,42 @@ void signature_check_clear(struct signature_check *sigc)
sigc-key = NULL;
 }
 
+static struct {
+   char result;
+   const char *check;
+} sigcheck_gpg_status[] = {
+   { 'G', \n[GNUPG:] GOODSIG  },
+   { 'B', \n[GNUPG:] BADSIG  },
+   { 'U', \n[GNUPG:] TRUST_NEVER },
+   { 'U', \n[GNUPG:] TRUST_UNDEFINED },
+};
+
+void parse_gpg_output(struct signature_check *sigc)
+{
+   const char *buf = sigc-gpg_status;
+   int i;
+
+   /* Iterate over all search strings */
+   for (i = 0; i  ARRAY_SIZE(sigcheck_gpg_status); i++) {
+   const char *found, *next;
+
+   if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, 
found)) {
+   found = strstr(buf, sigcheck_gpg_status[i].check);
+   if (!found)
+   continue;
+   found += strlen(sigcheck_gpg_status[i].check);
+   }
+   sigc-result = sigcheck_gpg_status[i].result;
+   /* The trust messages are not followed by key/signer 
information */
+   if (sigc-result != 'U') {
+   sigc-key = xmemdupz(found, 16);
+   found += 17;
+   next = strchrnul(found, '\n');
+   sigc-signer = xmemdupz(found, next - found);
+   }
+   }
+}
+
 void set_signing_key(const char *key)
 {
free(configured_signing_key);
diff --git a/gpg-interface.h b/gpg-interface.h
index 37c23da..8d677cc 100644
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -5,16 +5,23 @@ struct signature_check {
char *payload;
char *gpg_output;
char *gpg_status;
-   char result; /* 0 (not checked),
- * N (checked but no further result),
- * U (untrusted good),
- * G (good)
- * B (bad) */
+
+   /*
+* possible result:
+* 0 (not checked)
+* N (checked but no further result)
+* U (untrusted good)
+* G (good)
+* B (bad)
+*/
+   char result;
char *signer;
char *key;
 };
 
 extern void signature_check_clear(struct signature_check *sigc);
+extern void parse_gpg_output(struct signature_check *);
+
 extern int sign_buffer(struct strbuf *buffer, struct strbuf *signature, const 
char *signing_key);
 extern int verify_signed_buffer(const char *payload, size_t payload_size, 
const char *signature, size_t 

[PATCH 18/18] signed push: final protocol update

2014-08-19 Thread Junio C Hamano
With the interim protocol, we used to send the update commands even
though we already send a signed copy of the same information when
push certificate is in use.  Update the send-pack/receive-pack pair
not to do so.

The notable thing on the receive-pack side is that it makes sure
that there is no command sent over the traditional protocol packet
outside the push certificate.  Otherwise a pusher can claim to be
pushing one set of ref updates in the signed certificate while
issuing commands to update unrelated refs, and such an update will
evade later audits.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/technical/pack-protocol.txt | 20 -
 Documentation/technical/protocol-capabilities.txt | 12 +--
 builtin/receive-pack.c| 26 +++
 send-pack.c   |  2 +-
 4 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/Documentation/technical/pack-protocol.txt 
b/Documentation/technical/pack-protocol.txt
index a845d51..f6c3073 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -465,7 +465,7 @@ contain all the objects that the server will need to 
complete the new
 references.
 
 
-  update-request=  *shallow command-list [pack-file]
+  update-request=  *shallow ( command-list | push-cert ) [pack-file]
 
   shallow   =  PKT-LINE(shallow SP obj-id)
 
@@ -481,12 +481,30 @@ references.
   old-id=  obj-id
   new-id=  obj-id
 
+  push-cert = PKT-LINE(push-cert NUL capability-list LF)
+ PKT-LINE(certificate version 0.1 LF)
+ PKT-LINE(pusher ident LF)
+ PKT-LINE(LF)
+ *PKT-LINE(command LF)
+ *PKT-LINE(GPG signature lines LF)
+ PKT-LINE(push-cert-end LF)
+
   pack-file = PACK 28*(OCTET)
 
 
 If the receiving end does not support delete-refs, the sending end MUST
 NOT ask for delete command.
 
+If the receiving end does not support push-cert, the sending end MUST
+NOT send a push-cert command.
+
+When a push-cert command is sent, command-list MUST NOT be sent; the
+commands recorded in the push certificate is used instead.  The GPG
+signature lines are detached signature for the contents recorded in
+the push certificate before the signature block begins and is used
+to certify that the commands were given by the pusher which must be
+the signer.
+
 The pack-file MUST NOT be sent if the only command used is 'delete'.
 
 A pack-file MUST be sent if either create or update command is used,
diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index e174343..5b398e9 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,8 +18,8 @@ was sent.  Server MUST NOT ignore capabilities that client 
requested
 and server advertised.  As a consequence of these rules, server MUST
 NOT advertise capabilities it does not understand.
 
-The 'report-status', 'delete-refs', and 'quiet' capabilities are sent and
-recognized by the receive-pack (push to server) process.
+The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities
+are sent and recognized by the receive-pack (push to server) process.
 
 The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
 by both upload-pack and receive-pack protocols.  The 'agent' capability
@@ -250,3 +250,11 @@ allow-tip-sha1-in-want
 If the upload-pack server advertises this capability, fetch-pack may
 send want lines with SHA-1s that exist at the server but are not
 advertised by upload-pack.
+
+push-cert
+-
+
+The receive-pack server that advertises this capability is willing
+to accept a signed push certificate.  A send-pack client MUST NOT
+send push-cert packet unless the receive-pack server advertises this
+capability.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index abdc296..96e4c99 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -907,6 +907,28 @@ static struct command **queue_command(struct command **p,
return cmd-next;
 }
 
+static void queue_commands_from_cert(struct command **p,
+struct strbuf *push_cert)
+{
+   const char *boc, *eoc;
+
+   if (*p)
+   die(protocol error: got both push certificate and unsigned 
commands);
+
+   boc = strstr(push_cert-buf, \n\n);
+   if (!boc)
+   die(malformed push certificate %.*s, 100, push_cert-buf);
+   else
+   boc += 2;
+   eoc = push_cert-buf + parse_signature(push_cert-buf, push_cert-len);
+
+   while (boc  eoc) {
+   const char *eol = memchr(boc, '\n', eoc - boc);
+   p = queue_command(p, boc, eol ? eol - boc : eoc - eol);
+   boc = 

[PATCH 11/18] send-pack: clarify that cmds_sent is a boolean

2014-08-19 Thread Junio C Hamano
We use it to make sure that the feature request is sent only once on
the very first request packet (ignoring the shallow  line, which
was an unfortunate mistake we cannot retroactively fix with existing
receive-pack already deployed in the field) and we set it to true
with cmds_sent++, not because we care about the actual number of
updates sent but because it is merely an old idiomatic way.

Set it explicitly to one to clarify that the code that uses this
variable only cares about its zero-ness.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index f3262f2..05926d2 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -304,15 +304,16 @@ int send_pack(struct send_pack_args *args,
 
old_hex = sha1_to_hex(ref-old_sha1);
new_hex = sha1_to_hex(ref-new_sha1);
-   if (!cmds_sent)
+   if (!cmds_sent) {
packet_buf_write(req_buf,
 %s %s %s%c%s,
 old_hex, new_hex, ref-name, 0,
 cap_buf.buf);
-   else
+   cmds_sent = 1;
+   } else {
packet_buf_write(req_buf, %s %s %s,
 old_hex, new_hex, ref-name);
-   cmds_sent++;
+   }
}
 
if (args-stateless_rpc) {
-- 
2.1.0-301-g54593e2

--
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 15/18] the beginning of the signed push

2014-08-19 Thread Junio C Hamano
While signed tags and commits assert that the objects thusly signed
came from you, who signed these objects, there is not a good way to
assert that you wanted to have a particular object at the tip of a
particular branch.  My signing v2.0.1 tag only means I want to call
the version v2.0.1, and it does not mean I want to push it out to my
'master' branch---it is likely that I only want it in 'maint'.

Introduce a mechanism that allows you to sign a push certificate
(for the lack of better name) every time you push, asserting that
what object you are pushing to update which ref that used to point
at what other object.  Think of it as a cryptographic protection for
ref updates, similar to signed tags/commits but working on an
orthogonal axis.

The basic flow based on this mechanism goes like this:

 1. You push out your work with git push -s.

 2. The sending side learns where the remote refs are as usual,
together with what protocol extension the receiving end
supports.  If the receiving end does not advertise the protocol
extension push-cert, the sending side falls back to the normal
push that is not signed.

Otherwise, a text file, that looks like the following, is
prepared in core:

certificate version 0.1
pusher Junio C Hamano gits...@pobox.com 1315427886 -0700

7339ca65... 21580ecb... refs/heads/master
3793ac56... 12850bec... refs/heads/next

Each line shows the old and the new object name at the tip of
the ref this push tries to update, in the way identical to how
the underlying git push protocol exchange tells the ref
updates to the receiving end (by recording the old object
name, the push certificate also protects against replaying).  It
is expected that new command packet types other than the
old-new-refname kind will be included in push certificate in the
same way as would appear in the plain vanilla command packets in
unsigned pushes.

The user then is asked to sign this push certificate using GPG,
formatted in a way similar to how signed tag objects are signed,
and the result is sent to the other side (i.e. receive-pack).

In the protocol exchange, this step comes immediately before the
sender tells what the result of the push should be, which in
turn comes before it sends the pack data.

 3. When the receiving end sees a push certificate, the certificate
is written out as a blob.  The pre-receive hook can learn about
the certificate by checking GIT_PUSH_CERT environment variable,
which, if present, tells the object name of this blob, and make
the decision to allow or reject this push.  Additionally, the
post-receive hook can also look at the certificate, which may be
a good place to log all the received certificates for later
audits.

Because a push certificate carry the same information as the usual
command packets in the protocol exchange, we can omit the latter
when a push certificate is in use and reduce the protocol overhead.
This however is not included in this patch to make it easier to
review (in other words, the series at this step should never be
released without the remainder of the series, as it implements an
interim protocol that will be incompatible with the final one,
merely for reviewing purposes).  As such, the documentation update
for the protocol is left out of this step.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 Documentation/git-push.txt |  9 +-
 Documentation/git-receive-pack.txt | 16 -
 builtin/push.c |  1 +
 builtin/receive-pack.c | 43 -
 send-pack.c| 66 ++
 send-pack.h|  1 +
 t/t5534-push-signed.sh | 63 
 transport.c|  4 +++
 transport.h|  5 +++
 9 files changed, 205 insertions(+), 3 deletions(-)
 create mode 100755 t/t5534-push-signed.sh

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21cd455..21b3f29 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -10,7 +10,8 @@ SYNOPSIS
 
 [verse]
 'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run] 
[--receive-pack=git-receive-pack]
-  [--repo=repository] [-f | --force] [--prune] [-v | --verbose] [-u 
| --set-upstream]
+  [--repo=repository] [-f | --force] [--prune] [-v | --verbose]
+  [-u | --set-upstream] [--signed]
   [--force-with-lease[=refname[:expect]]]
   [--no-verify] [repository [refspec...]]
 
@@ -129,6 +130,12 @@ already exists on the remote side.
from the remote but are pointing at commit-ish that are
reachable from the refs being pushed.
 
+--signed::
+   GPG-sign the push request to update refs on the receiving
+   side, to allow it to be checked by the hooks and/or be

[PATCH 17/18] send-pack: send feature request on push-cert packet

2014-08-19 Thread Junio C Hamano
We would want to update the interim protocol so that we do not send
the usual update commands when the push certificate feature is in
use, as the same information is in the certificate.  Once that
happens, the push-cert packet may become the only protocol command,
but then there is no packet to put the feature request behind, like
we always did.

As we have prepared the receiving end that understands the push-cert
feature to accept the feature request on the first protocol packet
(other than shallow , which was an unfortunate historical mistake
that has to come before everything else), we can give the feature
request on the push-cert packet instead of the first update protocol
packet, in preparation for the next step to actually update to the
final protocol.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 send-pack.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index a073891..2de9826 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -225,9 +225,10 @@ static const char *next_line(const char *line, size_t len)
return nl + 1;
 }
 
-static void generate_push_cert(struct strbuf *req_buf,
-  const struct ref *remote_refs,
-  struct send_pack_args *args)
+static int generate_push_cert(struct strbuf *req_buf,
+ const struct ref *remote_refs,
+ struct send_pack_args *args,
+ const char *cap_string)
 {
const struct ref *ref;
char stamp[60];
@@ -256,7 +257,7 @@ static void generate_push_cert(struct strbuf *req_buf,
if (sign_buffer(cert, cert, signing_key))
die(_(failed to sign the push certificate));
 
-   packet_buf_write(req_buf, push-cert\n);
+   packet_buf_write(req_buf, push-cert%c%s, 0, cap_string);
for (cp = cert.buf; cp  cert.buf + cert.len; cp = np) {
np = next_line(cp, cert.buf + cert.len - cp);
packet_buf_write(req_buf,
@@ -267,6 +268,7 @@ static void generate_push_cert(struct strbuf *req_buf,
 free_return:
free(signing_key);
strbuf_release(cert);
+   return 1;
 }
 
 int send_pack(struct send_pack_args *args,
@@ -337,7 +339,8 @@ int send_pack(struct send_pack_args *args,
advertise_shallow_grafts_buf(req_buf);
 
if (!args-dry_run  args-push_cert)
-   generate_push_cert(req_buf, remote_refs, args);
+   cmds_sent = generate_push_cert(req_buf, remote_refs, args,
+  cap_buf.buf);
 
/*
 * Clear the status for each ref and see if we need to send
-- 
2.1.0-301-g54593e2

--
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] Allow the user to change the temporary file name for mergetool

2014-08-19 Thread Junio C Hamano
Johannes Sixt j...@kdbg.org writes:

 Am 19.08.2014 19:15, schrieb Robin Rosenberg:
 Using the original filename suffix for the temporary input files to
 the merge tool confuses IDEs like Eclipse. This patch introduces
 a configurtion option, mergetool.tmpsuffix, which get appended to
 the temporary file name. That way the user can choose to use a
 suffix like .tmp, which does not cause confusion.

 I have a merge tool that does syntax highlighting based on the file
 extension. Given this:

 +BACKUP=./$MERGED.BACKUP.$ext$tmpsuffix
 +LOCAL=./$MERGED.LOCAL.$ext$tmpsuffix
 +REMOTE=./$MERGED.REMOTE.$ext$tmpsuffix
 +BASE=./$MERGED.BASE.$ext$tmpsuffix

 I guess I lose syntax highlighting if I were to use mergetool.tmpsuffix;
 but then I don't use Eclipse. Could it be that this is really just a
 band-aid for Eclipse users, not IDEs in general as you are hinting in
 the Documentation of the new variable?

The phrase IDEs like Eclipse in the proposed log message did not
tell me (which I think is a good thing) if IDEs that need band-aid
are majority or minority, but I agree that we should not hint that
IDEs in general would benefit by setting this variable.  A warning
on the syntax-aware editors may be necessary.

Thanks for a careful reading.



--
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: Transaction patch series overview

2014-08-19 Thread Junio C Hamano
Ronnie Sahlberg sahlb...@google.com writes:

 List, please see here an overview and ordering of the ref transaction
 patch series.
 These series build on each other and needs to be applied in the order
 listed below.

 This is an update.

 rs/ref-transaction-0
 ---
 Early part of the ref transaction topic.

 * rs/ref-transaction-0:
   refs.c: change ref_transaction_update() to do error checking and
 return status
   refs.c: remove the onerr argument to ref_transaction_commit
   update-ref: use err argument to get error from ref_transaction_commit
   refs.c: make update_ref_write update a strbuf on failure
   refs.c: make ref_update_reject_duplicates take a strbuf argument
 for errors
   refs.c: log_ref_write should try to return meaningful errno
   refs.c: make resolve_ref_unsafe set errno to something meaningful on 
 error
   refs.c: commit_packed_refs to return a meaningful errno on failure
   refs.c: make remove_empty_directories always set errno to something sane
   refs.c: verify_lock should set errno to something meaningful
   refs.c: make sure log_ref_setup returns a meaningful errno
   refs.c: add an err argument to repack_without_refs
   lockfile.c: make lock_file return a meaningful errno on failurei
   lockfile.c: add a new public function unable_to_lock_message
   refs.c: add a strbuf argument to ref_transaction_commit for error 
 logging
   refs.c: allow passing NULL to ref_transaction_free
   refs.c: constify the sha arguments for
 ref_transaction_create|delete|update
   refs.c: ref_transaction_commit should not free the transaction
   refs.c: remove ref_transaction_rollback

 Has been merged into next.

I gave a quick re-read-thru on this and did not find anything
questionable, nor recall any issue pending.  Unless somebody
objects in the coming few days, let's move it to 'master' as part of
the first batch for 2.2.

 ref-transaction-1 (2014-07-16) 20 commits
 ...
  The second batch of the transactional ref update series.

 Has been merged into pu

 rs/ref-transaction (2014-07-17) 12 commits
 ...
 The third and final part of the basic ref-transaction work.
 
 Has been merged into pu.

I'll re-read these two before deciding to merge them to 'next'.  Any
pending issues from the previous discussions?

 This series is for once the previous transaction changes are in and
 this series will add
 a new backend for refs: refs-be-db.c which offloads all refs access to
 an external database daemon.
 An example reference implementation for an external daemon is also
 provided and can be used as basis for
 creating a daemon to plug into, say, a SQL database or similar.

A filesystem based backend/external daemon that is compatible with
the traditional on-disk format might be another fun demonstration.

Looking forward to shepherding the whole thing ahead ;-)  The most
fun part awaits us but there is a long way.

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 02/18] receive-pack: parse feature request a bit earlier

2014-08-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Ideally, we should have also allowed the first shallow to carry
 the feature request trailer, but that is water under the bridge
 now.  This makes the next step to factor out the queuing of commands
 easier to review.

 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
 ...
 @@ -853,7 +853,18 @@ static struct command *read_head_info(struct sha1_array 
 *shallow)
   continue;
   }
  
 - if (len  83 ||
 + linelen = strlen(line);
 + if (linelen  len) {
 + const char *feature_list = line + linelen + 1;
 + if (parse_feature_request(feature_list, 
 report-status))
 + report_status = 1;
 + if (parse_feature_request(feature_list, 
 side-band-64k))
 + use_sideband = LARGE_PACKET_MAX;
 + if (parse_feature_request(feature_list, quiet))
 + quiet = 1;
 + }
 +
 + if (linelen  83 ||
   line[40] != ' ' ||
   line[81] != ' ' ||
   get_sha1_hex(line, old_sha1) ||
 @@ -863,15 +874,6 @@ static struct command *read_head_info(struct sha1_array 
 *shallow)
  
   refname = line + 82;
   reflen = strlen(refname);

A later patch updates this to reflen = linelen - 82 while moving
this code to a helper function, but it may be better to do that in
this patch.
--
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 03/18] receive-pack: do not reuse old_sha1[] to other things

2014-08-19 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 This piece of code reads object names of shallow boundaries,
 not old_sha1[].

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

I should double the subject line and say do not reuse ... for other
things.

--
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 00/18] Signed push

2014-08-19 Thread Duy Nguyen
On Wed, Aug 20, 2014 at 5:06 AM, Junio C Hamano gits...@pobox.com wrote:
 While signed tags and commits assert that the objects thusly signed
 came from you, who signed these objects, there is not a good way to
 assert that you wanted to have a particular object at the tip of a
 particular branch.  My signing v2.0.1 tag only means I want to call
 the version v2.0.1, and it does not mean I want to push it out to my
 'master' branch---it is likely that I only want it in 'maint', so
 the signature on the object alone is insufficient.

 The only assurance to you that 'maint' points at what I wanted to
 place there comes from your trust on the hosting site and my
 authentication with it, which cannot easily audited later.

I only had a quick read of a few important patches and may miss
something. But all this audit recording is left to the hook, right? I
suppose git-notes could be used to store the push cert. blob, or the
server could make a signed tag to record this info in the ref.. or do
you intend any other way to record these blobs?
-- 
Duy
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >