Re: Additional plumbing commands

[Jeff King]

On Tue, Jan 06, 2015 at 06:37:34PM +0100, Christian Couder wrote:

 On Tue, Jan 6, 2015 at 5:05 PM, Charles Rudolph
 charles.w.rudo...@gmail.com wrote:
  I am writing some higher level git commands for
  https://github.com/Originate/git-town and would like some additional
  plumbing commands that can tell me
 
  1. is there a merge in progress?
  2. is there a rebase in progress?
  3. is there a cherry-pick in progress?
  4. are there unmerged paths?
 
  Currently the only way I know how to do this is with git status and
  looking for specific text.
 
 You may have a look at how contrib/completion/git-prompt.sh does it.
 [...]

The prompt code is rather long and knows a lot about the internal state
of $GIT_DIR. I do not think it would be a bad thing for git-status to
expose a machine-readable version of the state it discovers, and then at
least we can keep the logic in one place.

Charles, if you are interested in adding that, the wt_status_state code
in wt-status.c is the right place to start looking.

Though I think in many cases that discovering which state we are in is
only half the story that a caller wants. Knowing what each state _means_
and what operations are meaningful to perform is much trickier (e.g., if
we are in a rebase, you probably do not want to start a new rebase. But
is it wrong to cherry-pick?).

It would be nice if we could find a way to generalize in-progress
operations and what they mean for starting new operations, but that is
a much harder problem (if it is even possible at all).

-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] clean: style fix for 9f93e46 (git-clean: use a git-add-interactive ...)

[Duy Nguyen]

On Wed, Jan 7, 2015 at 2:17 AM, Junio C Hamano gits...@pobox.com wrote:
 When I do a SQUASH???, I expect the original authors use it as a
 hint in their rerolls, but because this series has seen no comments
 so far (no interests???), I do not foresee or expect you to feel a
 need for rerolling at this point.  If you agree that the remainder
 of the SQUASH??? (shown below) is sensible, I'll turn it into a
 fixup! for cc44d4fe (untracked cache: load from UNTR index
 extension, 2014-12-08) and requeue.

No there will be a reroll. I was going through the comments and this
is the easiest piece. Glad to hear it's already in 'maint'. The
remaining change in your SQUASH??? will be squashed in the right
patch.
-- 
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] git-completion: add --autostash for 'git rebase'

[Matthieu Moy]

This option was added in 587947750bd73544 (not to be confused with
--autosquash).

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 contrib/completion/git-completion.bash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 23988ec..5c369f7 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1693,6 +1693,7 @@ _git_rebase ()
--committer-date-is-author-date --ignore-date
--ignore-whitespace --whitespace=
--autosquash --fork-point --no-fork-point
+   --autostash

 
return
-- 
2.0.2.737.gfb43bde

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

[PATCHv12 00/10] atomic pushes

[Stefan Beller]

There wasn't discussion within the last 24 hours and we were discussing only 
about minor changes. Changes compared to v9 (the last time I completely sent the
series) is only found in the first 2 patches, where we had a back and forth 
about naming the method warn_if_skipped_connectivity_check and its behavior.
I'm mainly sending this as a whole series, so Junio can pick it up easier
as opposed to finding the latest version of each patch himself.

This patch series adds a flag to git push to update the remote refs atomically.
This series applies on top of origin/mh/reflog-expire
It can also be found at https://github.com/stefanbeller/git/tree/atomic-push-v12

Any comment is welcome!

Thanks,
Stefan

Ronnie Sahlberg (3):
  receive-pack.c: negotiate atomic push support
  send-pack.c: add --atomic command line argument
  push.c: add an --atomic argument

Stefan Beller (7):
  receive-pack.c: shorten the execute_commands loop over all commands
  receive-pack.c: die instead of error in case of possible future bug
  receive-pack.c: move iterating over all commands outside
execute_commands
  receive-pack.c: move transaction handling in a central place
  receive-pack.c: add execute_commands_atomic function
  send-pack: rename ref_update_to_be_sent to check_to_send_update
  t5543-atomic-push.sh: add basic tests for atomic pushes

 Documentation/git-push.txt|   7 +-
 Documentation/git-send-pack.txt   |   7 +-
 Documentation/technical/protocol-capabilities.txt |  13 +-
 builtin/push.c|   5 +
 builtin/receive-pack.c| 165 ++
 builtin/send-pack.c   |   6 +-
 remote.h  |   3 +-
 send-pack.c   |  65 +++-
 send-pack.h   |   3 +-
 t/t5543-atomic-push.sh| 194 ++
 transport.c   |   5 +
 transport.h   |   1 +
 12 files changed, 424 insertions(+), 50 deletions(-)
 create mode 100755 t/t5543-atomic-push.sh

-- 
2.2.1.62.g3f15098

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

[PATCHv12 06/10] receive-pack.c: negotiate atomic push support

[Stefan Beller]

From: Ronnie Sahlberg sahlb...@google.com

This adds the atomic protocol option to allow
receive-pack to inform the client that it has
atomic push capability.

This commit makes the functionality introduced
in the previous commits go live for the serving
side. The changes in documentation reflect the
protocol capabilities of the server.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v9:
 This was once part of [PATCH 1/7] receive-pack.c:
 add protocol support to negotiate atomic-push
 but now it only touches the receive-pack.c part
 and doesn't bother with the send-pack part any more.
 That is done in a later patch, when send-pack actually
 learns all the things it needs to know about the
 atomic push option.

 We can configure the remote if it wants to advertise
 atomicity!

All previous notes were lost due to my glorious
capability to operate git rebase.

 Documentation/technical/protocol-capabilities.txt | 13 +++--
 builtin/receive-pack.c| 11 +++
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/protocol-capabilities.txt 
b/Documentation/technical/protocol-capabilities.txt
index 6d5424c..4f8a7bf 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,8 +18,9 @@ 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', 'quiet', and 'push-cert' capabilities
-are sent and recognized by the receive-pack (push to server) process.
+The 'atomic', '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
@@ -244,6 +245,14 @@ respond with the 'quiet' capability to suppress 
server-side progress
 reporting if the local progress reporting is also being suppressed
 (e.g., via `push -q`, or if stderr does not go to a tty).
 
+atomic
+--
+
+If the server sends the 'atomic' capability it is capable of accepting
+atomic pushes. If the pushing client requests this capability, the server
+will update the refs in one atomic transaction. Either all refs are
+updated or none.
+
 allow-tip-sha1-in-want
 --
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 362d33f..4c069c5 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -37,6 +37,7 @@ static int receive_fsck_objects = -1;
 static int transfer_fsck_objects = -1;
 static int receive_unpack_limit = -1;
 static int transfer_unpack_limit = -1;
+static int advertise_atomic_push = 1;
 static int unpack_limit = 100;
 static int report_status;
 static int use_sideband;
@@ -159,6 +160,11 @@ static int receive_pack_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+   if (strcmp(var, receive.advertiseatomic) == 0) {
+   advertise_atomic_push = git_config_bool(var, value);
+   return 0;
+   }
+
return git_default_config(var, value, cb);
 }
 
@@ -174,6 +180,8 @@ static void show_ref(const char *path, const unsigned char 
*sha1)
 
strbuf_addstr(cap,
  report-status delete-refs side-band-64k quiet);
+   if (advertise_atomic_push)
+   strbuf_addstr(cap,  atomic);
if (prefer_ofs_delta)
strbuf_addstr(cap,  ofs-delta);
if (push_cert_nonce)
@@ -1263,6 +1271,9 @@ 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 (advertise_atomic_push
+parse_feature_request(feature_list, atomic))
+   use_atomic = 1;
}
 
if (!strcmp(line, push-cert)) {
-- 
2.2.1.62.g3f15098

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

[PATCHv12 09/10] push.c: add an --atomic argument

[Stefan Beller]

From: Ronnie Sahlberg sahlb...@google.com

Add a command line argument to the git push command to request atomic
pushes.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v8, v9:
no changes

v7:
Use OPT_BOOL instead of OPT_BIT.
This allows for --no-atomic option on the command line.
v6:
-   OPT_BIT(0, atomic, flags, N_(use an atomic transaction 
remote),
+   OPT_BIT(0, atomic, flags, N_(request atomic transaction on 
remote side),

skipped v4 v5

v2-v3:
* s/atomic-push/atomic/
* s/the an/an/
* no serverside, but just remote instead
* TRANSPORT_PUSH_ATOMIC instead of TRANSPORT_ATOMIC_PUSH

Changes v1 - v2
It's --atomic now! (dropping the -push)

 Documentation/git-push.txt | 7 ++-
 builtin/push.c | 5 +
 transport.c| 1 +
 transport.h| 1 +
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..4764fcf 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] [-n | 
--dry-run] [--receive-pack=git-receive-pack]
   [--repo=repository] [-f | --force] [--prune] [-v | --verbose]
   [-u | --set-upstream] [--signed]
   [--force-with-lease[=refname[:expect]]]
@@ -136,6 +136,11 @@ already exists on the remote side.
logged.  See linkgit:git-receive-pack[1] for the details
on the receiving end.
 
+--[no-]atomic::
+   Use an atomic transaction on the remote side if available.
+   Either all refs are updated, or on error, no refs are updated.
+   If the server does not support atomic pushes the push will fail.
+
 --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 a076b19..8f1d945 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -487,6 +487,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
int flags = 0;
int tags = 0;
int rc;
+   int atomic = 0;
const char *repo = NULL;/* default repository */
struct option options[] = {
OPT__VERBOSITY(verbosity),
@@ -518,6 +519,7 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
OPT_BIT(0, follow-tags, flags, N_(push missing but relevant 
tags),
TRANSPORT_PUSH_FOLLOW_TAGS),
OPT_BIT(0, signed, flags, N_(GPG sign the push), 
TRANSPORT_PUSH_CERT),
+   OPT_BOOL(0, atomic, atomic, N_(request atomic transaction 
on remote side)),
OPT_END()
};
 
@@ -533,6 +535,9 @@ int cmd_push(int argc, const char **argv, const char 
*prefix)
if (tags)
add_refspec(refs/tags/*);
 
+   if (atomic)
+   flags |= TRANSPORT_PUSH_ATOMIC;
+
if (argc  0) {
repo = argv[0];
set_refspecs(argv + 1, argc - 1, repo);
diff --git a/transport.c b/transport.c
index c67feee..1373152 100644
--- a/transport.c
+++ b/transport.c
@@ -830,6 +830,7 @@ static int git_transport_push(struct transport *transport, 
struct ref *remote_re
args.dry_run = !!(flags  TRANSPORT_PUSH_DRY_RUN);
args.porcelain = !!(flags  TRANSPORT_PUSH_PORCELAIN);
args.push_cert = !!(flags  TRANSPORT_PUSH_CERT);
+   args.atomic = !!(flags  TRANSPORT_PUSH_ATOMIC);
args.url = transport-url;
 
ret = send_pack(args, data-fd, data-conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..18d2cf8 100644
--- a/transport.h
+++ b/transport.h
@@ -125,6 +125,7 @@ struct transport {
 #define TRANSPORT_PUSH_NO_HOOK 512
 #define TRANSPORT_PUSH_FOLLOW_TAGS 1024
 #define TRANSPORT_PUSH_CERT 2048
+#define TRANSPORT_PUSH_ATOMIC 4096
 
 #define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
 #define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) - 
gettext_width(x)), (x)
-- 
2.2.1.62.g3f15098

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

[PATCHv12 02/10] receive-pack.c: die instead of error in case of possible future bug

[Stefan Beller]

Discussion on the previous patch revealed we rather want to err on the
safe side. To do so we need to stop receive-pack in case of the possible
future bug when connectivity is not checked on a shallow push.

Also while touching that code we considered that removing the reported
refs may be harmful in some situations. Sound the message more like a
This Cannot Happen, Please Investigate! instead of giving advice to
remove refs.

Suggested-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v12:
* no advice in case we die.

v11:
* only die at the end so the loop works out for all refs.
* Remove the advice to delete refs.

 If the message says
fatal: BUG: connectivity check skipped???
 then it has exactly the right amount of information to tell me what to
 do.  Now I have
 - a short string to grep for in the source code (or on the web) to
find out what happened

And do a git blame to see previous versions?

I am not so sure of this patch any more as it actually stops people
doing work if they want to do so. (They may deliberately choose to
ignore the BUG:... message, because of a deadline in 2 hours.)

So I do think this helps on getting people to report the bug in the
future if it arises faster, but on the other hand if we assume the
faulty hardware scenario and the deadline we actually stop people
from getting their desired work done.

This patch doesn't actually relate to the topic of the series
(atomic pushes), but is a cleanup-as-we-go patch. If we need to
have further discussion on this, I'd rather want to delay this patch
and have a follow up on top of the atomic series.

Thanks,
Stefan

v10:
* new in v10.

 builtin/receive-pack.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 2ebaf66..3bdb158 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1061,9 +1061,7 @@ static void warn_if_skipped_connectivity_check(struct 
command *commands,
}
}
if (!checked_connectivity)
-   error(BUG: run 'git fsck' for safety.\n
- If there are errors, try to remove 
- the reported refs above);
+   die(BUG: connectivity check skipped???);
 }
 
 static void execute_commands(struct command *commands,
-- 
2.2.1.62.g3f15098

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

[PATCHv12 08/10] send-pack.c: add --atomic command line argument

[Stefan Beller]

From: Ronnie Sahlberg sahlb...@google.com

This adds support to send-pack to negotiate and use atomic pushes
iff the server supports it. Atomic pushes are activated by a new command
line flag --atomic.

In order to do this we also need to change the semantics for send_pack()
slightly. The existing send_pack() function actually doesn't send all the
refs back to the server when multiple refs are involved, for example
when using --all. Several of the failure modes for pushes can already be
detected locally in the send_pack client based on the information from the
initial server side list of all the refs as generated by receive-pack.
Any such refs that we thus know would fail to push are thus pruned from
the list of refs we send to the server to update.

For atomic pushes, we have to deal thus with both failures that are detected
locally as well as failures that are reported back from the server. In order
to do so we treat all local failures as push failures too.

We introduce a new status code REF_STATUS_ATOMIC_PUSH_FAILED so we can
flag all refs that we would normally have tried to push to the server
but we did not due to local failures. This is to improve the error message
back to the end user to flag that these refs failed to update since the
atomic push operation failed.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v9:
 This patch now incorporates parts of the very first patch of
 the previous round. So this patch now implements all changes
 in send-pack apart from the refactoring in the previous patch.

v8:
no changes
v7:
 * return error(...); instead of error(...); return -1;

v6:
* realign to one error string:
+   error(atomic push failed for ref %s. status: %d\n,
+ failing_ref-name, failing_ref-status);

* Use correct value now (negative defined from previous patch)

skipped v4 v5

Changes v2 - v3:
 We avoid assignment inside a conditional.

Ok I switched to using a switch statement.

Changes v1 - v2:
 * Now we only need a very small change in the existing code and have
   a new static function, which cares about error reporting.
  Junio wrote:
   Hmph.  Is atomic push so special that it deserves a separate
   parameter?  When we come up with yet another mode of failure, 
would
   we add another parameter to the callers to this function?
 * error messages are worded differently (lower case!),
 * use of error function instead of fprintf

 * undashed the printed error message (atomic push failed);

 Documentation/git-send-pack.txt |  7 +-
 builtin/send-pack.c |  6 -
 remote.h|  3 ++-
 send-pack.c | 49 +++--
 send-pack.h |  3 ++-
 transport.c |  4 
 6 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-send-pack.txt b/Documentation/git-send-pack.txt
index 2a0de42..45c7725 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] 
[host:]directory [ref...]
 
 DESCRIPTION
 ---
@@ -62,6 +62,11 @@ be in a separate packet, and the list must end with a flush 
packet.
Send a thin pack, which records objects in deltified form based
on objects not included in the pack to reduce network traffic.
 
+--atomic::
+   Use an atomic transaction for updating the refs. If any of the refs
+   fails to update 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 b564a77..b961e5a 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] 
[host:]directory [ref...]\n
   --all and explicit ref specification are mutually exclusive.;
 
 static struct send_pack_args args;
@@ -170,6 +170,10 @@ int cmd_send_pack(int argc, const char **argv, const char 
*prefix)

[PATCHv12 01/10] receive-pack.c: shorten the execute_commands loop over all commands

[Stefan Beller]

Make the main execute_commands loop in receive-pack easier to read
by splitting out some steps into helper functions. The new helper
'should_process_cmd' checks if a ref update is unnecessary, whether
due to an error having occurred or for another reason. The helper
'warn_if_skipped_connectivity_check' warns if we have forgotten to
run a connectivity check on a ref which is shallow for the client
which would be a bug.

This will help us to duplicate less code in a later patch when we make
a second copy of the execute_commands loop.

No functional change intended.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v12:
* no changes
v11:
* rename assure_connectivity_checked to warn_if_skipped_connectivity_check

 This is why I choose the word assure.
If this patch depends on the next one, would it make sense to put them
in the opposite order?

With the next patch applied which dies if the assumption doesn't hold,
assure/assume sounds to me as if it describes the siuation well.

 My personal preference would be to refactor the preceding code to make
 the check unnecessary.

The way I understand it, the shallow case is spread out all over the place
not by choise but because it doesn't work better. So the original author
(Duy) was wise enough to put checks in place because knowing it is fragile
and may break in the future?
This series doesn't intend to refactor the shallow case.

So I picked up warn_if_skipped_connectivity_check as I'm ok with that.

v10:
* rename check_shallow_bugs to assure_connectivity_checked.
* reword commit message.

v9:
* simplified should_process_cmd to be a one liner
* check_shallow_bugs doesn't check of shallow_update being set, rather
  the function is just called if that option is set.

v8: no change

v7:
 new in v7 as in v7 I'd split up the previous
 [PATCH 4/7] receive-pack.c: receive-pack.c: use a single ref_transaction 
for atomic pushes
 as suggested by Eric.

 This is pretty much
 patch 1: Factor out code into helper functions which will be needed by
 the upcoming atomic and non-atomic worker functions. Example helpers:
 'cmd-error_string' and cmd-skip_update' check; and the
 'si-shallow_ref[cmd-index]' check and handling.

 builtin/receive-pack.c | 43 +++
 1 file changed, 27 insertions(+), 16 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 32fc540..2ebaf66 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1042,11 +1042,34 @@ static void reject_updates_to_hidden(struct command 
*commands)
}
 }
 
+static int should_process_cmd(struct command *cmd)
+{
+   return !cmd-error_string  !cmd-skip_update;
+}
+
+static void warn_if_skipped_connectivity_check(struct command *commands,
+  struct shallow_info *si)
+{
+   struct command *cmd;
+   int checked_connectivity = 1;
+
+   for (cmd = commands; cmd; cmd = cmd-next) {
+   if (should_process_cmd(cmd)  si-shallow_ref[cmd-index]) {
+   error(BUG: connectivity check has not been run on ref 
%s,
+ cmd-ref_name);
+   checked_connectivity = 0;
+   }
+   }
+   if (!checked_connectivity)
+   error(BUG: run 'git fsck' for safety.\n
+ If there are errors, try to remove 
+ the reported refs above);
+}
+
 static void execute_commands(struct command *commands,
 const char *unpacker_error,
 struct shallow_info *si)
 {
-   int checked_connectivity;
struct command *cmd;
unsigned char sha1[20];
struct iterate_data data;
@@ -1077,27 +1100,15 @@ static void execute_commands(struct command *commands,
free(head_name_to_free);
head_name = head_name_to_free = resolve_refdup(HEAD, 0, sha1, NULL);
 
-   checked_connectivity = 1;
for (cmd = commands; cmd; cmd = cmd-next) {
-   if (cmd-error_string)
-   continue;
-
-   if (cmd-skip_update)
+   if (!should_process_cmd(cmd))
continue;
 
cmd-error_string = update(cmd, si);
-   if (shallow_update  !cmd-error_string 
-   si-shallow_ref[cmd-index]) {
-   error(BUG: connectivity check has not been run on ref 
%s,
- cmd-ref_name);
-   checked_connectivity = 0;
-   }
}
 
-   if (shallow_update  !checked_connectivity)
-   error(BUG: run 'git fsck' for safety.\n
- If there are errors, try to remove 
- the reported refs above);
+   if (shallow_update)
+ 

[PATCHv12 04/10] receive-pack.c: move transaction handling in a central place

[Stefan Beller]

This moves all code related to transactions into the
execute_commands_non_atomic function. This includes
beginning and committing the transaction as well as
dealing with the errors which may occur during the
begin and commit phase of a transaction.

No functional changes intended.

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
 no changes

v9:
 This was split up into the patch before and this one.
 This patch only deals with the transactions now.

v8:
 move execute_commands_loop before execute_commands, so it compiles/links
 without warnings.

v7:
 new in v7, this is part of the previous [PATCH 4/7]
 receive-pack.c: receive-pack.c: use a single ref_transaction for atomic 
pushes

This covers the suggestion of patch 2 and 3 by Eric
 patch 2: Factor out the main 'for' loop of execute_commands() into a
 new function. This new function will eventually become
 execute_commands_non_atomic(). At this point, execute_commands() is
 pretty much in its final form with the exception of the upcoming 'if
 (use_atomic)' conditional.
 patch 3: Morph the function extracted in patch 2 into
 execute_commands_non_atomic() by adding transaction handling inside
 the 'for' loop (and applying the changes from the early part of the
 patch which go along with that).

 builtin/receive-pack.c | 51 --
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0ccfb3d..96e56a7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -66,6 +66,7 @@ static const char *NONCE_SLOP = SLOP;
 static const char *nonce_status;
 static long nonce_stamp_slop;
 static unsigned long nonce_stamp_slop_limit;
+static struct ref_transaction *transaction;
 
 static enum deny_action parse_deny_action(const char *var, const char *value)
 {
@@ -821,6 +822,7 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
}
 
if (is_null_sha1(new_sha1)) {
+   struct strbuf err = STRBUF_INIT;
if (!parse_object(old_sha1)) {
old_sha1 = NULL;
if (ref_exists(name)) {
@@ -830,35 +832,36 @@ static const char *update(struct command *cmd, struct 
shallow_info *si)
cmd-did_not_exist = 1;
}
}
-   if (delete_ref(namespaced_name, old_sha1, 0)) {
-   rp_error(failed to delete %s, name);
+   if (ref_transaction_delete(transaction,
+  namespaced_name,
+  old_sha1,
+  0, old_sha1 != NULL,
+  push, err)) {
+   rp_error(%s, err.buf);
+   strbuf_release(err);
return failed to delete;
}
+   strbuf_release(err);
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 shallow error;
 
-   transaction = ref_transaction_begin(err);
-   if (!transaction ||
-   ref_transaction_update(transaction, namespaced_name,
-  new_sha1, old_sha1, 0, 1, push,
-  err) ||
-   ref_transaction_commit(transaction, err)) {
-   ref_transaction_free(transaction);
-
+   if (ref_transaction_update(transaction,
+  namespaced_name,
+  new_sha1, old_sha1,
+  0, 1, push,
+  err)) {
rp_error(%s, err.buf);
strbuf_release(err);
+
return failed to update ref;
}
-
-   ref_transaction_free(transaction);
strbuf_release(err);
+
return NULL; /* good */
}
 }
@@ -1068,12 +1071,32 @@ static void execute_commands_non_atomic(struct command 
*commands,
struct shallow_info *si)
 {
struct command *cmd;
+   struct strbuf err = STRBUF_INIT;
+
for (cmd = commands; cmd; cmd = cmd-next) {
if (!should_process_cmd(cmd))
continue;
 
+   transaction = ref_transaction_begin(err);
+   if (!transaction) {
+   rp_error(%s, err.buf);

[PATCHv12 10/10] t5543-atomic-push.sh: add basic tests for atomic pushes

[Stefan Beller]

This adds tests for the atomic push option.
The first four tests check if the atomic option works in
good conditions and the last three patches check if the atomic
option prevents any change to be pushed if just one ref cannot
be updated.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v9:
 add test for 'atomic push is not advertised if configured'
 at the end.

v7, v8: no changes

v6:
the same as v2, so I can resend the whole series as v6

v3 v4 v5 were skipped

Changes v1 - v2:
 Please drop unused comments; they are distracting.

ok

 It is not wrong per-se, but haven't you already tested in
 combination with --mirror in the previous test?

I fixed the previous tests, so that there is no --mirror
and --atomic together. There is still a first --mirror push
for setup and a second with --atomic branchnames though

 check_branches upstream master HEAD@{2} second HEAD~

A similar function test_ref_upstream is introduced.

 What's the value of this test?  Isn't it a non-fast-forward check
 you already tested in the previous one?

I messed up there. Originally I wanted to test the 2 different
stages of rejection. A non-fast-forward check is done locally and
we don't even try pushing. But I also want to test if we locally
thing all is good, but the server refuses a ref to update.
This is now done with the last test named 'atomic push obeys
update hook preventing a branch to be pushed'. And that still fails.

I'll investigate that, while still sending out the series for another
review though.

* Redone the test helper, there is test_ref_upstream now.
  This tests explicitely for SHA1 values of the ref.
  (It's needed in the last test for example. The git push fails,
  but still modifies the ref :/ )
* checked all  chains and repaired them
* sometimes make use of git -C workdir

Notes v1:
Originally Ronnie had a similar patch prepared. But as I added
more tests and cleaned up the existing tests (e.g. use test_commit
instead of echo one file  gitadd file  git commit -a -m 'one',
removal of dead code), the file has changed so much that I'd rather
take ownership.

 t/t5543-atomic-push.sh | 194 +
 1 file changed, 194 insertions(+)
 create mode 100755 t/t5543-atomic-push.sh

diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
new file mode 100755
index 000..3480b33
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,194 @@
+#!/bin/sh
+
+test_description='pushing to a repository using the atomic push option'
+
+. ./test-lib.sh
+
+mk_repo_pair () {
+   rm -rf workbench upstream 
+   test_create_repo upstream 
+   test_create_repo workbench 
+   (
+   cd upstream 
+   git config receive.denyCurrentBranch warn
+   ) 
+   (
+   cd workbench 
+   git remote add up ../upstream
+   )
+}
+
+# Compare the ref ($1) in upstream with a ref value from workbench ($2)
+# i.e. test_refs second HEAD@{2}
+test_refs () {
+   test $# = 2 
+   git -C upstream rev-parse --verify $1 expect 
+   git -C workbench rev-parse --verify $2 actual 
+   test_cmp expect actual
+}
+
+test_expect_success 'atomic push works for a single branch' '
+   mk_repo_pair 
+   (
+   cd workbench 
+   test_commit one 
+   git push --mirror up 
+   test_commit two 
+   git push --atomic up master
+   ) 
+   test_refs master master
+'
+
+test_expect_success 'atomic push works for two branches' '
+   mk_repo_pair 
+   (
+   cd workbench 
+   test_commit one 
+   git branch second 
+   git push --mirror up 
+   test_commit two 
+   git checkout second 
+   test_commit three 
+   git push --atomic up master second
+   ) 
+   test_refs master master 
+   test_refs second second
+'
+
+test_expect_success 'atomic push works in combination with --mirror' '
+   mk_repo_pair 
+   (
+   cd workbench 
+   test_commit one 
+   git checkout -b second 
+   test_commit two 
+   git push --atomic --mirror up
+   ) 
+   test_refs master master 
+   test_refs second second
+'
+
+test_expect_success 'atomic push works in combination with --force' '
+   mk_repo_pair 
+   (
+   cd workbench 
+   test_commit one 
+   git branch second master 
+   test_commit two_a 
+   git checkout second 
+   test_commit two_b 
+   test_commit three_b 
+   test_commit four 
+   git push --mirror up 
+  

Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Eric Sunshine]

On Wed, Jan 7, 2015 at 5:19 PM, Reuben Hawkins reuben...@gmail.com wrote:
 On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 This check will automatically set the correct NO_NSEC setting.

 This commit message neglects to mention the important point that
 you're also now setting USE_ST_TIMESPEC when detected. You might
 revise the message like this:

 Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
 NO_NSEC appropriately.

 A side-effect of the above detection is that we also determine
 whether 'stat.st_mtimespec' is available, so, as a bonus, set the
 Makefile variable USE_ST_TIMESPEC, as well.

 I see you're single quoted 'tv_nsec' and 'struct stat'.  Should I also
 use single quotes in the first line of the commit msg like this...

 configure.ac: check 'tv_nsec' field in 'struct stat'

Quoting them was just my personal taste, however, consistency of
formatting between subject and the body of the message would be nice.
Use whatever seems correct to you.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Eric Sunshine]

On Wed, Jan 7, 2015 at 5:31 PM, Reuben Hawkins reuben...@gmail.com wrote:
 On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 +GIT_CHECK_FUNC(clock_gettime,
 +[HAVE_CLOCK_GETTIME=YesPlease],
 +[HAVE_CLOCK_GETTIME=])
 +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])

 You could simplify the above four lines to this one-liner:

 GIT_CHECK_FUNC(clock_gettime,
 GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))

 +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
 +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
 +   [AC_MSG_RESULT([yes])
 +   HAVE_CLOCK_MONOTONIC=YesPlease],
 +   [AC_MSG_RESULT([no])
 +   HAVE_CLOCK_MONOTONIC=])
 +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])

 Ditto regarding simplification:

 AC_MSG_CHECKING([for CLOCK_MONOTONIC])
 AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
 [AC_MSG_RESULT([yes])
 GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
 [AC_MSG_RESULT([no])])

 I *think* there's an issue with this simplification as used right
 here.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by
 setting it equal to nothing

 HAVE_CLOCK_MONOTONIC=

 So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =
 YesPlease' will be overridden.

 So this one needs to stay as is.

Yes, you're right. That means that the HAVE_CLOCK_GETTIME
simplification also suffers the same shortcoming. So, neither
simplification is appropriate in this instance. Sorry for the noise.
--
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] standardize usage info string format

[Alex Henrie]

This patch puts the usage info strings that were not already in docopt-
like format into docopt-like format, which will be a litle easier for
end users and a lot easier for translators. Changes include:

- Placing angle brackets around fill-in-the-blank parameters
- Putting dashes in multiword parameter names
- Adding spaces to [-f|--foobar] to make [-f | --foobar]
- Replacing foobar* with [foobar...]

Signed-off-by: Alex Henrie alexhenri...@gmail.com
---
 advice.c   |  2 +-
 archive.c  |  4 ++--
 builtin/add.c  |  2 +-
 builtin/apply.c|  2 +-
 builtin/blame.c|  4 ++--
 builtin/branch.c   |  8 
 builtin/cat-file.c |  4 ++--
 builtin/check-attr.c   |  4 ++--
 builtin/check-ignore.c |  4 ++--
 builtin/check-mailmap.c|  2 +-
 builtin/check-ref-format.c |  2 +-
 builtin/checkout-index.c   |  2 +-
 builtin/checkout.c |  8 
 builtin/clone.c|  2 +-
 builtin/column.c   |  2 +-
 builtin/commit.c   |  4 ++--
 builtin/config.c   |  2 +-
 builtin/describe.c |  4 ++--
 builtin/diff-files.c   |  2 +-
 builtin/diff-index.c   |  2 +-
 builtin/diff-tree.c|  2 +-
 builtin/fetch-pack.c   |  2 +-
 builtin/fmt-merge-msg.c|  2 +-
 builtin/for-each-ref.c |  2 +-
 builtin/fsck.c |  2 +-
 builtin/gc.c   |  2 +-
 builtin/grep.c |  2 +-
 builtin/hash-object.c  |  2 +-
 builtin/help.c |  2 +-
 builtin/init-db.c  |  2 +-
 builtin/log.c  |  6 +++---
 builtin/ls-files.c |  2 +-
 builtin/ls-remote.c|  2 +-
 builtin/mailinfo.c |  2 +-
 builtin/merge-base.c   |  4 ++--
 builtin/merge-file.c   |  4 ++--
 builtin/merge-index.c  |  2 +-
 builtin/merge.c|  4 ++--
 builtin/mv.c   |  2 +-
 builtin/name-rev.c |  6 +++---
 builtin/notes.c| 24 
 builtin/pack-redundant.c   |  2 +-
 builtin/pack-refs.c|  2 +-
 builtin/prune-packed.c |  2 +-
 builtin/remote.c   |  4 ++--
 builtin/repack.c   |  2 +-
 builtin/rerere.c   |  2 +-
 builtin/rev-parse.c|  6 +++---
 builtin/revert.c   |  4 ++--
 builtin/rm.c   |  2 +-
 builtin/shortlog.c |  2 +-
 builtin/show-branch.c  |  4 ++--
 builtin/show-ref.c |  2 +-
 builtin/symbolic-ref.c |  4 ++--
 builtin/tag.c  |  4 ++--
 builtin/update-index.c |  2 +-
 builtin/update-ref.c   |  6 +++---
 builtin/verify-commit.c|  2 +-
 builtin/verify-pack.c  |  2 +-
 builtin/verify-tag.c   |  2 +-
 credential-store.c |  2 +-
 git-bisect.sh  |  2 +-
 git.c  |  2 +-
 63 files changed, 103 insertions(+), 103 deletions(-)

diff --git a/advice.c b/advice.c
index 3b8bf3c..575bec2 100644
--- a/advice.c
+++ b/advice.c
@@ -105,7 +105,7 @@ void detach_advice(const char *new_name)
state without impacting any branches by performing another 
checkout.\n\n
If you want to create a new branch to retain commits you create, you 
may\n
do so (now or later) by using -b with the checkout command again. 
Example:\n\n
- git checkout -b new_branch_name\n\n;
+ git checkout -b new-branch-name\n\n;
 
fprintf(stderr, fmt, new_name);
 }
diff --git a/archive.c b/archive.c
index 9e30246..96057ed 100644
--- a/archive.c
+++ b/archive.c
@@ -8,9 +8,9 @@
 #include dir.h
 
 static char const * const archive_usage[] = {
-   N_(git archive [options] tree-ish [path...]),
+   N_(git archive [options] tree-ish [path...]),
N_(git archive --list),
-   N_(git archive --remote repo [--exec cmd] [options] tree-ish 
[path...]),
+   N_(git archive --remote repo [--exec cmd] [options] tree-ish 
[path...]),
N_(git archive --remote repo [--exec cmd] --list),
NULL
 };
diff --git a/builtin/add.c b/builtin/add.c
index 1074e32..3390933 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -19,7 +19,7 @@
 #include argv-array.h
 
 static const char * const builtin_add_usage[] = {
-   N_(git add [options] [--] pathspec...),
+   N_(git add [options] [--] pathspec...),
NULL
 };
 static int patch_interactive, add_interactive, edit_interactive;
diff --git a/builtin/apply.c b/builtin/apply.c
index 0aad912..7cd9a3b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -55,7 +55,7 @@ static const char *fake_ancestor;
 static int line_termination = '\n';
 static unsigned int p_context = UINT_MAX;
 static const char * const apply_usage[] = {
-   N_(git apply [options] [patch...]),
+   N_(git apply [options] [patch...]),
NULL
 };
 
diff --git a/builtin/blame.c b/builtin/blame.c
index 303e217..f0fac65 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -27,12 +27,12 @@
 #include line-range.h
 #include line-log.h
 
-static char blame_usage[] = N_(git blame [options] 

[PATCHv12 03/10] receive-pack.c: move iterating over all commands outside execute_commands

[Stefan Beller]

This commit allows us in a later patch to easily distinguish between
the non atomic way to update the received refs and the atomic way which
is introduced in a later patch.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
no changes

v9:
 new and shiny. But makes the next patch easier to review.

 builtin/receive-pack.c | 19 +--
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 3bdb158..0ccfb3d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1064,6 +1064,18 @@ static void warn_if_skipped_connectivity_check(struct 
command *commands,
die(BUG: connectivity check skipped???);
 }
 
+static void execute_commands_non_atomic(struct command *commands,
+   struct shallow_info *si)
+{
+   struct command *cmd;
+   for (cmd = commands; cmd; cmd = cmd-next) {
+   if (!should_process_cmd(cmd))
+   continue;
+
+   cmd-error_string = update(cmd, si);
+   }
+}
+
 static void execute_commands(struct command *commands,
 const char *unpacker_error,
 struct shallow_info *si)
@@ -1098,12 +1110,7 @@ static void execute_commands(struct command *commands,
free(head_name_to_free);
head_name = head_name_to_free = resolve_refdup(HEAD, 0, sha1, NULL);
 
-   for (cmd = commands; cmd; cmd = cmd-next) {
-   if (!should_process_cmd(cmd))
-   continue;
-
-   cmd-error_string = update(cmd, si);
-   }
+   execute_commands_non_atomic(commands, si);
 
if (shallow_update)
warn_if_skipped_connectivity_check(commands, si);
-- 
2.2.1.62.g3f15098

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

[PATCHv12 07/10] send-pack: rename ref_update_to_be_sent to check_to_send_update

[Stefan Beller]

This renames ref_update_to_be_sent to check_to_send_update and inverts
the meaning of the return value. Having the return value inverted we
can have different values for the error codes. This is useful in a
later patch when we want to know if we hit the CHECK_REF_STATUS_REJECTED
case.

Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v7, v8, v9:
* no changes

v6:
* negative #define'd values

skipped v4 v5

This was introduced with the [PATCHv2] series.
Changes v2 - v3:

* Rename to check_to_send_update
* Negative error values.
* errors values are #define'd and not raw numbers.

 send-pack.c | 18 +++---
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/send-pack.c b/send-pack.c
index 949cb61..4974825 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,10 +190,13 @@ 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)
+#define CHECK_REF_NO_PUSH -1
+#define CHECK_REF_STATUS_REJECTED -2
+#define CHECK_REF_UPTODATE -3
+static int check_to_send_update(const struct ref *ref, const struct 
send_pack_args *args)
 {
if (!ref-peer_ref  !args-send_mirror)
-   return 0;
+   return CHECK_REF_NO_PUSH;
 
/* Check for statuses set by set_ref_status_for_push() */
switch (ref-status) {
@@ -203,10 +206,11 @@ static int ref_update_to_be_sent(const struct ref *ref, 
const struct send_pack_a
case REF_STATUS_REJECT_NEEDS_FORCE:
case REF_STATUS_REJECT_STALE:
case REF_STATUS_REJECT_NODELETE:
+   return CHECK_REF_STATUS_REJECTED;
case REF_STATUS_UPTODATE:
-   return 0;
+   return CHECK_REF_UPTODATE;
default:
-   return 1;
+   return 0;
}
 }
 
@@ -250,7 +254,7 @@ static int generate_push_cert(struct strbuf *req_buf,
strbuf_addstr(cert, \n);
 
for (ref = remote_refs; ref; ref = ref-next) {
-   if (!ref_update_to_be_sent(ref, args))
+   if (check_to_send_update(ref, args)  0)
continue;
update_seen = 1;
strbuf_addf(cert, %s %s %s\n,
@@ -359,7 +363,7 @@ int send_pack(struct send_pack_args *args,
 * the pack data.
 */
for (ref = remote_refs; ref; ref = ref-next) {
-   if (!ref_update_to_be_sent(ref, args))
+   if (check_to_send_update(ref, args)  0)
continue;
 
if (!ref-deletion)
@@ -380,7 +384,7 @@ int send_pack(struct send_pack_args *args,
if (args-dry_run || args-push_cert)
continue;
 
-   if (!ref_update_to_be_sent(ref, args))
+   if (check_to_send_update(ref, args)  0)
continue;
 
old_hex = sha1_to_hex(ref-old_sha1);
-- 
2.2.1.62.g3f15098

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

[PATCHv12 05/10] receive-pack.c: add execute_commands_atomic function

[Stefan Beller]

This introduces the new function execute_commands_atomic which will use
one atomic transaction for all updates. The default behavior is still
the old non atomic way, one ref at a time. 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 behavior so we make it
opt-in for now.

A later patch will add the possibility to actually use the functionality
added by this patch. For now use_atomic is always 0.

Inspired-by: Ronnie Sahlberg sahlb...@google.com
Helped-by: Eric Sunshine sunsh...@sunshineco.com
Signed-off-by: Stefan Beller sbel...@google.com
---

Notes:
v10, v11, v12:
* no changes

v9:
Because the patches have been reordered, we introduce use_atomic
in this patch, but don't touch it.

 Minor comment: This cleanup code is repeated in both the success and
 fail branches. It _might_ (or not) be a bit cleaner and more
 maintainable to replace the above three lines with:

Personally I have no strong opinion about that one.
I implemented it as you suggested, having a cleanup and a failure
label. It seems to look ok and has less lines of code.

However writing this code made me feel a bit like producing
spagetti code here. (Goto is evil!, I accepted goto as a
convenient way to have one common cleanup or failure before
exiting the function before, but now we have two jump targets.)

Changes in v8:
removed superflous } to make it compile again

Changes in v7:
Eric suggested to replace [PATCH 4/7] receive-pack.c:
receive-pack.c: use a single ref_transaction for atomic pushes
by smaller patches
This is the last patch replacing said large commit.

Changes in v6:
This is a complete rewrite of the patch essentially.
Eric suggested to split up the flow into functions, so
it is easier to read. It's more lines of code, but indeed
it's easier to follow. Thanks Eric!

Note there is another patch following this one
moving the helper functions above execute_commands.
I just choose the order of the functions in this patch
to have a better diff (just inserting the call to the function
execute_commands_non_atomic and that function directly follows.)
The next patch of the series will move that up.

Because of the rewrite and the fixes of the previous five
versions there is not much left of Ronnies original patch,
so I'll claim authorship of this one.

Changes v1 - v2:
* update(...) assumes to be always in a transaction
* Caring about when to begin/commit transactions is put
  into execute_commands
v2-v3:
* meditated about the error flow. Now we always construct a local
  strbuf err if required. Then the flow is easier to follow and
  destruction of it is performed nearby.
* early return in execute_commands if transaction_begin fails.

v3-v4:
* revamp logic again. This should keep the non atomic behavior
  as is (in case of error say so, in non error case just free the
  transaction). In the atomic case we either do nothing (when no error),
  or abort with the goto.

if (!cmd-error_string) {
if (!use_atomic
 ref_transaction_commit(transaction, err)) {
ref_transaction_free(transaction);
rp_error(%s, err.buf);
strbuf_release(err);
cmd-error_string = failed to update ref;
}
} else if (use_atomic) {
goto atomic_failure;
} else {
ref_transaction_free(transaction);
}

 * Having the goto directly there when checking for cmd-error_string,
   we don't need to do it again, so the paragraph explaining the error
   checking is gone as well. (Previous patch had the following, this is
   put at the end of the function, where the goto jumps to and the 
comment
   has been dropped.
+   /*
+* update(...) may abort early (i.e. because the hook refused to
+* update that ref) which then doesn't even record a transaction
+* regarding that ref. Make sure all commands are without error
+* and then commit atomically.
+*/
+   for (cmd = commands; cmd; cmd = cmd-next)
+   if (cmd-error_string)
+   break;

v4-v5:
Eric wrote:
 Repeating from my earlier review[1]: If the 'pre-receive' hook
 declines, then this transaction is left dangling (and its 

Probably a bug with ~ symbol in filenames on Windows 7 x64 in git 1.9.5

[Dmitry Bykov]

Hello,

Recently I installed 1.9.5 git version and faced the problem that one
of the files in my cloned repository with a name ICON~714.PNG is
marked as deleted even repository was freshly cloned. Trying to do
anything with that file resulted in constant Invalid Path errors.
Reverting back to 1.9.4. fixed that problem.

Thanks,
Dmitry
--
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] remote-curl: fall back to Basic auth if Negotiate fails

[brian m. carlson]

On Tue, Jan 06, 2015 at 04:07:01PM +, Dan Langille (dalangil) wrote:

 HTTP/1.1 401 Authorization Required
 Date: Tue, 06 Jan 2015 16:02:48 GMT
 Server: Apache
 WWW-Authenticate: Negotiate


Your server is set up incorrectly.  You should see a Negotiate line and 
a Basic line as well.  Right now, you only have Negotiate set up, so if 
you don't have a ticket, it's going to fail.


I'd recommend making sure that you can access it using a web browser 
after running kdestroy.  That will ensure that it's working properly.

--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: Probably a bug with ~ symbol in filenames on Windows 7 x64 in git 1.9.5

[Stefan Beller]

On Wed, Jan 7, 2015 at 3:26 PM, Dmitry Bykov pvr...@gmail.com wrote:
 Hello,

 Recently I installed 1.9.5 git version and faced the problem that one
 of the files in my cloned repository with a name ICON~714.PNG is
 marked as deleted even repository was freshly cloned. Trying to do
 anything with that file resulted in constant Invalid Path errors.
 Reverting back to 1.9.4. fixed that problem.

 Thanks,
 Dmitry

Git had a security issue with filenames which look similar to the .git
repository.
Please see the announcement at
http://article.gmane.org/gmane.linux.kernel/1853266
(That also updated 1.9.4 - 1.9.5)

I'm not sure if I can advice though.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Probably a bug with ~ symbol in filenames on Windows 7 x64 in git 1.9.5

[Junio C Hamano]

Dscho, this sounds to me like the additional 8.3 ambiguity
protection (which is only in Git for Windows) in action. Any
thoughts?

On Wed, Jan 7, 2015 at 3:26 PM, Dmitry Bykov pvr...@gmail.com wrote:
 Hello,

 Recently I installed 1.9.5 git version and faced the problem that one
 of the files in my cloned repository with a name ICON~714.PNG is
 marked as deleted even repository was freshly cloned. Trying to do
 anything with that file resulted in constant Invalid Path errors.
 Reverting back to 1.9.4. fixed that problem.

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

[PATCH v3] remote-curl: fall back to Basic auth if Negotiate fails

[brian m. carlson]

Apache servers using mod_auth_kerb can be configured to allow the user
to authenticate either using Negotiate (using the Kerberos ticket) or
Basic authentication (using the Kerberos password).  Often, one will
want to use Negotiate authentication if it is available, but fall back
to Basic authentication if the ticket is missing or expired.

However, libcurl will try very hard to use something other than Basic
auth, even over HTTPS.  If Basic and something else are offered, libcurl
will never attempt to use Basic, even if the other option fails.
Teach the HTTP client code to stop trying authentication mechanisms that
don't use a password (currently Negotiate) after the first failure,
since if they failed the first time, they will never succeed.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
Signed-off-by: Jeff King p...@peff.net
---
Peff's original change was to get_curl_handle; however, we retry the
second time with the same slot and we may not call get_curl_handle
again, so I had to move that change to get_active_slot.  This has been
tested pushing with both Negotiate and Basic against an HTTPS server
both when info/refs was protected and when it was not.

 http.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/http.c b/http.c
index 040f362..44b130c 100644
--- a/http.c
+++ b/http.c
@@ -62,6 +62,9 @@ static const char *user_agent;
 
 static struct credential cert_auth = CREDENTIAL_INIT;
 static int ssl_cert_password_required;
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+static unsigned long http_auth_methods = CURLAUTH_ANY;
+#endif
 
 static struct curl_slist *pragma_header;
 static struct curl_slist *no_pragma_header;
@@ -580,6 +583,9 @@ struct active_request_slot *get_active_slot(void)
curl_easy_setopt(slot-curl, CURLOPT_UPLOAD, 0);
curl_easy_setopt(slot-curl, CURLOPT_HTTPGET, 1);
curl_easy_setopt(slot-curl, CURLOPT_FAILONERROR, 1);
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   curl_easy_setopt(slot-curl, CURLOPT_HTTPAUTH, http_auth_methods);
+#endif
if (http_auth.password)
init_curl_http_auth(slot-curl);
 
@@ -870,6 +876,9 @@ int handle_curl_result(struct slot_results *results)
credential_reject(http_auth);
return HTTP_NOAUTH;
} else {
+#ifdef LIBCURL_CAN_HANDLE_AUTH_ANY
+   http_auth_methods = ~CURLAUTH_GSSNEGOTIATE;
+#endif
return HTTP_REAUTH;
}
} else {
@@ -986,6 +995,7 @@ static void extract_content_type(struct strbuf *raw, struct 
strbuf *type,
strbuf_addstr(charset, ISO-8859-1);
 }
 
+
 /* http_request() targets */
 #define HTTP_REQUEST_STRBUF0
 #define HTTP_REQUEST_FILE  1
-- 
2.2.1.209.g41e5f3a

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

Re: [PATCH v3 1/2] t4255: test am submodule with diff.submodule

[Eric Sunshine]

On Wed, Jan 7, 2015 at 2:31 PM, Doug Kelly dougk@gmail.com wrote:
 git am will break when using diff.submodule=log; add some test cases
 to illustrate this breakage as simply as possible.  There are
 currently two ways this can fail:

 * With errors (unrecognized input), if only change
 * Silently (no submodule change), if other files change

 Test for both conditions and ensure without diff.submodule this works.

 Signed-off-by: Doug Kelly dougk@gmail.com
 Thanks-to: Eric Sunshine sunsh...@sunshineco.com
 Thanks-to: Junio C Hamano gits...@pobox.com

On this project, it's customary to say Helped-by: rather than
Thanks-to:. Also, place your sign-off last.

 ---
 Updated with Eric Sunshine's comments and changes to reduce complexity,
 and also changed to include Junio's suggestions for using test_config,
 test_unconfig, and test_might_fail (since we don't know if a previous
 am failed or not -- we always want to clean up first).

Looking much better. Thanks. A couple minor comments below...

 diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
 index 8bde7db..523accf 100755
 --- a/t/t4255-am-submodule.sh
 +++ b/t/t4255-am-submodule.sh
 @@ -18,4 +18,76 @@ am_3way () {
  KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
  test_submodule_switch am_3way

 +test_expect_success 'setup diff.submodule' '
 +   test_commit one 
 +   INITIAL=$(git rev-parse HEAD) 
 +
 +   git init submodule 
 +   (
 +   cd submodule 
 +   test_commit two 
 +   git rev-parse HEAD ../initial-submodule
 +   ) 
 +   git submodule add ./submodule 
 +   git commit -m first 
 +
 +   (
 +   cd submodule 
 +   test_commit three 
 +   git rev-parse HEAD ../first-submodule
 +   ) 
 +   git add submodule 
 +   test_tick 

You can drop this test_tick (as I did in my squash[1]).

 +   git commit -m second 
 +   SECOND=$(git rev-parse HEAD) 
 +
 +   (
 +   cd submodule 
 +   git mv two.t four.t 
 +   test_tick 

And this one (which I overlooked in [1]).

The reason I suggest dropping the test_tick invocations is that they
do not impact these tests at all, yet their presence misleads the
reader into thinking that they are somehow significant.

 +   git commit -m second submodule 
 +   git rev-parse HEAD ../second-submodule
 +   ) 
 +   test_commit four 
 +   git add submodule 
 +   git commit --amend --no-edit 
 +   THIRD=$(git rev-parse HEAD) 
 +   git submodule update --init
 +'

[1]: http://article.gmane.org/gmane.comp.version-control.git/261852
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] t4255: test am submodule with diff.submodule

[Junio C Hamano]

Doug Kelly dougk@gmail.com writes:

 On Mon, Dec 29, 2014 at 9:42 AM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 +   (git am --abort || true) 

 Why (x || y)?  Is 'x' so unreliable that we do not know how should exit?
 Should this be test_must_fail git am --abort?

 Updated to test_might_fail -- we don't know if a merge is in progress or not.
 We still need to clean up, but disregard failure if a merge isn't in progress.

Ah, OK.  But even with test_might_fail, it may not be clear why it
might fail, so it would be easier to maintain if we can read we
don't know if a merge is in progress next to the test_might_fail.

For now we can add a comment, but in the longer term it might not be
a bad idea to change test_might_fail to require two args, one is a
command to run and the other is a text that explains why the outcome
is unknown.

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

Re: [PATCH 1/2] support for --no-relative and diff.relative

[Junio C Hamano]

kel...@shysecurity.com writes:

 Content-Type: text/plain; charset=utf-8; format=flowed
Please.  No format=flawed.  Really.
 I'll figure out the line-wrapping.

 Also this step is not about --no-relative and diff.relative but is
 only about --no-relative option.
 Should I submit as two independent patches then? I took the approach
 of splitting them out into 1/2 vs 2/2 to distinguish, but it sounds
 like that isn't optimal.

They are indeed better to be 1/2 and 2/2; they do not have to share
the same subject, though.  1/2 now adds only --no-relative and makes
sure an earlier --relative is cancelled without even knowing that
diff.relative might appear in the future (well, you may know that,
but the system with only 1/2 applied without 2/2 would work perfectly
fine).  2/2 adds diff.relative and makes sure --no-relative cancels
its effect as well.

 On review, this may be a bad approach though. Non-locality makes it
 harder to follow/understand and introduces a subtle bug.
 current:  git-diff --relative=path --no-relative --relative ==
 git-diff --relative=path
 expected: git-diff --relative=path --no-relative --relative ==
 git-diff --relative

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

[PATCH v5 1/2] t4255: test am submodule with diff.submodule

[Doug Kelly]

git am will break when using diff.submodule=log; add some test cases
to illustrate this breakage as simply as possible.  There are
currently two ways this can fail:

* With errors (unrecognized input), if only change
* Silently (no submodule change), if other files change

Test for both conditions and ensure without diff.submodule this works.

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Doug Kelly dougk@gmail.com
---
Added a comment for why test_might_fail is used to abort merges
in progress.

 t/t4255-am-submodule.sh | 72 +
 1 file changed, 72 insertions(+)

diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 8bde7db..450d261 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -18,4 +18,76 @@ am_3way () {
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
 test_submodule_switch am_3way
 
+test_expect_success 'setup diff.submodule' '
+   test_commit one 
+   INITIAL=$(git rev-parse HEAD) 
+
+   git init submodule 
+   (
+   cd submodule 
+   test_commit two 
+   git rev-parse HEAD ../initial-submodule
+   ) 
+   git submodule add ./submodule 
+   git commit -m first 
+
+   (
+   cd submodule 
+   test_commit three 
+   git rev-parse HEAD ../first-submodule
+   ) 
+   git add submodule 
+   git commit -m second 
+   SECOND=$(git rev-parse HEAD) 
+
+   (
+   cd submodule 
+   git mv two.t four.t 
+   git commit -m second submodule 
+   git rev-parse HEAD ../second-submodule
+   ) 
+   test_commit four 
+   git add submodule 
+   git commit --amend --no-edit 
+   THIRD=$(git rev-parse HEAD) 
+   git submodule update --init
+'
+
+run_test() {
+   START_COMMIT=$1 
+   EXPECT=$2 
+   # Abort any merges in progress: the previous
+   # test may have failed, and we should clean up.
+   test_might_fail git am --abort 
+   git reset --hard $START_COMMIT 
+   rm -f *.patch 
+   git format-patch -1 
+   git reset --hard $START_COMMIT^ 
+   git submodule update 
+   git am *.patch 
+   git submodule update 
+   git -C submodule rev-parse HEAD actual 
+   test_cmp $EXPECT actual
+}
+
+test_expect_success 'diff.submodule unset' '
+   test_unconfig diff.submodule 
+   run_test $SECOND first-submodule
+'
+
+test_expect_success 'diff.submodule unset with extra file' '
+   test_unconfig diff.submodule 
+   run_test $THIRD second-submodule
+'
+
+test_expect_failure 'diff.submodule=log' '
+   test_config diff.submodule log 
+   run_test $SECOND first-submodule
+'
+
+test_expect_failure 'diff.submodule=log with extra file' '
+   test_config diff.submodule log 
+   run_test $THIRD second-submodule
+'
+
 test_done
-- 
2.0.5

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

[PATCH v5 2/2] format-patch: ignore diff.submodule setting

[Doug Kelly]

diff.submodule when set to log produces output which git-am cannot
handle. Ignore this setting when generating patch output.

Signed-off-by: Doug Kelly dougk@gmail.com
---
 builtin/log.c   | 2 +-
 t/t4255-am-submodule.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 734aab3..cb14db4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (!strcmp(var, diff.color) || !strcmp(var, color.diff) ||
-   !strcmp(var, color.ui)) {
+   !strcmp(var, color.ui) || !strcmp(var, diff.submodule)) {
return 0;
}
if (!strcmp(var, format.numbered)) {
diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 450d261..0ba8194 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -80,12 +80,12 @@ test_expect_success 'diff.submodule unset with extra file' '
run_test $THIRD second-submodule
 '
 
-test_expect_failure 'diff.submodule=log' '
+test_expect_success 'diff.submodule=log' '
test_config diff.submodule log 
run_test $SECOND first-submodule
 '
 
-test_expect_failure 'diff.submodule=log with extra file' '
+test_expect_success 'diff.submodule=log with extra file' '
test_config diff.submodule log 
run_test $THIRD second-submodule
 '
-- 
2.0.5

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

[PATCH v4 1/2] t4255: test am submodule with diff.submodule

[Doug Kelly]

git am will break when using diff.submodule=log; add some test cases
to illustrate this breakage as simply as possible.  There are
currently two ways this can fail:

* With errors (unrecognized input), if only change
* Silently (no submodule change), if other files change

Test for both conditions and ensure without diff.submodule this works.

Helped-by: Eric Sunshine sunsh...@sunshineco.com
Helped-by: Junio C Hamano gits...@pobox.com
Signed-off-by: Doug Kelly dougk@gmail.com
---
Updated to remove test_ticks and clean the commit message.

 t/t4255-am-submodule.sh | 70 +
 1 file changed, 70 insertions(+)

diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 8bde7db..a38c305 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -18,4 +18,74 @@ am_3way () {
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
 test_submodule_switch am_3way
 
+test_expect_success 'setup diff.submodule' '
+   test_commit one 
+   INITIAL=$(git rev-parse HEAD) 
+
+   git init submodule 
+   (
+   cd submodule 
+   test_commit two 
+   git rev-parse HEAD ../initial-submodule
+   ) 
+   git submodule add ./submodule 
+   git commit -m first 
+
+   (
+   cd submodule 
+   test_commit three 
+   git rev-parse HEAD ../first-submodule
+   ) 
+   git add submodule 
+   git commit -m second 
+   SECOND=$(git rev-parse HEAD) 
+
+   (
+   cd submodule 
+   git mv two.t four.t 
+   git commit -m second submodule 
+   git rev-parse HEAD ../second-submodule
+   ) 
+   test_commit four 
+   git add submodule 
+   git commit --amend --no-edit 
+   THIRD=$(git rev-parse HEAD) 
+   git submodule update --init
+'
+
+run_test() {
+   START_COMMIT=$1 
+   EXPECT=$2 
+   test_might_fail git am --abort 
+   git reset --hard $START_COMMIT 
+   rm -f *.patch 
+   git format-patch -1 
+   git reset --hard $START_COMMIT^ 
+   git submodule update 
+   git am *.patch 
+   git submodule update 
+   git -C submodule rev-parse HEAD actual 
+   test_cmp $EXPECT actual
+}
+
+test_expect_success 'diff.submodule unset' '
+   test_unconfig diff.submodule 
+   run_test $SECOND first-submodule
+'
+
+test_expect_success 'diff.submodule unset with extra file' '
+   test_unconfig diff.submodule 
+   run_test $THIRD second-submodule
+'
+
+test_expect_failure 'diff.submodule=log' '
+   test_config diff.submodule log 
+   run_test $SECOND first-submodule
+'
+
+test_expect_failure 'diff.submodule=log with extra file' '
+   test_config diff.submodule log 
+   run_test $THIRD second-submodule
+'
+
 test_done
-- 
2.0.5

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

[PATCH v4 2/2] format-patch: ignore diff.submodule setting

[Doug Kelly]

diff.submodule when set to log produces output which git-am cannot
handle. Ignore this setting when generating patch output.

Signed-off-by: Doug Kelly dougk@gmail.com
---
 builtin/log.c   | 2 +-
 t/t4255-am-submodule.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 734aab3..cb14db4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (!strcmp(var, diff.color) || !strcmp(var, color.diff) ||
-   !strcmp(var, color.ui)) {
+   !strcmp(var, color.ui) || !strcmp(var, diff.submodule)) {
return 0;
}
if (!strcmp(var, format.numbered)) {
diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index a38c305..27ea698 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -78,12 +78,12 @@ test_expect_success 'diff.submodule unset with extra file' '
run_test $THIRD second-submodule
 '
 
-test_expect_failure 'diff.submodule=log' '
+test_expect_success 'diff.submodule=log' '
test_config diff.submodule log 
run_test $SECOND first-submodule
 '
 
-test_expect_failure 'diff.submodule=log with extra file' '
+test_expect_success 'diff.submodule=log with extra file' '
test_config diff.submodule log 
run_test $THIRD second-submodule
 '
-- 
2.0.5

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

v2 patches for fixes on RHEL3

[Reuben Hawkins]

These patches add a few autoconfig checks for nanosecond resolution fields in
struct stat, CLOCK_MONOTONIC, and HMAC_CTX_cleanup.  I'm fairly sure I've fixed
the concerns/doubts the first set of patched raised.

(I rarely use git-send-email so forgive me if I botch this)
--
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/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Reuben Hawkins]

CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
systems being used in production.  This change makes compiling git
less tedious on older platforms without CLOCK_MONOTONIC.
---
 Makefile |  4 
 config.mak.uname |  1 +
 configure.ac | 22 ++
 trace.c  |  2 +-
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 7482a4d..af551a0 100644
--- a/Makefile
+++ b/Makefile
@@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
EXTLIBS += -lrt
 endif
 
+ifdef HAVE_CLOCK_MONOTONIC
+   BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
diff --git a/config.mak.uname b/config.mak.uname
index a2f380f..926773e 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -35,6 +35,7 @@ ifeq ($(uname_S),Linux)
LIBC_CONTAINS_LIBINTL = YesPlease
HAVE_DEV_TTY = YesPlease
HAVE_CLOCK_GETTIME = YesPlease
+   HAVE_CLOCK_MONOTONIC = YesPlease
 endif
 ifeq ($(uname_S),GNU/kFreeBSD)
HAVE_ALLOCA_H = YesPlease
diff --git a/configure.ac b/configure.ac
index dcc4bf0..424dec5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
  [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
 #
+# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
+GIT_CHECK_FUNC(clock_gettime,
+[HAVE_CLOCK_GETTIME=YesPlease],
+[HAVE_CLOCK_GETTIME=])
+GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])
+
+AC_DEFUN([CLOCK_MONOTONIC_SRC], [
+AC_LANG_PROGRAM([[
+#include time.h
+clockid_t id = CLOCK_MONOTONIC;
+]], [])])
+
+#
+# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
+AC_MSG_CHECKING([for CLOCK_MONOTONIC])
+AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
+   [AC_MSG_RESULT([yes])
+   HAVE_CLOCK_MONOTONIC=YesPlease],
+   [AC_MSG_RESULT([no])
+   HAVE_CLOCK_MONOTONIC=])
+GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])
+#
 # Define NO_SETITIMER if you don't have setitimer.
 GIT_CHECK_FUNC(setitimer,
 [NO_SETITIMER=],
diff --git a/trace.c b/trace.c
index 4778608..bfbd48f 100644
--- a/trace.c
+++ b/trace.c
@@ -324,7 +324,7 @@ int trace_want(struct trace_key *key)
return !!get_trace_fd(key);
 }
 
-#ifdef HAVE_CLOCK_GETTIME
+#if defined(HAVE_CLOCK_GETTIME)  defined(HAVE_CLOCK_MONOTONIC)
 
 static inline uint64_t highres_nanos(void)
 {
-- 
2.2.0.68.g8f72f0c.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/3] configure.ac: check for HMAC_CTX_cleanup

[Reuben Hawkins]

OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.
Newer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and
fall back to HMAC_cleanup when the newer function is missing.
---
 Makefile  | 3 +++
 configure.ac  | 7 +++
 git-compat-util.h | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/Makefile b/Makefile
index af551a0..d3c2b58 100644
--- a/Makefile
+++ b/Makefile
@@ -1059,6 +1059,9 @@ ifndef NO_OPENSSL
ifdef NEEDS_CRYPTO_WITH_SSL
OPENSSL_LIBSSL += -lcrypto
endif
+   ifdef NO_HMAC_CTX_CLEANUP
+   BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP
+   endif
 else
BASIC_CFLAGS += -DNO_OPENSSL
BLK_SHA1 = 1
diff --git a/configure.ac b/configure.ac
index 424dec5..c282663 100644
--- a/configure.ac
+++ b/configure.ac
@@ -923,6 +923,13 @@ AC_CHECK_LIB([iconv], [locale_charset],
  [CHARSET_LIB=-lcharset])])
 GIT_CONF_SUBST([CHARSET_LIB])
 #
+# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.
+AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
+   [NO_HMAC_CTX_CLEANUP=],
+   [NO_HMAC_CTX_CLEANUP=YesPlease],
+   [])
+GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP])
+#
 # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 GIT_CHECK_FUNC(clock_gettime,
 [HAVE_CLOCK_GETTIME=YesPlease],
diff --git a/git-compat-util.h b/git-compat-util.h
index 400e921..2fdca2d 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -213,6 +213,9 @@ extern char *gitbasename(char *);
 #ifndef NO_OPENSSL
 #include openssl/ssl.h
 #include openssl/err.h
+#ifdef NO_HMAC_CTX_CLEANUP
+#define HMAC_CTX_cleanup HMAC_cleanup
+#endif
 #endif
 
 /* On most systems netdb.h would have given us this, but
-- 
2.2.0.68.g8f72f0c.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/3] configure.ac: check tv_nsec field in struct stat

[Reuben Hawkins]

This check will automatically set the correct NO_NSEC setting.
---
 configure.ac | 12 
 1 file changed, 12 insertions(+)

diff --git a/configure.ac b/configure.ac
index 6af9647..dcc4bf0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],
 [#include sys/time.h])
 GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
 #
+# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist
+# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor 
stat.st_mtimespec.tv_nsec exist
+AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
+AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
+if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
+   USE_ST_TIMESPEC=YesPlease
+   GIT_CONF_SUBST([USE_ST_TIMESPEC])
+elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
+   NO_NSEC=YesPlease
+   GIT_CONF_SUBST([NO_NSEC])
+fi
+#
 # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
 AC_CHECK_MEMBER(struct dirent.d_ino,
 [NO_D_INO_IN_DIRENT=],
-- 
2.2.0.68.g8f72f0c.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 v2 2/2] t/t3308-notes-merge.sh: succeed with relaxed notes refs

[Junio C Hamano]

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

 On Tue, Jan 06, 2015 at 02:20:33AM -0800, Junio C Hamano wrote:

 The fact that git notes merge refs/heads/master fails is a very
 good prevention of end-user mistakes, and this removal of test
 demonstrates that we are dropping a valuable safety.

 Is it really that valuable? If it were:

   git notes merge master

 I could see somebody running that accidentally.
 ...
 But we are talking about
 somebody who is already fully-qualifying a ref (and anything unqualified
 continues to get looked up under refs/notes).

That (specifically 'merge') is not my real worry.  It's the other
way around, actually.

Because expand_notes_ref() makes sure that any given notes ref is
prefixed appropriately to start with refs/notes/,

git notes --ref=refs/heads/master add ...blah...
git notes --ref=refs/tag/v1.0 add ...blah...

would be a sensible way when somebody wants to keep a forest of
notes refs, one per real ref.  Wouldn't they have already been
trained to spell refs/heads/master when they want to refer to
refs/notes/refs/heads/master because of this?
--
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

A better git log --graph?

[Yuri D'Elia]

Hi everyone,

git log --graph is hard for me to parse mentally when developing a
project which has a lot of branches.

All the tools I've been using seem to just parse log --graph's output,
and thus are no better at showing history.

I would love to have a graph mode where each branch is assigned a
column, and stays there. If my log section shows the history of 3
branches, column 1 should always refer to master, 2 to the hypothetical
development branch and 3 to feature.

Of course the mode will waste more horizontal space, but it would be
immediately more apparent which branch is merging into which.

I saw this idea proposed a couple of times in the mailing list, but I
saw no action behind the proposal. Since I don't have time to work on
it, has anyone already started some work that he would like to share as
a starting point? Even just to have a felling if it's worth the effort.

Does anybody know of another tool to graph the history using something
that is not based on git log --graph?

I've seen a couple of graphviz-based ones, but both failed to work out
of the box for me.

Thanks a lot for any pointer.

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

Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Eric Sunshine]

On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 This check will automatically set the correct NO_NSEC setting.

This commit message neglects to mention the important point that
you're also now setting USE_ST_TIMESPEC when detected. You might
revise the message like this:

Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
NO_NSEC appropriately.

A side-effect of the above detection is that we also determine
whether 'stat.st_mtimespec' is available, so, as a bonus, set the
Makefile variable USE_ST_TIMESPEC, as well.

Also, your sign-off is missing (as mentioned in my previous review[1]).

[1]: http://article.gmane.org/gmane.comp.version-control.git/261626

 ---
  configure.ac | 12 
  1 file changed, 12 insertions(+)

 diff --git a/configure.ac b/configure.ac
 index 6af9647..dcc4bf0 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],
  [#include sys/time.h])
  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
  #
 +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist

It would be slightly more accurate to drop the .tv_nsec bit from this comment.

Also: s/exist/exists./

 +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor 
 stat.st_mtimespec.tv_nsec exist

Perhaps wrap this long comment over two lines.

Also: s/exist/exist./

 +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
 +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
 +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
 +   USE_ST_TIMESPEC=YesPlease
 +   GIT_CONF_SUBST([USE_ST_TIMESPEC])
 +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
 +   NO_NSEC=YesPlease
 +   GIT_CONF_SUBST([NO_NSEC])
 +fi
 +#
  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
  AC_CHECK_MEMBER(struct dirent.d_ino,
  [NO_D_INO_IN_DIRENT=],
 --
 2.2.0.68.g8f72f0c.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 3/3] configure.ac: check for HMAC_CTX_cleanup

[Eric Sunshine]

On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 OpenSSL version 0.9.6b and before defined the function HMAC_cleanup.
 Newer versions define HMAC_CTX_cleanup.  Check for HMAC_CTX_cleanup and
 fall back to HMAC_cleanup when the newer function is missing.

Missing sign-off.

Overall, these patches are nicely improved from the previous round. A
few more comments below...

 ---
  Makefile  | 3 +++
  configure.ac  | 7 +++
  git-compat-util.h | 3 +++
  3 files changed, 13 insertions(+)

 diff --git a/Makefile b/Makefile
 index af551a0..d3c2b58 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1059,6 +1059,9 @@ ifndef NO_OPENSSL
 ifdef NEEDS_CRYPTO_WITH_SSL
 OPENSSL_LIBSSL += -lcrypto
 endif
 +   ifdef NO_HMAC_CTX_CLEANUP
 +   BASIC_CFLAGS += -DNO_HMAC_CTX_CLEANUP
 +   endif

You need to document this new Makefile variable (NO_HMAC_CTX_CLEANUP)
at the top of Makefile (as mentioned in my previous review[1]).

[1]: http://article.gmane.org/gmane.comp.version-control.git/261631

  else
 BASIC_CFLAGS += -DNO_OPENSSL
 BLK_SHA1 = 1
 diff --git a/configure.ac b/configure.ac
 index 424dec5..c282663 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -923,6 +923,13 @@ AC_CHECK_LIB([iconv], [locale_charset],
   [CHARSET_LIB=-lcharset])])
  GIT_CONF_SUBST([CHARSET_LIB])
  #
 +# Define NO_HMAC_CTX_CLEANUP=YesPlease if HMAC_CTX_cleanup is missing.
 +AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
 +   [NO_HMAC_CTX_CLEANUP=],
 +   [NO_HMAC_CTX_CLEANUP=YesPlease],
 +   [])
 +GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP])

It is customary to drop empty trailing arguments in m4.

Also, you can simplify this entire check to:

AC_CHECK_LIB([crypto], [HMAC_CTX_cleanup],
[], [GIT_CONF_SUBST([NO_HMAC_CTX_CLEANUP], [YesPlease])])

  # Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
  GIT_CHECK_FUNC(clock_gettime,
  [HAVE_CLOCK_GETTIME=YesPlease],
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Reuben Hawkins]

On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 This check will automatically set the correct NO_NSEC setting.

 This commit message neglects to mention the important point that
 you're also now setting USE_ST_TIMESPEC when detected. You might
 revise the message like this:

 Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
 NO_NSEC appropriately.

 A side-effect of the above detection is that we also determine
 whether 'stat.st_mtimespec' is available, so, as a bonus, set the
 Makefile variable USE_ST_TIMESPEC, as well.

I see you're single quoted 'tv_nsec' and 'struct stat'.  Should I also
use single quotes in the first line of the commit msg like this...

configure.ac: check 'tv_nsec' field in 'struct stat'

?


 Also, your sign-off is missing (as mentioned in my previous review[1]).

 [1]: http://article.gmane.org/gmane.comp.version-control.git/261626

 ---
  configure.ac | 12 
  1 file changed, 12 insertions(+)

 diff --git a/configure.ac b/configure.ac
 index 6af9647..dcc4bf0 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],
  [#include sys/time.h])
  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
  #
 +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist

 It would be slightly more accurate to drop the .tv_nsec bit from this 
 comment.

 Also: s/exist/exists./

 +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor 
 stat.st_mtimespec.tv_nsec exist

 Perhaps wrap this long comment over two lines.

 Also: s/exist/exist./

 +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
 +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
 +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
 +   USE_ST_TIMESPEC=YesPlease
 +   GIT_CONF_SUBST([USE_ST_TIMESPEC])
 +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
 +   NO_NSEC=YesPlease
 +   GIT_CONF_SUBST([NO_NSEC])
 +fi
 +#
  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
  AC_CHECK_MEMBER(struct dirent.d_ino,
  [NO_D_INO_IN_DIRENT=],
 --
 2.2.0.68.g8f72f0c.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 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Reuben Hawkins]

On Wed, Jan 7, 2015 at 1:37 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
 systems being used in production.  This change makes compiling git
 less tedious on older platforms without CLOCK_MONOTONIC.

 The second sentence is implied by the very presence of this patch,
 thus adds no value. I, personally, would drop it.

 Also, your sign-off is missing (as mentioned in my previous review[1]).

 ---
 diff --git a/Makefile b/Makefile
 index 7482a4d..af551a0 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
 EXTLIBS += -lrt
  endif

 +ifdef HAVE_CLOCK_MONOTONIC
 +   BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
 +endif

 You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)
 at the top of Makefile (as mentioned in my previous review[1]), so
 that people who build without running 'configure' will know that they
 may need to tweak it.

  ifeq ($(TCLTK_PATH),)
  NO_TCLTK = NoThanks
  endif
 diff --git a/configure.ac b/configure.ac
 index dcc4bf0..424dec5 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
   [CHARSET_LIB=-lcharset])])
  GIT_CONF_SUBST([CHARSET_LIB])
  #
 +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 +GIT_CHECK_FUNC(clock_gettime,
 +[HAVE_CLOCK_GETTIME=YesPlease],
 +[HAVE_CLOCK_GETTIME=])
 +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])

 You could simplify the above four lines to this one-liner:

 GIT_CHECK_FUNC(clock_gettime,
 GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))

 +
 +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
 +AC_LANG_PROGRAM([[
 +#include time.h
 +clockid_t id = CLOCK_MONOTONIC;
 +]], [])])

 No need to pass empty trailing arguments in m4. It's customary to drop
 them altogether (since they are implied).

 +#
 +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
 +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
 +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
 +   [AC_MSG_RESULT([yes])
 +   HAVE_CLOCK_MONOTONIC=YesPlease],
 +   [AC_MSG_RESULT([no])
 +   HAVE_CLOCK_MONOTONIC=])
 +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])

 Ditto regarding simplification:

 AC_MSG_CHECKING([for CLOCK_MONOTONIC])
 AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
 [AC_MSG_RESULT([yes])
 GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
 [AC_MSG_RESULT([no])])

I *think* there's an issue with this simplification as used right
here.  In the 'no' case, HAVE_CLOCK_MONOTONIC *must* be undefined by
setting it equal to nothing

HAVE_CLOCK_MONOTONIC=

So that the setting in config.mak.uname 'HAVE_CLOCK_MONOTINIC =
YesPlease' will be overridden.

So this one needs to stay as is.


 +#
  # Define NO_SETITIMER if you don't have setitimer.
  GIT_CHECK_FUNC(setitimer,
  [NO_SETITIMER=],

 [1]: http://article.gmane.org/gmane.comp.version-control.git/261630
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Reuben Hawkins]

On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 This check will automatically set the correct NO_NSEC setting.

 This commit message neglects to mention the important point that
 you're also now setting USE_ST_TIMESPEC when detected. You might
 revise the message like this:

 Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
 NO_NSEC appropriately.

 A side-effect of the above detection is that we also determine
 whether 'stat.st_mtimespec' is available, so, as a bonus, set the
 Makefile variable USE_ST_TIMESPEC, as well.

 Also, your sign-off is missing (as mentioned in my previous review[1]).

 [1]: http://article.gmane.org/gmane.comp.version-control.git/261626

 ---
  configure.ac | 12 
  1 file changed, 12 insertions(+)

 diff --git a/configure.ac b/configure.ac
 index 6af9647..dcc4bf0 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],
  [#include sys/time.h])
  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
  #
 +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist

 It would be slightly more accurate to drop the .tv_nsec bit from this 
 comment.

The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec.  If I drop
tv_nsec from the comment should I also drop it in the check?

I thought it was better to be very explicit because the code using the
check is using that .tv_nsec field...I figured the check may as well
do exactly what the code is doing...



 Also: s/exist/exists./

 +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor 
 stat.st_mtimespec.tv_nsec exist

 Perhaps wrap this long comment over two lines.

 Also: s/exist/exist./

 +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
 +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
 +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
 +   USE_ST_TIMESPEC=YesPlease
 +   GIT_CONF_SUBST([USE_ST_TIMESPEC])
 +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
 +   NO_NSEC=YesPlease
 +   GIT_CONF_SUBST([NO_NSEC])
 +fi
 +#
  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
  AC_CHECK_MEMBER(struct dirent.d_ino,
  [NO_D_INO_IN_DIRENT=],
 --
 2.2.0.68.g8f72f0c.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 2/3] configure.ac: check for clock_gettime and CLOCK_MONOTONIC

[Eric Sunshine]

On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 CLOCK_MONOTONIC isn't available on RHEL3, but there are still RHEL3
 systems being used in production.  This change makes compiling git
 less tedious on older platforms without CLOCK_MONOTONIC.

The second sentence is implied by the very presence of this patch,
thus adds no value. I, personally, would drop it.

Also, your sign-off is missing (as mentioned in my previous review[1]).

 ---
 diff --git a/Makefile b/Makefile
 index 7482a4d..af551a0 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1382,6 +1382,10 @@ ifdef HAVE_CLOCK_GETTIME
 EXTLIBS += -lrt
  endif

 +ifdef HAVE_CLOCK_MONOTONIC
 +   BASIC_CFLAGS += -DHAVE_CLOCK_MONOTONIC
 +endif

You need to document this new Makefile variable (HAVE_CLOCK_MONOTONIC)
at the top of Makefile (as mentioned in my previous review[1]), so
that people who build without running 'configure' will know that they
may need to tweak it.

  ifeq ($(TCLTK_PATH),)
  NO_TCLTK = NoThanks
  endif
 diff --git a/configure.ac b/configure.ac
 index dcc4bf0..424dec5 100644
 --- a/configure.ac
 +++ b/configure.ac
 @@ -923,6 +923,28 @@ AC_CHECK_LIB([iconv], [locale_charset],
   [CHARSET_LIB=-lcharset])])
  GIT_CONF_SUBST([CHARSET_LIB])
  #
 +# Define HAVE_CLOCK_GETTIME=YesPlease if clock_gettime is available.
 +GIT_CHECK_FUNC(clock_gettime,
 +[HAVE_CLOCK_GETTIME=YesPlease],
 +[HAVE_CLOCK_GETTIME=])
 +GIT_CONF_SUBST([HAVE_CLOCK_GETTIME])

You could simplify the above four lines to this one-liner:

GIT_CHECK_FUNC(clock_gettime,
GIT_CONF_SUBST([HAVE_CLOCK_GETTIME], [YesPlease]))

 +
 +AC_DEFUN([CLOCK_MONOTONIC_SRC], [
 +AC_LANG_PROGRAM([[
 +#include time.h
 +clockid_t id = CLOCK_MONOTONIC;
 +]], [])])

No need to pass empty trailing arguments in m4. It's customary to drop
them altogether (since they are implied).

 +#
 +# Define HAVE_CLOCK_MONOTONIC=YesPlease if CLOCK_MONOTONIC is available.
 +AC_MSG_CHECKING([for CLOCK_MONOTONIC])
 +AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
 +   [AC_MSG_RESULT([yes])
 +   HAVE_CLOCK_MONOTONIC=YesPlease],
 +   [AC_MSG_RESULT([no])
 +   HAVE_CLOCK_MONOTONIC=])
 +GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC])

Ditto regarding simplification:

AC_MSG_CHECKING([for CLOCK_MONOTONIC])
AC_COMPILE_IFELSE([CLOCK_MONOTONIC_SRC],
[AC_MSG_RESULT([yes])
GIT_CONF_SUBST([HAVE_CLOCK_MONOTONIC], [YesPlease])],
[AC_MSG_RESULT([no])])

 +#
  # Define NO_SETITIMER if you don't have setitimer.
  GIT_CHECK_FUNC(setitimer,
  [NO_SETITIMER=],

[1]: http://article.gmane.org/gmane.comp.version-control.git/261630
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Eric Sunshine]

On Wed, Jan 7, 2015 at 4:33 PM, Reuben Hawkins reuben...@gmail.com wrote:
 On Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
 On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins reuben...@gmail.com wrote:
 +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist

 It would be slightly more accurate to drop the .tv_nsec bit from this 
 comment.

 The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec.  If I drop
 tv_nsec from the comment should I also drop it in the check?

No. My observation was just about the comment.

 I thought it was better to be very explicit because the code using the
 check is using that .tv_nsec field...I figured the check may as well
 do exactly what the code is doing...

Indeed, the check and code should agree. However, from the perspective
of the person reading comment, the .tv_nsec is just an
implementation detail of the check itself. The final outcome (the
setting of USE_ST_TIMESPEC) is independent of how that check was made:
it matters only that 'stat.st_mtimespec' was detected _somehow_.

Anyhow, it's just a minor observation, hence my qualification of it as
_slightly_ more accurate. If you feel strongly that it should remain
as is, then that's fine.
--
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] t/t3308-notes-merge.sh: succeed with relaxed notes refs

[Junio C Hamano]

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

 Alternatively (or additionally), for issue (2), we could add a
 --disable-ref-safety option to 'git notes', to explicitly disable the
 safety checks for experimental use.

I actually would rather prefer to see a proper plumbing use
supported, either by a new git notes-plumb subcommand or git
notes --some option that gives the scriptors a stable interface
to the low-level machinery without enforcing any Policy that belongs
to the Porcelain layer.  At the very least, the plumbing should:

 - disable anything that introduces potential ambiguity, e.g. DWIMs
   done by expand_notes_ref(), but not limited to it.

 - lift any restrictions based on policy, e.g. where can notes
   trees live (again, but not limited to this).

--
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: Additional plumbing commands

[Junio C Hamano]

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

 ... Knowing what each state _means_
 and what operations are meaningful to perform is much trickier (e.g., if
 we are in a rebase, you probably do not want to start a new rebase. But
 is it wrong to cherry-pick?).

 It would be nice if we could find a way to generalize in-progress
 operations and what they mean for starting new operations, but that is
 a much harder problem (if it is even possible at all).

Very good points. Thinking aloud, to see if we can start from a few
rules of thumb.

You can be in the middle for two largely different reasons.

One is that the command inherently wants to give control back to
you.  Think of a case where you said exec false in the rebase -i
insn sheet, or bisect checked out a version for you to try.

The other is that the command wanted to continue in the ideal world,
but couldn't and stopped asking for your help.  Think of a case
where am stopped due to corrupt patch, cherry-pick A..B or
revert stopped due to conflicts.

In the former case, depending on the nature of the command, what are
sensible things you can do are very different (during bisect you
would typically not want to do anything that causes a commit
created.  During rebase -i you may want to run any command that
creates a commit, to insert a new one into the series).  But a good
rule of thumb would be If I should be able to edit the working tree
file, I should also be able to do cherry-pick --no-commit, merge
--no-commit, apply, etc. If I should be able to manually
commit, I should also be able to cherry-pick, merge, etc.

In the latter, the _only_ reason you are given control back is to
help the interrupted operation.  So cherry-pick --no-commit or
apply in lieu of editing files manually in order to fix conflicts
should be allowed, but a random git merge (without --no-commit)
would not be.

So after thinking aloud for a while, I very much agree with you that
you cannot say X is allowed but not Y in many situations.

One thing we can say for sure is that in a middle of a multi-step
operation (e.g. rebase, range pick) is stopped for either one of the
two reasons, you cannot do another multi-step operation.  This is
not fundamental but a consequence of how the sequencing machinery is
implemented.  But other than that, it really is case-by-case and not
even command-by-command.
--
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: A better git log --graph?

[Yuri D'Elia]

On 01/07/2015 04:47 PM, Johan Herland wrote:
 Have you looked at git show-branch --all?

I didn't. Helpful, but I need to get used to the output.

The first impression after looking at some random repository histories
is that it's still not what I had in mind, though.


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

Re: A better git log --graph?

[Junio C Hamano]

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

 Have you looked at git show-branch --all?

 ...Johan

Yeah, sounds vaguely like it.  Its display certainly is easier to
read while the set of branches you have is minimum and everything
fits in a window; that is exactly why I wrote it back when the
branches I was handling were toy-sized (I am not saying Git itself
was toy-sized---the work-in-progress on top of Git I was doing was).


 On Wed, Jan 7, 2015 at 3:23 PM, Yuri D'Elia wav...@thregr.org wrote:
 Hi everyone,

 git log --graph is hard for me to parse mentally when developing a
 project which has a lot of branches.

 All the tools I've been using seem to just parse log --graph's output,
 and thus are no better at showing history.

 I would love to have a graph mode where each branch is assigned a
 column, and stays there. If my log section shows the history of 3
 branches, column 1 should always refer to master, 2 to the hypothetical
 development branch and 3 to feature.

 Of course the mode will waste more horizontal space, but it would be
 immediately more apparent which branch is merging into which.

 I saw this idea proposed a couple of times in the mailing list, but I
 saw no action behind the proposal. Since I don't have time to work on
 it, has anyone already started some work that he would like to share as
 a starting point? Even just to have a felling if it's worth the effort.

 Does anybody know of another tool to graph the history using something
 that is not based on git log --graph?

 I've seen a couple of graphviz-based ones, but both failed to work out
 of the box for me.

 Thanks a lot for any pointer.

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

Re: [PATCH 1/2] support for --no-relative and diff.relative

[Junio C Hamano]

kel...@shysecurity.com writes:

 Content-Type: text/plain; charset=utf-8; format=flowed

Please.  No format=flawed.  Really.

 Subject: Re: [PATCH 1/2] support for --no-relative and diff.relative

diff: teach --no-relative to override earlier --relative or
something.  Saying the affeced area upfront, terminated with a colon
':', will make it easier to spot in git shortlog output later.

Also this step is not about --no-relative and diff.relative but is
only about --no-relative option.

 added --no-relative option for git-diff (code, documentation, and tests)

Add --no-relative option...; we write in imperative, as if we are
giving an order to the project secretary to make the code do/be so.

 --no-relative overrides --relative causing a return to standard behavior

OK (modulo missing full-stop).


 Signed-off-by: Brandon Phillips kel...@shysecurity.com

Please also have

From: Brandon Phillips kel...@shysecurity.com

as the first line of the body of your e-mail message, if you are
letting your MUA only give your e-mail address without name.
Alternatively, please ask/configure your MUA to put your name as
well as your address on From: header of the e-mail (which is
preferrable).

 diff --git a/diff.c b/diff.c
 index d1bd534..7bceba8 100644
 --- a/diff.c
 +++ b/diff.c
 @@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options,
 const char **av, int ac)

Line-wrapped.

   DIFF_OPT_SET(options, RELATIVE_NAME);
   options-prefix = arg;
   }
 + else if (!strcmp(arg, --no-relative))
 + DIFF_OPT_CLR(options, RELATIVE_NAME);


When --relative is given, options-prefix is set to something as
we can see above.

The --relative option is described as optionally taking path in
the doc:

 --relative[=path]::
When run from a subdirectory of the project, it can be
told to exclude changes outside the directory and show
pathnames relative to it with this option.  When you are
not in a subdirectory (e.g. in a bare repository), you
can name which subdirectory to make the output relative
to by giving a path as an argument.

Doesn't --no-relative codepath have to undo the effect of that
assignment to options-prefix?

For example, after applying this patch, shouldn't

$ cd t
$ git show --relative=Documentation --no-relative --relative

work the same way as

$ cd t
$ git show --relative

i.e. limiting its output to the changes in the 't/' directory and
not to the changes in the 'Documentation/' directory?

Patch 2/2 also seems to share similar line-wrapping breakages that
make it unappliable, but more importantly, the configuration that is
supposed to correspond to --relative option only parses a boolean.
Is that the right design, or should it also be able to substitute a
command line `--relative=path` with an argument?

The last was a half-way rhetorical question and my answer is that
boolean-only is the best you could do, because we cannot do the
usual bool or string trick when string can be arbitrary.  In
other words, diff.relative=true could mean limit to the current
subdirectory aka --relative or it could mean limit to true/
subdirectory aka --relative=true, and there is no good way to
disambiguate between the two [*1*].

So I can agree with the design decision but only after spending 6
lines to think about it.  For the end-users, this design decision
needs to be explained and spelled out in the documentation.  Saying
equivalent to `--relative` is not sufficient, because the way
`--relative` option itself is described elsewhere.  The option
appears as `--relative[=path]` (see above), so some people _will_
read equivalent to `--relative` to mean Setting diff.relative=t
should be equivalent to --relative=t, which is not what actually
happens.


[Footnote]

*1* Actually, you could declare that diff.relative=true/ means the
'true/' directory while diff.relative=true means the boolean
'true' aka 'diff --relative', but I think it is too confusing.
Let's not make it worse by going that route.
--
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] doc: core.ignoreStat update, and clarify the --assume-unchanged effect

[Junio C Hamano]

Philip Oakley philipoak...@iee.org writes:

 The assume-unchanged bit, and consequently core.ignoreStat, can be
 misunderstood. Be assertive about the expectation that file changes should
 notified to Git.

 Overhaul the general wording thus:
 1. direct description of what is ignored given first.
 2. example instruction of the user manual action required.
 3. use sideways indirection for assume-unchanged and update-index
references.
 4. add a 'normally' to give leeway for the change detection.

 Signed-off-by: Philip Oakley philipoak...@iee.org
 ---

 This is the corrected patch which now applies on top of next and has been
 checked on AsciiDoc. (was $gmane/261974/focus=262022)

 I have included the previous 'after three-dashes' comment and included
 them in the commit message. I'm happy for it to be tweaked as appropriate.

Thanks.

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 52eeadd..fe179d0 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -375,15 +375,18 @@ This is useful for excluding servers inside a firewall 
 from
  proxy use, while defaulting to a common proxy for external domains.
  
  core.ignoreStat::
 + If true, Git will avoid using lstat() calls to detect if files have
 + changed. Git will set the assume-unchanged bit for those tracked files
 + which it has updated identically in both the index and working tree.

I wonder if this is better stated in two seemingly independent
sentences (like your version), or ... if files have changed by
setting the assume-unchanged bit  to clarify where the setting
of the bits to these files come into the big picture, but it is
minor.  Either way, I think it is a lot easier to understand than
what we have in 'master'.

 ++
 +When files are modified outside of Git, the user will need to stage
 +the modified files explicitly (e.g. see 'Examples' section in
 +linkgit:git-update-index[1]).
 +Git will not normally detect changes to those files.
 ++
 +This is useful on systems where lstat() calls are very slow, such as
 +CIFS/Microsoft Windows.
 +False by default.

I think you are trying to make the result more readable by using
separate paragraphs for separate conceptual points, but then isn't
it wrong to have False by default as part of stating which
platforms are intended targets?  I wonder if we want to have that
last line as its own paragraph instead.

Thanks.

  
  core.preferSymlinkRefs::
   Instead of the default symref format for HEAD
--
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: Tab completion missing for --includes and branch description in git config

[Stefan Beller]

On Tue, Jan 6, 2015 at 4:58 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 Hi all,

 While toying with the tab completion script for bash, I found a couple
 of things missing that cannot be completed:
 - git config --includes
 - git config branch.$BRANCH_NAME.description
 Attached are trivial patches based on master to fix those things.
 Regards,
 --
 Michael

Thanks for your effort on improving git!

Please have a look at Documentation/SubmittingPatches in git[1],
specially section (4) Sending your patches. so discussion on the
patches is easier.

Do not attach the patch as a MIME attachment, compressed or not.
Do not let your e-mail client send quoted-printable.  Do not let
your e-mail client send format=flowed which would destroy
whitespaces in your patches. Many
popular e-mail applications will not always transmit a MIME
attachment as plain text, making it impossible to comment on
your code.  A MIME attachment also takes a bit more time to
process.  This does not decrease the likelihood of your
MIME-attached change being accepted, but it makes it more likely
that it will be postponed.

The easiest way to comply with all these rules outlined in SubmittingPatches
is to use git format-patch and git send-email (as they follow the best
practice).

I recently wrote about my experiences when sending patches to the git
mailing list,
including how to configure git send-email[2], maybe that helps.

Thanks,
Stefan


[1] for example it can be found at
https://raw.githubusercontent.com/git/git/master/Documentation/SubmittingPatches

[2]http://thread.gmane.org/gmane.comp.version-control.git/261900
--
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] gitk: Bind g to focus the sha1 entry box

[Ismael Luceno]

This allows switching to some commit quickly.

Signed-off-by: Ismael Luceno ism...@iodev.co.uk
---
 gitk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gitk b/gitk
index 78358a7..9552fd4 100755
--- a/gitk
+++ b/gitk
@@ -2561,6 +2561,7 @@ proc makewindow {} {
 bindkey b prevfile
 bindkey d $ctext yview scroll 18 units
 bindkey u $ctext yview scroll -18 units
+bindkey g {$sha1entry delete 0 end; focus $sha1entry}
 bindkey / {focus $fstring}
 bindkey Key-KP_Divide {focus $fstring}
 bindkey Key-Return {dofind 1 1}
@@ -2980,6 +2981,7 @@ proc keys {} {
 [mc %s-FFind $M1T]
 [mc %s-GMove to next find hit $M1T]
 [mc Return  Move to next find hit]
+[mc g Go to commit]
 [mc / Focus the search box]
 [mc ? Move to previous find hit]
 [mc f Scroll diff view to next file]
-- 
2.2.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 1/2] support for --no-relative and diff.relative

[Junio C Hamano]

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

 Patch 2/2 also seems to share similar line-wrapping breakages that
 make it unappliable, but more importantly, the configuration that is
 supposed to correspond to --relative option only parses a boolean.
 Is that the right design, or should it also be able to substitute a
 command line `--relative=path` with an argument?

 The last was a half-way rhetorical question and my answer is that
 boolean-only is the best you could do...
 ...
 [Footnote]

 *1* Actually, you could declare that diff.relative=true/ means the
 'true/' directory while diff.relative=true means the boolean
 'true' aka 'diff --relative', but I think it is too confusing.
 Let's not make it worse by going that route.

Addendum.

It was only a half-way rhetorical question, because I am willing
to be persuaded that diff.relative=true/ vs diff.relative=true is
*not* too subtle/confusing to be a good idea, if enough people whose
judgement I trust agrees.

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

Re: [PATCH 1/2] support for --no-relative and diff.relative

[kelson]

Content-Type: text/plain; charset=utf-8; format=flowed

Please.  No format=flawed.  Really.

I'll figure out the line-wrapping.


Also this step is not about --no-relative and diff.relative but is only about 
--no-relative option.
Should I submit as two independent patches then? I took the approach of 
splitting them out into 1/2 vs 2/2 to distinguish, but it sounds like 
that isn't optimal.



When --relative is given, options-prefix is set to something as we can see 
above.

The --relative option is described as optionally taking path in the doc:

...

Doesn't --no-relative codepath have to undo the effect of that assignment to 
options-prefix?
diff_setup_done NULLs options-prefix when DIFF_OPT_TST(options, 
RELATIVE_NAME) is cleared (--no-relative clears this option).


On review, this may be a bad approach though. Non-locality makes it 
harder to follow/understand and introduces a subtle bug.
current:  git-diff --relative=path --no-relative --relative == 
git-diff --relative=path
expected: git-diff --relative=path --no-relative --relative == 
git-diff --relative



So I can agree with the design decision but only after spending 6
lines to think about it.  For the end-users, this design decision
needs to be explained and spelled out in the documentation.

Agreed; update to come.

-Original Message-
From: Junio C Hamano gits...@pobox.com
Sent: 01/07/2015 01:09 PM
To: kel...@shysecurity.com
CC: Git Mailing List git@vger.kernel.org,  Philip Oakley 
philipoak...@iee.org,  Duy Nguyen pclo...@gmail.com,  Jonathan 
Nieder jrnie...@gmail.com

Subject: Re: [PATCH 1/2] support for --no-relative and diff.relative


kel...@shysecurity.com writes:



Content-Type: text/plain; charset=utf-8; format=flowed


Please.  No format=flawed.  Really.


Subject: Re: [PATCH 1/2] support for --no-relative and diff.relative


diff: teach --no-relative to override earlier --relative or
something.  Saying the affeced area upfront, terminated with a colon
':', will make it easier to spot in git shortlog output later.

Also this step is not about --no-relative and diff.relative but is
only about --no-relative option.


added --no-relative option for git-diff (code, documentation, and tests)


Add --no-relative option...; we write in imperative, as if we are
giving an order to the project secretary to make the code do/be so.


--no-relative overrides --relative causing a return to standard behavior


OK (modulo missing full-stop).



Signed-off-by: Brandon Phillips kel...@shysecurity.com


Please also have

From: Brandon Phillips kel...@shysecurity.com

as the first line of the body of your e-mail message, if you are
letting your MUA only give your e-mail address without name.
Alternatively, please ask/configure your MUA to put your name as
well as your address on From: header of the e-mail (which is
preferrable).


diff --git a/diff.c b/diff.c
index d1bd534..7bceba8 100644
--- a/diff.c
+++ b/diff.c
@@ -3695,6 +3695,8 @@ int diff_opt_parse(struct diff_options *options,
const char **av, int ac)


Line-wrapped.


DIFF_OPT_SET(options, RELATIVE_NAME);
options-prefix = arg;
}
+   else if (!strcmp(arg, --no-relative))
+   DIFF_OPT_CLR(options, RELATIVE_NAME);



When --relative is given, options-prefix is set to something as
we can see above.

The --relative option is described as optionally taking path in
the doc:

  --relative[=path]::
 When run from a subdirectory of the project, it can be
 told to exclude changes outside the directory and show
 pathnames relative to it with this option.  When you are
 not in a subdirectory (e.g. in a bare repository), you
 can name which subdirectory to make the output relative
 to by giving a path as an argument.

Doesn't --no-relative codepath have to undo the effect of that
assignment to options-prefix?

For example, after applying this patch, shouldn't

$ cd t
$ git show --relative=Documentation --no-relative --relative

work the same way as

$ cd t
$ git show --relative

i.e. limiting its output to the changes in the 't/' directory and
not to the changes in the 'Documentation/' directory?

Patch 2/2 also seems to share similar line-wrapping breakages that
make it unappliable, but more importantly, the configuration that is
supposed to correspond to --relative option only parses a boolean.
Is that the right design, or should it also be able to substitute a
command line `--relative=path` with an argument?

The last was a half-way rhetorical question and my answer is that
boolean-only is the best you could do, because we cannot do the
usual bool or string trick when string can be arbitrary.  In
other words, diff.relative=true could mean limit to the current
subdirectory aka --relative or it could mean limit to true/
subdirectory aka --relative=true, and there is no good way to
disambiguate between the two [*1*].

So I can agree with the design 

[PATCH v3 2/2] format-patch: ignore diff.submodule setting

[Doug Kelly]

diff.submodule when set to log produces output which git-am cannot
handle. Ignore this setting when generating patch output.

Signed-off-by: Doug Kelly dougk@gmail.com
---
 builtin/log.c   | 2 +-
 t/t4255-am-submodule.sh | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index 734aab3..cb14db4 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -705,7 +705,7 @@ static int git_format_config(const char *var, const char 
*value, void *cb)
return 0;
}
if (!strcmp(var, diff.color) || !strcmp(var, color.diff) ||
-   !strcmp(var, color.ui)) {
+   !strcmp(var, color.ui) || !strcmp(var, diff.submodule)) {
return 0;
}
if (!strcmp(var, format.numbered)) {
diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 523accf..31cbdba 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -80,12 +80,12 @@ test_expect_success 'diff.submodule unset with extra file' '
run_test $THIRD second-submodule
 '
 
-test_expect_failure 'diff.submodule=log' '
+test_expect_success 'diff.submodule=log' '
test_config diff.submodule log 
run_test $SECOND first-submodule
 '
 
-test_expect_failure 'diff.submodule=log with extra file' '
+test_expect_success 'diff.submodule=log with extra file' '
test_config diff.submodule log 
run_test $THIRD second-submodule
 '
-- 
2.0.5

--
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 1/2] t4255: test am submodule with diff.submodule

[Doug Kelly]

git am will break when using diff.submodule=log; add some test cases
to illustrate this breakage as simply as possible.  There are
currently two ways this can fail:

* With errors (unrecognized input), if only change
* Silently (no submodule change), if other files change

Test for both conditions and ensure without diff.submodule this works.

Signed-off-by: Doug Kelly dougk@gmail.com
Thanks-to: Eric Sunshine sunsh...@sunshineco.com
Thanks-to: Junio C Hamano gits...@pobox.com
---
Updated with Eric Sunshine's comments and changes to reduce complexity,
and also changed to include Junio's suggestions for using test_config,
test_unconfig, and test_might_fail (since we don't know if a previous
am failed or not -- we always want to clean up first).

 t/t4255-am-submodule.sh | 72 +
 1 file changed, 72 insertions(+)

diff --git a/t/t4255-am-submodule.sh b/t/t4255-am-submodule.sh
index 8bde7db..523accf 100755
--- a/t/t4255-am-submodule.sh
+++ b/t/t4255-am-submodule.sh
@@ -18,4 +18,76 @@ am_3way () {
 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1
 test_submodule_switch am_3way
 
+test_expect_success 'setup diff.submodule' '
+   test_commit one 
+   INITIAL=$(git rev-parse HEAD) 
+
+   git init submodule 
+   (
+   cd submodule 
+   test_commit two 
+   git rev-parse HEAD ../initial-submodule
+   ) 
+   git submodule add ./submodule 
+   git commit -m first 
+
+   (
+   cd submodule 
+   test_commit three 
+   git rev-parse HEAD ../first-submodule
+   ) 
+   git add submodule 
+   test_tick 
+   git commit -m second 
+   SECOND=$(git rev-parse HEAD) 
+
+   (
+   cd submodule 
+   git mv two.t four.t 
+   test_tick 
+   git commit -m second submodule 
+   git rev-parse HEAD ../second-submodule
+   ) 
+   test_commit four 
+   git add submodule 
+   git commit --amend --no-edit 
+   THIRD=$(git rev-parse HEAD) 
+   git submodule update --init
+'
+
+run_test() {
+   START_COMMIT=$1 
+   EXPECT=$2 
+   test_might_fail git am --abort 
+   git reset --hard $START_COMMIT 
+   rm -f *.patch 
+   git format-patch -1 
+   git reset --hard $START_COMMIT^ 
+   git submodule update 
+   git am *.patch 
+   git submodule update 
+   git -C submodule rev-parse HEAD actual 
+   test_cmp $EXPECT actual
+}
+
+test_expect_success 'diff.submodule unset' '
+   test_unconfig diff.submodule 
+   run_test $SECOND first-submodule
+'
+
+test_expect_success 'diff.submodule unset with extra file' '
+   test_unconfig diff.submodule 
+   run_test $THIRD second-submodule
+'
+
+test_expect_failure 'diff.submodule=log' '
+   test_config diff.submodule log 
+   run_test $SECOND first-submodule
+'
+
+test_expect_failure 'diff.submodule=log with extra file' '
+   test_config diff.submodule log 
+   run_test $THIRD second-submodule
+'
+
 test_done
-- 
2.0.5

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

Re: [PATCH 1/2] t4255: test am submodule with diff.submodule

[Doug Kelly]

On Mon, Dec 29, 2014 at 9:42 AM, Junio C Hamano gits...@pobox.com wrote:
 Eric Sunshine sunsh...@sunshineco.com writes:

 +   (git am --abort || true) 

 Why (x || y)?  Is 'x' so unreliable that we do not know how should exit?
 Should this be test_must_fail git am --abort?

Updated to test_might_fail -- we don't know if a merge is in progress or not.
We still need to clean up, but disregard failure if a merge isn't in progress.

 +   (cd submodule  git rev-parse HEAD ../actual) 

 git -C submodule rev-parse HEAD actual perhaps?

Seems sane to me.

 +test_expect_success 'diff.submodule unset' '
 +   (git config --unset diff.submodule || true) 

 I think test_config and test_unconfig were invented for things like
 this (same for all the other use of git config).
Yep, much nicer. :) Also updated to test_commit as suggested by Eric.

Thanks!

--Doug
--
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: A better git log --graph?

[Johan Herland]

Have you looked at git show-branch --all?

...Johan

On Wed, Jan 7, 2015 at 3:23 PM, Yuri D'Elia wav...@thregr.org wrote:
 Hi everyone,

 git log --graph is hard for me to parse mentally when developing a
 project which has a lot of branches.

 All the tools I've been using seem to just parse log --graph's output,
 and thus are no better at showing history.

 I would love to have a graph mode where each branch is assigned a
 column, and stays there. If my log section shows the history of 3
 branches, column 1 should always refer to master, 2 to the hypothetical
 development branch and 3 to feature.

 Of course the mode will waste more horizontal space, but it would be
 immediately more apparent which branch is merging into which.

 I saw this idea proposed a couple of times in the mailing list, but I
 saw no action behind the proposal. Since I don't have time to work on
 it, has anyone already started some work that he would like to share as
 a starting point? Even just to have a felling if it's worth the effort.

 Does anybody know of another tool to graph the history using something
 that is not based on git log --graph?

 I've seen a couple of graphviz-based ones, but both failed to work out
 of the box for me.

 Thanks a lot for any pointer.

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



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