[PATCH 1/1] tests: make comment match the code
GnuPG homedir is generated on the fly and keys are imported from armored key file. Make commet match available key info and new key generation procedure. Signed-off-by: Christian Hesse m...@eworm.de --- t/lib-gpg.sh | 16 +--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh index 33de402..d88da29 100755 --- a/t/lib-gpg.sh +++ b/t/lib-gpg.sh @@ -12,10 +12,20 @@ else say Your version of gpg (1.0.6) is too buggy for testing ;; *) - # key generation info: gpg --homedir t/lib-gpg --gen-key - # Type DSA and Elgamal, size 2048 bits, no expiration date. - # Name and email: C O Mitter commit...@example.com + # Available key info: + # * Type DSA and Elgamal, size 2048 bits, no expiration date, + # name and email: C O Mitter commit...@example.com + # * Type RSA, size 2048 bits, no expiration date, + # name and email: Eris Discordia disc...@example.net # No password given, to enable non-interactive operation. + # To generate new key: + # gpg --homedir /tmp/gpghome --gen-key + # To write armored exported key to keyring: + # gpg --homedir /tmp/gpghome --export-secret-keys \ + # --armor 0xDEADBEEF lib-gpg/keyring.gpg + # To export ownertrust: + # gpg --homedir /tmp/gpghome --export-ownertrust \ + #lib-gpg/ownertrust mkdir ./gpghome chmod 0700 ./gpghome GNUPGHOME=$(pwd)/gpghome -- 2.2.0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote: allow adding remote w same name as alias
Hi Anastas, On Tue, 16 Dec 2014, Anastas Dancha wrote: When ~/.gitconfig contains an alias (i.e. myremote) and you are adding a new remote using the same name for remote, Git will refuse to add the remote with the same name as one of the aliases, even though the remote with such name is not setup for current repo. Just to make sure we're on the same page... you are talking about [remote myremote] not [alias] myremote = ... yes? If so, please avoid using the term alias... Further, I assume that your .gitconfig lists the myremote without a URL? Also: - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || - remote-fetch_refspec_nr)) - die(_(remote %s already exists.), name); + if (remote (remote-url_nr 1 || remote-fetch_refspec_nr)) + die(_(remote %s %s already exists.), name, url); The real problem here is that strcmp() is performed even if url_nr == 0, *and* that it compares the name – instead of the url – to the remote's URL. That is incorrect, so the correct fix would be: - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || + (remote-url_nr == 1 strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); In other words, we would still verify that there is no existing remote, even if that remote was declared in ~/.gitconfig. However, if a remote exists without any URL, or if it has a single URL that matches the provided one, and there are no fetch refspecs, *then* there is nothing to complain about and we continue. Ciao, Johannes
Re: [PATCH] remote: allow adding remote w same name as alias
Anastas Dancha schrieb am 16.12.2014 um 03:30: From f80bdf3272e7bdf790ee67fb94196a8aa139331f Mon Sep 17 00:00:00 2001 From: Anastas Dancha anap...@random.io Date: Mon, 15 Dec 2014 16:30:50 -0500 Subject: [PATCH] remote: allow adding remote w same name as alias When ~/.gitconfig contains an alias (i.e. myremote) and you are adding a new remote using the same name for remote, Git will refuse to add the remote with the same name as one of the aliases, even though the remote with such name is not setup for current repo. $ git remote add myremote g...@host.com:team/repo.git fatal: remote myremote already exists. The fatal error comes from strcmp(name, remote-url[0]) condition, which compares a name of the new remote with existing urls of all the remotes, including the ones from ~/.gitconfig (or global variant). I'm not sure why that is necessary, unless Git is meant to prevent users from naming their remotes as their remote aliases.. Imho, if someone want's to git remote add myremote myremote, they should, since git-remote always takes 2 arguments, first being the new remote's name and second being new remote's url (or alias, if set). While that is true for git remote, it is wrong for git push and the like, which takes remote name or remote URL as a parameter. Therefore, remote names and remote aliases need to be unique within the same namespace. Thanks to @mymuss for sanity check and debugging. Signed-off-by: Anastas Dancha anap...@random.io --- builtin/remote.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] remote: allow adding remote w same name as alias
My bad Johannes, Then I wrote alias, I've meant the following: ``` [url g...@githost.com] insteadOf = myalias pushInsteadOf = myalias ``` Unfortunately, your suggested fix will not allow my [poorly] described use case. Hope this makes more sense now. Thank you for looking into this. -Anastas On Tue, Dec 16, 2014 at 4:01 AM, Johannes Schindelin johannes.schinde...@gmx.de wrote: Hi Anastas, On Tue, 16 Dec 2014, Anastas Dancha wrote: When ~/.gitconfig contains an alias (i.e. myremote) and you are adding a new remote using the same name for remote, Git will refuse to add the remote with the same name as one of the aliases, even though the remote with such name is not setup for current repo. Just to make sure we're on the same page... you are talking about [remote myremote] not [alias] myremote = ... yes? If so, please avoid using the term alias... Further, I assume that your .gitconfig lists the myremote without a URL? Also: - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || - remote-fetch_refspec_nr)) - die(_(remote %s already exists.), name); + if (remote (remote-url_nr 1 || remote-fetch_refspec_nr)) + die(_(remote %s %s already exists.), name, url); The real problem here is that strcmp() is performed even if url_nr == 0, *and* that it compares the name – instead of the url – to the remote's URL. That is incorrect, so the correct fix would be: - if (remote (remote-url_nr 1 || strcmp(name, remote-url[0]) || + if (remote (remote-url_nr 1 || + (remote-url_nr == 1 strcmp(url, remote-url[0])) || remote-fetch_refspec_nr)) die(_(remote %s already exists.), name); In other words, we would still verify that there is no existing remote, even if that remote was declared in ~/.gitconfig. However, if a remote exists without any URL, or if it has a single URL that matches the provided one, and there are no fetch refspecs, *then* there is nothing to complain about and we continue. Ciao, Johannes -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
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:
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 | 176 +
1 file changed, 176 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..6354fc0
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,176 @@
+#!/bin/sh
+
+test_description='pushing to a repository using the atomic push option'
+
+. ./test-lib.sh
+
+D=`pwd`
+
+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
+ )
+}
+
+# refname, expected value, e.g.
+# test_ref_upstream refs/heads/master HEADS{0}
+test_ref_upstream () {
+ test $# == 2 # if this fails, we have a bug in this script.
+ test $(git -C upstream rev-parse --verify $1) == $2
+}
+
+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_ref_upstream master $(git -C workbench rev-parse --verify 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_ref_upstream master $(git -C workbench rev-parse --verify master)
+ test_ref_upstream second $(git -C workbench rev-parse --verify 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_ref_upstream master $(git -C workbench rev-parse --verify master)
+ test_ref_upstream second $(git -C workbench rev-parse --verify 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
+ # The actual test is below
+ git checkout master
+ test_commit
[PATCHv2 1/6] receive-pack.c: add protocol support to negotiate atomic-push
From: Ronnie Sahlberg sahlb...@google.com
This adds support to the protocol between send-pack and receive-pack to
* allow receive-pack to inform the client that it has atomic push capability
* allow send-pack to request atomic push back.
There is currently no setting in send-pack to actually request that atomic
pushes are to be used yet. This only adds protocol capability not ability
for the user to activate it.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
Changes v1 - v2:
* Name it atomic instead of atomic-push. The name changes affects
the flags send over the wire as well as the flags in
struct send_pack_args
* Add check, which was part of the later patch here:
if (args-atomic !atomic_supported) {
fprintf(stderr, Server does not support atomic push.);
return -1;
}
Documentation/technical/protocol-capabilities.txt | 13 +++--
builtin/receive-pack.c| 6 +-
send-pack.c | 11 +++
send-pack.h | 3 ++-
4 files changed, 29 insertions(+), 4 deletions(-)
diff --git a/Documentation/technical/protocol-capabilities.txt
b/Documentation/technical/protocol-capabilities.txt
index 6d5424c..68ec23d 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 single 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 32fc540..e76e5d5 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -40,6 +40,7 @@ static int transfer_unpack_limit = -1;
static int unpack_limit = 100;
static int report_status;
static int use_sideband;
+static int use_atomic;
static int quiet;
static int prefer_ofs_delta = 1;
static int auto_update_server_info;
@@ -171,7 +172,8 @@ static void show_ref(const char *path, const unsigned char
*sha1)
struct strbuf cap = STRBUF_INIT;
strbuf_addstr(cap,
- report-status delete-refs side-band-64k quiet);
+ report-status delete-refs side-band-64k quiet
+ atomic);
if (prefer_ofs_delta)
strbuf_addstr(cap, ofs-delta);
if (push_cert_nonce)
@@ -1179,6 +1181,8 @@ static struct command *read_head_info(struct sha1_array
*shallow)
use_sideband = LARGE_PACKET_MAX;
if (parse_feature_request(feature_list, quiet))
quiet = 1;
+ if (parse_feature_request(feature_list, atomic))
+ use_atomic = 1;
}
if (!strcmp(line, push-cert)) {
diff --git a/send-pack.c b/send-pack.c
index 949cb61..2a513f4 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+ int use_atomic;
+ int atomic_supported = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports(no-thin))
args-use_thin_pack = 0;
+ if (server_supports(atomic))
+ atomic_supported = 1;
if (args-push_cert) {
int len;
@@ -328,6 +332,11 @@ int send_pack(struct send_pack_args *args,
Perhaps you should specify a branch such as
'master'.\n);
return 0;
}
+ if (args-atomic !atomic_supported)
[PATCHv2 3/6] send-pack.c: add --atomic command line argument
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:
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 | 36 ++--
transport.c | 4
5 files changed, 51 insertions(+), 5 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)
args.use_thin_pack = 1;
continue;
}
+ if (!strcmp(arg, --atomic)) {
+ args.atomic = 1;
+ continue;
+ }
if (!strcmp(arg, --stateless-rpc)) {
args.stateless_rpc = 1;
continue;
diff --git a/remote.h b/remote.h
index 8b62efd..f346524 100644
--- a/remote.h
+++ b/remote.h
@@ -115,7 +115,8 @@ struct ref {
REF_STATUS_REJECT_SHALLOW,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
- REF_STATUS_EXPECTING_REPORT
+ REF_STATUS_EXPECTING_REPORT,
+ REF_STATUS_ATOMIC_PUSH_FAILED
} status;
[PATCHv2 5/6] push.c: add an --atomic-push argument
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:
Changes v1 - v2
It's --atomic now! (dropping the -push)
Documentation/git-push.txt | 7 ++-
builtin/push.c | 2 ++
transport.c| 1 +
transport.h| 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..3feacc5 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n |
--dry-run] [--receive-pack=git-receive-pack]
[--repo=repository] [-f | --force] [--prune] [-v | --verbose]
[-u | --set-upstream] [--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.
+--atomic::
+ Use the an atomic transaction on the server 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..fe0b8cc 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -518,6 +518,8 @@ 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_BIT(0, atomic, flags, N_(use a single atomic
transaction at the serverside, if available),
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
diff --git a/transport.c b/transport.c
index c67feee..5b29514 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_ATOMIC_PUSH);
args.url = transport-url;
ret = send_pack(args, data-fd, data-conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 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_ATOMIC_PUSH 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.0.31.gad78000.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
[PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
From: Ronnie Sahlberg sahlb...@google.com
Update receive-pack to use an atomic transaction iff the client negotiated
that it wanted atomic-push. This leaves the default behavior to be the old
non-atomic one ref at a time update. This is to cause as little disruption
as possible to existing clients. It is unknown if there are client scripts
that depend on the old non-atomic behavior so we make it opt-in for now.
If it turns out over time that there are no client scripts that depend on the
old behavior we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
Changes v1 - v2:
* update(...) assumes to be always in a transaction
* Caring about when to begin/commit transactions is put
into execute_commands
builtin/receive-pack.c | 64 ++
1 file changed, 49 insertions(+), 15 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e76e5d5..0803fd2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = SLOP;
static const char *nonce_status;
static long nonce_stamp_slop;
static unsigned long nonce_stamp_slop_limit;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -832,34 +834,32 @@ 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;
}
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 */
}
@@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = transaction error;
+ return;
+ }
+ }
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0, data))
@@ -1086,8 +1096,23 @@ static void execute_commands(struct command *commands,
if (cmd-skip_update)
continue;
-
+ if (!use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ rp_error(%s, err.buf);
+ strbuf_release(err);
+ cmd-error_string = failed to start
transaction;
+ }
+ }
cmd-error_string = update(cmd, si);
+ if (!use_atomic)
+ if (ref_transaction_commit(transaction, err)) {
+
[PATCHv2 2/6] send-pack: Invert the return value of ref_update_to_be_sent
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 REF_STATUS_REJECT_* case.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
New in the series. For consistency with all the other patches
it also reads v2.
send-pack.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 2a513f4..1c4ac75 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,10 +190,10 @@ 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)
+static int no_ref_update_to_be_sent(const struct ref *ref, const struct
send_pack_args *args)
{
if (!ref-peer_ref !args-send_mirror)
- return 0;
+ return 1;
/* Check for statuses set by set_ref_status_for_push() */
switch (ref-status) {
@@ -203,10 +203,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 2;
case REF_STATUS_UPTODATE:
- return 0;
+ return 3;
default:
- return 1;
+ return 0;
}
}
@@ -250,7 +251,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 (no_ref_update_to_be_sent(ref, args))
continue;
update_seen = 1;
strbuf_addf(cert, %s %s %s\n,
@@ -370,7 +371,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 (no_ref_update_to_be_sent(ref, args))
continue;
if (!ref-deletion)
@@ -391,7 +392,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 (no_ref_update_to_be_sent(ref, args))
continue;
old_hex = sha1_to_hex(ref-old_sha1);
--
2.2.0.31.gad78000.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] t5400: remove dead code
Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] tests: make comment match the code
Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/6] receive-pack.c: add protocol support to negotiate atomic-push
Stefan Beller sbel...@google.com writes:
diff --git a/send-pack.c b/send-pack.c
index 949cb61..2a513f4 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -294,6 +294,8 @@ int send_pack(struct send_pack_args *args,
int use_sideband = 0;
int quiet_supported = 0;
int agent_supported = 0;
+ int use_atomic;
+ int atomic_supported = 0;
unsigned cmds_sent = 0;
int ret;
struct async demux;
@@ -314,6 +316,8 @@ int send_pack(struct send_pack_args *args,
agent_supported = 1;
if (server_supports(no-thin))
args-use_thin_pack = 0;
+ if (server_supports(atomic))
+ atomic_supported = 1;
if (args-push_cert) {
int len;
@@ -328,6 +332,11 @@ int send_pack(struct send_pack_args *args,
Perhaps you should specify a branch such as
'master'.\n);
return 0;
}
+ if (args-atomic !atomic_supported) {
+ fprintf(stderr, Server does not support atomic push.);
+ return -1;
I'd tweak this to
return error(server does not support atomic push.);
to (0) shorten, (1) make sure the message is terminated with LF,
and (2) match the other error messages in the program.
Other than that looks good.
Thanks.
+ }
+ use_atomic = atomic_supported args-atomic;
if (status_report)
strbuf_addstr(cap_buf, report-status);
@@ -335,6 +344,8 @@ int send_pack(struct send_pack_args *args,
strbuf_addstr(cap_buf, side-band-64k);
if (quiet_supported (args-quiet || !args-progress))
strbuf_addstr(cap_buf, quiet);
+ if (use_atomic)
+ strbuf_addstr(cap_buf, atomic);
if (agent_supported)
strbuf_addf(cap_buf, agent=%s, git_user_agent_sanitized());
diff --git a/send-pack.h b/send-pack.h
index 5635457..b664648 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -13,7 +13,8 @@ struct send_pack_args {
use_ofs_delta:1,
dry_run:1,
push_cert:1,
- stateless_rpc:1;
+ stateless_rpc:1,
+ atomic:1;
};
int send_pack(struct send_pack_args *args,
--
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/1] tests: make comment match the code
On Tue, Dec 16, 2014 at 3:40 AM, Christian Hesse m...@eworm.de wrote: GnuPG homedir is generated on the fly and keys are imported from armored key file. Make commet match available key info and new key s/commet/comment/ generation procedure. Signed-off-by: Christian Hesse m...@eworm.de -- 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: [PATCHv2 2/6] send-pack: Invert the return value of ref_update_to_be_sent
Stefan Beller sbel...@google.com writes:
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 REF_STATUS_REJECT_* case.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
New in the series. For consistency with all the other patches
it also reads v2.
Sorry, but ECANNOTPARSE especially it also read v2 part.
send-pack.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/send-pack.c b/send-pack.c
index 2a513f4..1c4ac75 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -190,10 +190,10 @@ 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)
+static int no_ref_update_to_be_sent(const struct ref *ref, const struct
send_pack_args *args)
{
if (!ref-peer_ref !args-send_mirror)
- return 0;
+ return 1;
/* Check for statuses set by set_ref_status_for_push() */
switch (ref-status) {
@@ -203,10 +203,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 2;
case REF_STATUS_UPTODATE:
- return 0;
+ return 3;
default:
- return 1;
+ return 0;
Hmmm. It used to be will we send this one? It is fine if the
function wants to differenciate various kinds of error, but
(1) unexplained 1, 2 and 3 looks cryptic and unmaintainable;
(2) no_ prefix in the function name is usually a bad idea, because
it easily invites if (!no_foo(...)) double negation; and
(3) unless there is a good reason to do otherwise, a function that
returns 0 on success should signal an error with a negative
return.
static int check_to_send_update() or something, perhaps?
if (check_to_send_update() 0)
/* skip as this is an error */
continue;
does not look too bad.
}
@@ -250,7 +251,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 (no_ref_update_to_be_sent(ref, args))
continue;
update_seen = 1;
strbuf_addf(cert, %s %s %s\n,
@@ -370,7 +371,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 (no_ref_update_to_be_sent(ref, args))
continue;
if (!ref-deletion)
@@ -391,7 +392,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 (no_ref_update_to_be_sent(ref, args))
continue;
old_hex = sha1_to_hex(ref-old_sha1);
--
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] receive-pack: refuse all commands if one fails in atomic mode
This patch will be incorporated into the right places in v3 of the series.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
I don't want to resend the patch series today to accumulate comments,
but this makes the last test pass, which is the whole point of the series.
I'll put it into the right places in a reroll.
builtin/receive-pack.c | 13 -
t/t5543-atomic-push.sh | 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0803fd2..3477f7c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1122,7 +1122,18 @@ static void execute_commands(struct command *commands,
}
if (use_atomic) {
- if (ref_transaction_commit(transaction, err)) {
+ /* 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;
+ if (cmd) {
+ for (cmd = commands; cmd; cmd = cmd-next)
+ if (!cmd-error_string)
+ cmd-error_string = atomic push
failure;
+ } else if (ref_transaction_commit(transaction, err)) {
rp_error(%s, err.buf);
for (cmd = commands; cmd; cmd = cmd-next)
cmd-error_string = err.buf;
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 6354fc0..f0e54d9 100755
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -142,7 +142,7 @@ test_expect_success 'atomic push fails if one tag fails
remotely' '
test_ref_upstream second $(git -C workbench rev-parse --verify HEAD@{1})
'
-test_expect_failure 'atomic push obeys update hook preventing a branch to be
pushed' '
+test_expect_success 'atomic push obeys update hook preventing a branch to be
pushed' '
mk_repo_pair
(
cd workbench
--
2.2.0.31.gad78000.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: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller sbel...@google.com wrote:
From: Ronnie Sahlberg sahlb...@google.com
Update receive-pack to use an atomic transaction iff the client negotiated
that it wanted atomic-push. This leaves the default behavior to be the old
non-atomic one ref at a time update. This is to cause as little disruption
as possible to existing clients. It is unknown if there are client scripts
that depend on the old non-atomic behavior so we make it opt-in for now.
If it turns out over time that there are no client scripts that depend on the
old behavior we can change git to default to use atomic pushes and instead
offer an opt-out argument for people that do not want atomic pushes.
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e76e5d5..0803fd2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = SLOP;
static const char *nonce_status;
static long nonce_stamp_slop;
static unsigned long nonce_stamp_slop_limit;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
Should these be static?
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = transaction error;
+ return;
+ }
+ }
Not seen in this diff, but just below this point, the pre-receive hook
is invoked. If it declines, then execute_commands() returns without
releasing the transaction which was begun here. Is that correct
behavior?
For robustness, it might also be sane to release the 'err' strbuf at
this early return (though the current code does not strictly leak it).
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0,
data))
@@ -1086,8 +1096,23 @@ static void execute_commands(struct command *commands,
if (cmd-skip_update)
continue;
-
+ if (!use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ rp_error(%s, err.buf);
+ strbuf_release(err);
+ cmd-error_string = failed to start
transaction;
Here, you assign cmd-error_string...
+ }
+ }
cmd-error_string = update(cmd, si);
and then immediately overwrite it here. Is it correct to invoke
update() when ref_transaction_begin() has failed? Seems fishy.
+ if (!use_atomic)
+ if (ref_transaction_commit(transaction, err)) {
+ ref_transaction_free(transaction);
Shouldn't you be freeing the transaction outside of this conditional
rather than only in case of failure?
+ rp_error(%s, err.buf);
+ strbuf_release(err);
+ cmd-error_string = failed to update ref;
+ }
+
if (shallow_update !cmd-error_string
si-shallow_ref[cmd-index]) {
error(BUG: connectivity check has not been run on
ref %s,
@@ -1096,6 +1121,14 @@ static void execute_commands(struct command *commands,
}
}
+ if (use_atomic) {
+ if (ref_transaction_commit(transaction, err)) {
+ rp_error(%s, err.buf);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = err.buf;
+ }
+ ref_transaction_free(transaction);
+ }
if (shallow_update !checked_connectivity)
error(BUG: run 'git fsck' for safety.\n
If there are errors, try to remove
@@ -1543,5 +1576,6 @@ int cmd_receive_pack(int argc, const char **argv, const
char *prefix)
sha1_array_clear(shallow);
sha1_array_clear(ref);
free((void *)push_cert_nonce);
+ strbuf_release(err);
return 0;
}
--
2.2.0.31.gad78000.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: [PATCHv2 3/6] send-pack.c: add --atomic command line argument
Stefan Beller sbel...@google.com writes:
* 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?
I suspect that you completely mis-read me.
If you add an extra arg that is *specifically* for atomic push, like
this:
-static int update_to_send(...);
+static int update_to_send(..., int *atomic_push_failed);
what signal does it send to the next person who may want to add
a new nuclear push option? Should the patch look like
-static int update_to_send(..., int *atomic_push_failed);
+static int update_to_send(..., int *atomic_push_failed, int
*nuclear_push_failed);
by adding yet another separate variable for error reporting?
I would have understood if the new variable was named something like
failure_reason, which may be set to PUSH_FAILURE_ATOMIC when an
atomic push failure was detected. Making the helper return the
reason why the push failed would be another way, like you did in the
2/6 patch in this round.
Either way, the next person would know to add a code to do his
nuclear push and set the reason to PUSH_FAILURE_NUCLEAR when it
fails, instead of piling yet another error reporting variable that
is specific to the feature.
This is all about code maintainability, which is very different from
who cares about error reporting.
diff --git a/remote.h b/remote.h
index 8b62efd..f346524 100644
--- a/remote.h
+++ b/remote.h
@@ -115,7 +115,8 @@ struct ref {
REF_STATUS_REJECT_SHALLOW,
REF_STATUS_UPTODATE,
REF_STATUS_REMOTE_REJECT,
- REF_STATUS_EXPECTING_REPORT
+ REF_STATUS_EXPECTING_REPORT,
+ REF_STATUS_ATOMIC_PUSH_FAILED
} status;
...
for (ref = remote_refs; ref; ref = ref-next) {
- if (no_ref_update_to_be_sent(ref, args))
+ int reject_reason;
+ if ((reject_reason = no_ref_update_to_be_sent(ref, args))) {
We avoid assignment inside a conditional.
+ /* When we know the server would reject a ref update if
+ * we were to send it and we're trying to send the refs
+ * atomically, abort the whole operation */
+ if (use_atomic reject_reason == 2)
+ return atomic_push_failure(args,
+remote_refs,
+ref);
continue;
Perhaps
switch (check_to_send_update(...)) {
case 0: /* no error */
break;
case -REF_STATUS_ATOMIC_PUSH_FAILED:
return atomic_push_failure(args, remote_refs, ref);
break;
default:
continue;
}
or something?
--
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: [PATCHv2 5/6] push.c: add an --atomic-push argument
On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller sbel...@google.com wrote:
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:
Changes v1 - v2
It's --atomic now! (dropping the -push)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..3feacc5 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n |
--dry-run] [--receive-pack=git-receive-pack]
s/atomic-push/atomic/
[--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.
+--atomic::
+ Use the an atomic transaction on the server side if available.
s/the an/an/
+ 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..fe0b8cc 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -518,6 +518,8 @@ 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_BIT(0, atomic, flags, N_(use a single atomic
transaction at the serverside, if available),
single atomic sounds awfully redundant.
serverside is odd. Perhaps server side or merely remote or remote side.
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
diff --git a/transport.c b/transport.c
index c67feee..5b29514 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_ATOMIC_PUSH);
args.url = transport-url;
ret = send_pack(args, data-fd, data-conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 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_ATOMIC_PUSH 4096
For consistency with existing names, should this be TRANSPORT_PUSH_ATOMIC?
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) -
gettext_width(x)), (x)
--
2.2.0.31.gad78000.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
bug patch: exit codes from internal commands are handled incorrectly
(Apologies if I’ve missed anything here - I’m still climbing the git learning
curve.)
Bug: exit codes from (at least) internal commands are handled incorrectly.
E.g. git-merge-file, docs say:
The exit value of this program is negative on error, and the number of
conflicts otherwise. If the merge was clean, the exit value is 0.
But only 8 bits get carried through exit, so 256 conflicts gives exit(0), which
means the merge was clean.
Issue shows up on:
git version 1.8.5.2 (Apple Git-48)
build from source (git version 2.2.0.68.g64396d6.dirty after patch below
applied)
Reproduce by:
Put the following test program in an empty directory as buggen.pl
TEST PROGRAM START
open OUTB, basefile or die;
print OUTB this is the base file\n;
close OUTB;
open OUT1, bfile1 or die;
open OUT2, bfile2 or die;
$c = shift;
while($c 0){
print OUT1 a\nb\nc\nd\nchange $c file 1\n;
print OUT2 a\nb\nc\nd\nchange $c file 2\n;
$c--;
}
TEST PROGRAM END
Do these tests:
perl buggen.pl 0
git merge-file -p bfile1 basefile bfile2
echo $status
0
perl buggen.pl 1
git merge-file -p bfile1 basefile bfile2
echo $status
1
perl buggen.pl 256
git merge-file -p bfile1 basefile bfile2
echo $status
0 ===OOPS
Proposed patches:
diff --git a/git.c b/git.c
index 82d7a1c..8228a3c 100644
--- a/git.c
+++ b/git.c
@@ -349,6 +349,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const
trace_argv_printf(argv, trace: built-in: git);
status = p-fn(argc, argv, prefix);
+ if (status 255)
+ status = 255; /* prevent exit() from truncating to 0 */
if (status)
return status;
diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index d2fc12e..76e6a11 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -41,7 +41,8 @@ lines from `other-file`, or lines from both respectively. T
conflict markers can be given with the `--marker-size` option.
The exit value of this program is negative on error, and the number of
-conflicts otherwise. If the merge was clean, the exit value is 0.
+conflicts otherwise (but 255 will be reported even if more than 255 conflicts
+exist). If the merge was clean, the exit value is 0.
'git merge-file' is designed to be a minimal clone of RCS 'merge'; that is, it
implements all of RCS 'merge''s functionality which is needed by
Open questions:
1) Is 255 safe for all operating systems?
2) Does this issue affect any other places?
Thanks,
Keni
k...@his.com
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
Stefan Beller sbel...@google.com writes:
* update(...) assumes to be always in a transaction
* Caring about when to begin/commit transactions is put
into execute_commands
I am obviously biased, but I find that the new code structure and
separation of responsibility between update() and execute()
functions a lot clearer than the previous one.
Thanks.
builtin/receive-pack.c | 64
++
1 file changed, 49 insertions(+), 15 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e76e5d5..0803fd2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -67,6 +67,8 @@ static const char *NONCE_SLOP = SLOP;
static const char *nonce_status;
static long nonce_stamp_slop;
static unsigned long nonce_stamp_slop_limit;
+struct strbuf err = STRBUF_INIT;
+struct ref_transaction *transaction;
static enum deny_action parse_deny_action(const char *var, const char *value)
{
@@ -832,34 +834,32 @@ 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;
}
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 */
}
@@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = transaction error;
+ return;
+ }
+ }
data.cmds = commands;
data.si = si;
if (check_everything_connected(iterate_receive_command_list, 0, data))
@@ -1086,8 +1096,23 @@ static void execute_commands(struct command *commands,
if (cmd-skip_update)
continue;
-
+ if (!use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ rp_error(%s, err.buf);
+ strbuf_release(err);
+ cmd-error_string = failed to start
transaction;
+ }
+ }
cmd-error_string = update(cmd, si);
+ if (!use_atomic)
+ if (ref_transaction_commit(transaction, err)) {
+ ref_transaction_free(transaction);
+ rp_error(%s, err.buf);
+ strbuf_release(err);
+ cmd-error_string = failed to update ref;
+ }
+
if (shallow_update !cmd-error_string
si-shallow_ref[cmd-index]) {
error(BUG: connectivity check has not been run on ref
%s,
@@ -1096,6 +1121,14 @@ static void execute_commands(struct command *commands,
}
}
+ if (use_atomic) {
+ if
Re: [PATCHv2 5/6] push.c: add an --atomic-push argument
Stefan Beller sbel...@google.com writes:
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:
Changes v1 - v2
It's --atomic now! (dropping the -push)
Also from the subject line ;-)
Documentation/git-push.txt | 7 ++-
builtin/push.c | 2 ++
transport.c| 1 +
transport.h| 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 21b3f29..3feacc5 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -9,7 +9,7 @@ git-push - Update remote refs along with associated objects
SYNOPSIS
[verse]
-'git push' [--all | --mirror | --tags] [--follow-tags] [-n | --dry-run]
[--receive-pack=git-receive-pack]
+'git push' [--all | --mirror | --tags] [--follow-tags] [--atomic-push] [-n |
--dry-run] [--receive-pack=git-receive-pack]
[--repo=repository] [-f | --force] [--prune] [-v | --verbose]
[-u | --set-upstream] [--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.
+--atomic::
+ Use the an atomic transaction on the server 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..fe0b8cc 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -518,6 +518,8 @@ 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_BIT(0, atomic, flags, N_(use a single atomic
transaction at the serverside, if available),
+ TRANSPORT_ATOMIC_PUSH),
OPT_END()
};
diff --git a/transport.c b/transport.c
index c67feee..5b29514 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_ATOMIC_PUSH);
args.url = transport-url;
ret = send_pack(args, data-fd, data-conn, remote_refs,
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 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_ATOMIC_PUSH 4096
#define TRANSPORT_SUMMARY_WIDTH (2 * DEFAULT_ABBREV + 3)
#define TRANSPORT_SUMMARY(x) (int)(TRANSPORT_SUMMARY_WIDTH + strlen(x) -
gettext_width(x)), (x)
--
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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller sbel...@google.com wrote:
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
---
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
new file mode 100755
index 000..6354fc0
--- /dev/null
+++ b/t/t5543-atomic-push.sh
@@ -0,0 +1,176 @@
+#!/bin/sh
+
+test_description='pushing to a repository using the atomic push option'
+
+. ./test-lib.sh
+
+D=`pwd`
Junio already mentioned this previously, but this should be $(pwd)
rather than `pwd`.
However, more importantly, why is this variable declared at all since
it is never used by the script?
+mk_repo_pair () {
+ rm -rf workbench upstream
+ test_create_repo upstream
+ test_create_repo workbench
+ (
+ cd upstream git config receive.denyCurrentBranch warn
This would be slightly easier to read if split over two lines.
+ )
+ (
+ cd workbench git remote add up ../upstream
Ditto.
+ )
+}
+
--
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
confusion with some `git reset` commands
Hi, From the command help I see - [arup@to_do_app]$ git reset -h usage: git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [commit] or: git reset [-q] tree-ish [--] paths... or: git reset --patch [tree-ish] [--] [paths...] -q, --quiet be quiet, only report errors --mixed reset HEAD and index --softreset only HEAD --hardreset HEAD, index and working tree --merge reset HEAD, index and working tree --keepreset HEAD but keep local changes -p, --patch select hunks interactively But I am looking for any differences - a) git reset --soft and git reset --keep b) git reset --hard and git reset --merge -- Regards, Arup Rakshit Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it. --Brian Kernighan -- 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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
Stefan Beller sbel...@google.com writes:
+test_description='pushing to a repository using the atomic push option'
+
+. ./test-lib.sh
+
+D=`pwd`
$(pwd)?
+mk_repo_pair () {
+ rm -rf workbench upstream
+ test_create_repo upstream
+ test_create_repo workbench
+ (
+ cd upstream git config receive.denyCurrentBranch warn
+ )
I was wondering how you would do this part after suggesting use of
test_create_repo, knowing very well that one of them was a bare one
;-).
We might want to extend test_create_repo to allow creating a bare
repository, but this is also OK.
+ (
+ cd workbench git remote add up ../upstream
+ )
+}
+# refname, expected value, e.g.
+# test_ref_upstream refs/heads/master HEADS{0}
+test_ref_upstream () {
+ test $# == 2 # if this fails, we have a bug in this script.
This is not C; test $# = 2 (notice a single equal sign). And you
do not need dq-pair around these.
+ test $(git -C upstream rev-parse --verify $1) == $2
+}
Seeing that all callers of test_ref_upstream computes $2 as
git -C workbench rev-parse --verify something
I have a feeling that
+ test_ref_upstream second second
would be easier for them to write than
+ test_ref_upstream second $(git -C workbench rev-parse --verify second)
That is
# refname in upstream and expected value from workbench
# E.g. test_ref_upstream master HEAD makes sure that HEAD in
# workbench matches the master branch in upstream repository.
test_ref_upstream () {
test $# = 2
test $(git -C upstream rev-parse --verify $1) == \
$(git -C workbench rev-parse --verify $2)
}
or something. We may however want to do the usual
test $# = 2
git -C upstream rev-parse --verify $1 expect
git -C workbench rev-parse --verify $2 actual
test_cmp expect actual
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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano gits...@pobox.com wrote:
Seeing that all callers of test_ref_upstream computes $2 as
git -C workbench rev-parse --verify something
Only in the first tests, where this should be the case after push.
In the failure tests, we go with HEAD@{N} which needs to be computed
inside the workbench repo.
Alternatively we could check if the reflog of upstream has a certain
number of entries
which would indicate the push was not recorded (i.e. not performed?)
I think we should keep it similar to this one.
--
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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
Stefan Beller sbel...@google.com writes: 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. It is tricky to arrange a test to fail a fast-forward check on the receiver side, because the local test is done by reading from the other side, not relying on your remote tracking branches. The usual flow is: pusher says to the receiver I want to push receiver says to the pusher Here are the tips of my refs pusher thinks: Ah, I was about to update their master branch with this commit, but what they have is (1) not known to me so by definition I cannot fast-forward, or (2) known to me and I can definitely tell I am not fast-forwarding it, so I'd locally fail this push. You need to invent a way to successfully race with an ongoing push. After receiver gives the tips of its refs (all of which are ancestors of what is going to be pushed) but before the pusher finishes the thinking above, you would somehow update the refs at the receiver so that the push will not fast-forward. Such a raced flow would look like: pusher says to the receiver I want to push receiver says to the pusher Here are the tips of my refs pusher thinks: OK, everything I'll send will fast-forward pusher thinks: Let's start generating a packfile you intervene and update receiver's 'master' at this point. pusher send a pack and tells the receiver I want to update your master from OLD to NEW. receiver thinks: Huh, that OLD is not where my 'master' is recevier says to the pusher No fast-forward. But I do not think it is practical to run such a test. Rejecting on the receiver's end using pre-receive or update hook should be testable and should be tested. Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 4/6] receive-pack.c: use a single ref_transaction for atomic pushes
On Tue, Dec 16, 2014 at 2:29 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, Dec 16, 2014 at 1:49 PM, Stefan Beller sbel...@google.com wrote:
---
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e76e5d5..0803fd2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1059,6 +1059,16 @@ static void execute_commands(struct command *commands,
return;
}
+ if (use_atomic) {
+ transaction = ref_transaction_begin(err);
+ if (!transaction) {
+ error(%s, err.buf);
+ strbuf_release(err);
+ for (cmd = commands; cmd; cmd = cmd-next)
+ cmd-error_string = transaction error;
+ return;
+ }
+ }
Not seen in this diff, but just below this point, the pre-receive hook
is invoked. If it declines, then execute_commands() returns without
releasing the transaction which was begun here. Is that correct
behavior?
For robustness, it might also be sane to release the 'err' strbuf at
this early return (though the current code does not strictly leak it).
To clarify: By this early return, I mean the early return taken when
pre-receive hook declines.
--
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] receive-pack: refuse all commands if one fails in atomic mode
Stefan Beller sbel...@google.com writes:
This patch will be incorporated into the right places in v3 of the series.
Signed-off-by: Stefan Beller sbel...@google.com
---
Notes:
I don't want to resend the patch series today to accumulate comments,
but this makes the last test pass, which is the whole point of the series.
I'll put it into the right places in a reroll.
builtin/receive-pack.c | 13 -
t/t5543-atomic-push.sh | 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 0803fd2..3477f7c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1122,7 +1122,18 @@ static void execute_commands(struct command *commands,
}
if (use_atomic) {
- if (ref_transaction_commit(transaction, err)) {
+ /* 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. */
/*
* The first line of our multi-line comment
* has only opening slash-asterisk and nothing else.
* The last line has asterisk-slash and nothing else.
*/
+ for (cmd = commands; cmd; cmd = cmd-next)
+ if (cmd-error_string)
+ break;
+ if (cmd) {
+ for (cmd = commands; cmd; cmd = cmd-next)
+ if (!cmd-error_string)
+ cmd-error_string = atomic push
failure;
+ } else if (ref_transaction_commit(transaction, err)) {
rp_error(%s, err.buf);
for (cmd = commands; cmd; cmd = cmd-next)
cmd-error_string = err.buf;
Makes sense.
diff --git a/t/t5543-atomic-push.sh b/t/t5543-atomic-push.sh
index 6354fc0..f0e54d9 100755
--- a/t/t5543-atomic-push.sh
+++ b/t/t5543-atomic-push.sh
@@ -142,7 +142,7 @@ test_expect_success 'atomic push fails if one tag fails
remotely' '
test_ref_upstream second $(git -C workbench rev-parse --verify HEAD@{1})
'
-test_expect_failure 'atomic push obeys update hook preventing a branch to be
pushed' '
+test_expect_success 'atomic push obeys update hook preventing a branch to be
pushed' '
mk_repo_pair
(
cd workbench
--
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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
On Tue, Dec 16, 2014 at 12:30 PM, Junio C Hamano gits...@pobox.com wrote: Stefan Beller sbel...@google.com writes: 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. It is tricky to arrange a test to fail a fast-forward check on the receiver side, because the local test is done by reading from the other side, not relying on your remote tracking branches. The usual flow is: pusher says to the receiver I want to push receiver says to the pusher Here are the tips of my refs pusher thinks: Ah, I was about to update their master branch with this commit, but what they have is (1) not known to me so by definition I cannot fast-forward, or (2) known to me and I can definitely tell I am not fast-forwarding it, so I'd locally fail this push. You need to invent a way to successfully race with an ongoing push. After receiver gives the tips of its refs (all of which are ancestors of what is going to be pushed) but before the pusher finishes the thinking above, you would somehow update the refs at the receiver so that the push will not fast-forward. Such a raced flow would look like: pusher says to the receiver I want to push receiver says to the pusher Here are the tips of my refs pusher thinks: OK, everything I'll send will fast-forward pusher thinks: Let's start generating a packfile you intervene and update receiver's 'master' at this point. pusher send a pack and tells the receiver I want to update your master from OLD to NEW. receiver thinks: Huh, that OLD is not where my 'master' is recevier says to the pusher No fast-forward. But I do not think it is practical to run such a test. Rejecting on the receiver's end using pre-receive or update hook should be testable and should be tested. Thanks. Yes, that was my understanding as well. I just messed up mentally. Now that the update hook test is in place, we do have tests from the local and the remote side rejecting, so it should be fine now. -- 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: [PATCHv2 5/6] push.c: add an --atomic-push argument
Eric Sunshine sunsh...@sunshineco.com writes:
+--atomic::
+ Use the an atomic transaction on the server side if available.
s/the an/an/
...
+ OPT_BIT(0, atomic, flags, N_(use a single atomic
transaction at the serverside, if available),
single atomic sounds awfully redundant.
serverside is odd. Perhaps server side or merely remote or remote
side.
...
diff --git a/transport.h b/transport.h
index 3e0091e..25fa1da 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_ATOMIC_PUSH 4096
For consistency with existing names, should this be TRANSPORT_PUSH_ATOMIC?
As always, thanks for a careful reading. I missed everything you
pointed out (I agree with 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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
Stefan Beller sbel...@google.com writes:
On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano gits...@pobox.com wrote:
Seeing that all callers of test_ref_upstream computes $2 as
git -C workbench rev-parse --verify something
Only in the first tests, where this should be the case after push.
In the failure tests, we go with HEAD@{N} which needs to be computed
inside the workbench repo.
Aren't we saying the same thing? The suggested alternative is to do
the git -C workbench rev-parse bit inside the helper.
--
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: [PATCHv2 6/6] t5543-atomic-push.sh: add basic tests for atomic pushes
On Tue, Dec 16, 2014 at 12:46 PM, Junio C Hamano gits...@pobox.com wrote:
Stefan Beller sbel...@google.com writes:
On Tue, Dec 16, 2014 at 11:46 AM, Junio C Hamano gits...@pobox.com wrote:
Seeing that all callers of test_ref_upstream computes $2 as
git -C workbench rev-parse --verify something
Only in the first tests, where this should be the case after push.
In the failure tests, we go with HEAD@{N} which needs to be computed
inside the workbench repo.
Aren't we saying the same thing? The suggested alternative is to do
the git -C workbench rev-parse bit inside the helper.
Now I am saying the same. (I finally understood what you were saying)
Will incorporate in a reroll.
--
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-compat-util: suppress unavoidable Apple-specific deprecation warnings
With the release of Mac OS X 10.7 in July 2011, Apple deprecated all openssl.h functionality due to OpenSSL ABI (application binary interface) instability, resulting in an explosion of compilation warnings about deprecated SSL, SHA1, and X509 functions (among others). 61067954ce (cache.h: eliminate SHA-1 deprecation warnings on Mac OS X; 2013-05-19) and be4c828b76 (imap-send: eliminate HMAC deprecation warnings on Mac OS X; 2013-05-19) attempted to ameliorate the situation by taken advantage of drop-in replacement functionality provided by Apple's (ABI-stable) CommonCrypto facility, however CommonCrypto supplies only a subset of deprecated OpenSSL functionality, thus a host of warnings remain. Despite this shortcoming, it was hoped that Apple would ultimately provide CommonCrypto replacements for all deprecated OpenSSL functionality, and that the effort started by 61067954ce and be4c828b76 would be continued and eventually eliminate all deprecation warnings. However, now 3.5 years later, and with Mac OS X at 10.10, the hoped-for CommonCrypto replacements have not yet materialized, nor is there any indication that they will be forthcoming. These Apple-specific warnings are pure noise: they don't tell us anything useful and we have no control over them, nor is Apple likely to provide replacements any time soon. Such noise may obscure other legitimate warnings, therefore silence them. Signed-off-by: Eric Sunshine sunsh...@sunshineco.com --- Related discussion: http://thread.gmane.org/gmane.comp.version-control.git/260463/ git-compat-util.h | 4 1 file changed, 4 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 400e921..433b8f2 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -211,8 +211,12 @@ extern char *gitbasename(char *); #endif #ifndef NO_OPENSSL +#define __AVAILABILITY_MACROS_USES_AVAILABILITY 0 +#define MAC_OS_X_VERSION_MIN_REQUIRED MAC_OS_X_VERSION_10_6 #include openssl/ssl.h #include openssl/err.h +#undef MAC_OS_X_VERSION_MIN_REQUIRED +#undef __AVAILABILITY_MACROS_USES_AVAILABILITY #endif /* On most systems netdb.h would have given us this, but -- 2.2.0.209.gd6426a0 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Update documentation occurrences of filename .sh
Peter van der Does wrote: Documentation in the completion scripts for Bash and Zsh state the wrong filenames. Signed-off-by: Peter van der Does pe...@avirtualhome.com --- contrib/completion/git-completion.bash | 4 ++-- contrib/completion/git-completion.zsh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) Reviewed-by: Jonathan Nieder jrnie...@gmail.com Thanks. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: hooks scripts and noexec partition
On Sun, Dec 14, 2014 at 02:44:35AM +0100, krz...@gmail.com wrote: Thanks for the patch, however it is not working (no change, hooks still dont work on noexec partition). Since I see that you are fluent in git code and C can you by any chance tell me how to modify run-command.c to make git run hooks as: /bin/sh hook_path ? I do not think that is a smart thing to do in general, as there is no guarantee that the hook is in fact a shell script (and not a binary, or some other scripting language). But if you want do a one-off patch for yourself, knowing that you will only use shell scripts, it is probably something like: diff --git a/run-command.c b/run-command.c index a476999..ccfccf0 100644 --- a/run-command.c +++ b/run-command.c @@ -812,6 +812,7 @@ int run_hook_ve(const char *const *env, const char *name, va_list args) if (!p) return 0; + argv_array_push(hook.args, /bin/sh); argv_array_push(hook.args, p); while ((p = va_arg(args, const char *))) argv_array_push(hook.args, p); -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
Saving space/network on common repos
At Khan Academy, we are running a Jenkins installation as our build server. By design, our Jenkins machine has several different directories that each hold a copy of the same git repository. (For instance, Jenkins may be running tests on our repo at several different commits at the same time.) When Jenkins decides to run a test -- I'm simplifying a bit -- it will pick one of the copies of the repo, do a 'git fetch origin git checkout some commit' and the run the tests. Our repo has a lot of churn and some big files, and this git fetch can take a long time. I'd like to reduce both the time to fetch and the disk space used by sharing objects between the repo copies. My research has turned up three techniques that try to address this use case: * git clone --reference * git clone --shared * git clone local repo, which creates hard links I can probably use any of these approaches, but git clone --reference would be the easiest to set up. I would do so by creating a 'cache' repo that is just created to serve as a reference and not used in any other way, so I wouldn't have to worry about the dangers with pruning, accidentally deleting the repo, etc. My big concern is that all these methods seem to just affect clone. So: Question 1) If I do 'git clone --reference, will the reference repo be used for subsequent fetches as well? What about 'git clone --shared'? Question 2) If I git clone a local repo, will subsequent fetches also create hard links? Question 3) If the answer to any of the above is yes, how does this work with packing? Say I pack the reference repo (being careful not to prune anything). Will subsequent fetches still be able to get the objects they need from the reference repo? An added complication is submodules. We have a submodule that is as big and slow to fetch as our main repository. Question 4) Is there a practical way to set up submodules so they can use the same object-sharing framework that the main repo does? I'm not keen on rewriting .gitmodules in each of my repos, so probably something that uses info/alternates is the most workable. I have a scheme for setting that up that maybe will work, but it's a moot point if info/alternates doesn't work for fetching. I'm wondering if the best approach for us might be to use GIT_OBJECT_DIRECTORY: set GIT_OBJECT_DIRECTORY to the shared cached directory for each of our repos, so they all fetch to the same place. Question 5) I haven't seen this mentioned anywhere else, so I'm guessing it won't work. Am I missing a big problem? Question 6) Will git be sad if two different repos that share an object directory, both do 'git fetch' at the same time? I could maybe protect fetches with an flock, but jenkins can do git operations behind my back so it would be easier if I didn't have to worry about locking. Question 7) Is GIT_OBJECT_DIRECTORY supposed to work with subrepos? In my experimentation, it looks like it doesn't: when I run 'GIT_OBJECT_DIRECTORY=../obj git submodule update --init' it still puts the objects in .git/modules/submodule/objects/. Is this a bug? Is there any way to work around it? Any suggestions would be appreciated! It feels to me like this is something that git should support pretty easily given its architecture, but I just don't see a way to do it. Thanks, craig -- 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
