Re: [PATCH] rerere: make rr-cache fanout directory honor umask
On Mon, Jul 09, 2012 at 04:28:21PM -0700, Junio C Hamano wrote: This is the last remaining call to mkdir(2) that restricts the permission bits by passing 0755. Just use the same mkdir_in_gitdir() used to create the leaf directories. Signed-off-by: Junio C Hamano gits...@pobox.com Looks obviously correct to me. I notice that grepping finds a few 0644 modes, too. Most of them are false-positives (e.g., we store and transmit 100644 as a shorthand for normal non-executable permissions). This is the only one that looked legitimate to me: -- 8 -- Subject: [PATCH] add: create ADD_EDIT.patch with mode 0666 We should be letting the user's umask take care of restricting permissions. Even though this is a temporary file and probably nobody would notice, this brings us in line with other temporary file creations in git (e.g., choosing edit from git-add--interactive). Signed-off-by: Jeff King p...@peff.net --- builtin/add.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/add.c b/builtin/add.c index 41edd63..815ac4b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -287,7 +287,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix) argc = setup_revisions(argc, argv, rev, NULL); rev.diffopt.output_format = DIFF_FORMAT_PATCH; DIFF_OPT_SET(rev.diffopt, IGNORE_DIRTY_SUBMODULES); - out = open(file, O_CREAT | O_WRONLY, 0644); + out = open(file, O_CREAT | O_WRONLY, 0666); if (out 0) die (_(Could not open '%s' for writing.), file); rev.diffopt.file = xfdopen(out, w); -- 1.7.10.5.16.ga1c6f1c -- To unsubscribe from this list: send the line 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: Problems pushing???
Thanks for all your help. Running through this a few more times and reading the net I managed it ; ) I tested it cloning from the command line and all seems good. For developement I use eclipse with the eGit plugin. When I try cloning it putting all the relevant info in it fails. I am sure this is due to the ssh keys. I have set-up my keys accordingly. Tried using the ssh agent but with no success. I can see the agent is running and tested that and all seems good with that. The error get is ssh://paul@localhost:22: org.eclipse.jgit.transport.CredentialItem$StringType:Password: ssh://paul@localhost:22: org.eclipse.jgit.transport.CredentialItem$StringType:Password: Hope you can help -- View this message in context: http://git.661346.n2.nabble.com/Problems-pushing-tp7562695p7562762.html Sent from the git mailing list archive at Nabble.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
[PATCH v4 00/19] git apply --3way
With finishing touches (mostly updates to in-code comments and log messages). Previous ones were: http://thread.gmane.org/gmane.comp.version-control.git/197538 http://thread.gmane.org/gmane.comp.version-control.git/197637 http://thread.gmane.org/gmane.comp.version-control.git/199936 Teach git apply a similar -3way merge fallback option with this series, and you can now apply your patches without having to reduce context. As it will leave the conflicted halves in the index and let you manually resolve conflicts in the working tree, --3way implies --index, and cannot be used with --cached or --reject. I've been using this in my regular workflow, which involves a fair amount of git diff P.diff later followed by git apply P.diff when flipping patches in topics around, and things seem to work OK. Junio C Hamano (19): apply: fix an incomplete comment in check_patch() apply: a bit more comments on PATH_TO_BE_DELETED apply: clear_image() clears things a bit more apply: refactor read_file_or_gitlink() apply: factor out checkout_target() helper function apply: split load_preimage() helper function out apply: refactor previous patch logic apply: further split load_preimage() apply: move check_to_create_blob() closer to its sole caller apply: move already exists logic to check_to_create() apply: accept -3/--3way command line option apply: fall back on three-way merge apply: plug the three-way merge logic in apply: move verify_index_match() higher apply: --3way with add/add conflict apply: register conflicted stages to the index apply: allow rerere() to work on --3way results apply: document --3way option apply: tests for the --3way option Documentation/git-apply.txt | 11 +- builtin/apply.c | 557 ++-- t/t4108-apply-threeway.sh | 157 + t/t4117-apply-reject.sh | 8 + 4 files changed, 615 insertions(+), 118 deletions(-) create mode 100755 t/t4108-apply-threeway.sh -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line 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 01/19] apply: fix an incomplete comment in check_patch()
This check is not only about type-change (for which it would be sufficient to check only was_deleted()) but is also about a swap rename. Otherwise to_be_deleted() check is not justified. Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 24 +++- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 725712d..44f6de9 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -3218,16 +3218,22 @@ static int check_patch(struct patch *patch) return status; old_name = patch-old_name; + /* +* A type-change diff is always split into a patch to delete +* old, immediately followed by a patch to create new (see +* diff.c::run_diff()); in such a case it is Ok that the entry +* to be deleted by the previous patch is still in the working +* tree and in the index. +* +* A patch to swap-rename between A and B would first rename A +* to B and then rename B to A. While applying the first one, +* the presense of B should not stop A from getting renamed to +* B; ask to_be_deleted() about the later rename. Removal of +* B and rename from A to B is handled the same way by asking +* was_deleted(). +*/ if ((tpatch = in_fn_table(new_name)) - (was_deleted(tpatch) || to_be_deleted(tpatch))) - /* -* A type-change diff is always split into a patch to -* delete old, immediately followed by a patch to -* create new (see diff.c::run_diff()); in such a case -* it is Ok that the entry to be deleted by the -* previous patch is still in the working tree and in -* the index. -*/ + (was_deleted(tpatch) || to_be_deleted(tpatch))) ok_if_exists = 1; else ok_if_exists = 0; -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 02/19] apply: a bit more comments on PATH_TO_BE_DELETED
The code is littered with to_be_deleted() whose purpose is not so clear. Describe where it matters. Also remove an extra space before #define that snuck in by mistake at 7fac0ee (builtin-apply: keep information about files to be deleted, 2009-04-11). Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index 44f6de9..35460c9 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -2970,9 +2970,15 @@ static struct patch *in_fn_table(const char *name) * item-util in the filename table records the status of the path. * Usually it points at a patch (whose result records the contents * of it after applying it), but it could be PATH_WAS_DELETED for a - * path that a previously applied patch has already removed. + * path that a previously applied patch has already removed, or + * PATH_TO_BE_DELETED for a path that a later patch would remove. + * + * The latter is needed to deal with a case where two paths A and B + * are swapped by first renaming A to B and then renaming B to A; + * moving A to B should not be prevented due to presense of B as we + * will remove it in a later patch. */ - #define PATH_TO_BE_DELETED ((struct patch *) -2) +#define PATH_TO_BE_DELETED ((struct patch *) -2) #define PATH_WAS_DELETED ((struct patch *) -1) static int to_be_deleted(struct patch *patch) -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 03/19] apply: clear_image() clears things a bit more
The clear_image() function did not clear the line table in the image
structure; this does not matter for the current callers, as the function
is only called from the codepaths that deal with binary patches where the
line table is never populated, and the codepaths that do populate the line
table free it themselves.
But it will start to matter when we introduce a codepath to retry a failed
patch, so make sure it clears and frees everything.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 35460c9..09f5df3 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -371,8 +371,8 @@ static void prepare_image(struct image *image, char *buf,
size_t len,
static void clear_image(struct image *image)
{
free(image-buf);
- image-buf = NULL;
- image-len = 0;
+ free(image-line_allocated);
+ memset(image, 0, sizeof(*image));
}
/* fmt must contain _one_ %s and no other substitution */
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line 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 04/19] apply: refactor read_file_or_gitlink()
Reading a blob out of the object store does not have to require that the
caller has a cache entry for it.
Create a read_blob_object() helper function that takes the object name and
mode, and use it to reimplement the original function as a thin wrapper to
it.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 18 +++---
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 09f5df3..15bcbb0 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2930,20 +2930,17 @@ static int apply_fragments(struct image *img, struct
patch *patch)
return 0;
}
-static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)
+static int read_blob_object(struct strbuf *buf, const unsigned char *sha1,
unsigned mode)
{
- if (!ce)
- return 0;
-
- if (S_ISGITLINK(ce-ce_mode)) {
+ if (S_ISGITLINK(mode)) {
strbuf_grow(buf, 100);
- strbuf_addf(buf, Subproject commit %s\n,
sha1_to_hex(ce-sha1));
+ strbuf_addf(buf, Subproject commit %s\n, sha1_to_hex(sha1));
} else {
enum object_type type;
unsigned long sz;
char *result;
- result = read_sha1_file(ce-sha1, type, sz);
+ result = read_sha1_file(sha1, type, sz);
if (!result)
return -1;
/* XXX read_sha1_file NUL-terminates */
@@ -2952,6 +2949,13 @@ static int read_file_or_gitlink(struct cache_entry *ce,
struct strbuf *buf)
return 0;
}
+static int read_file_or_gitlink(struct cache_entry *ce, struct strbuf *buf)
+{
+ if (!ce)
+ return 0;
+ return read_blob_object(buf, ce-sha1, ce-ce_mode);
+}
+
static struct patch *in_fn_table(const char *name)
{
struct string_list_item *item;
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line 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 05/19] apply: factor out checkout_target() helper function
When a patch wants to touch a path, if the path exists in the index
but is missing in the working tree, git apply --index checks out
the file to the working tree from the index automatically and then
applies the patch.
Split this logic out to a separate helper function.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 15bcbb0..487e403 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3034,6 +3034,18 @@ static void prepare_fn_table(struct patch *patch)
}
}
+static int checkout_target(struct cache_entry *ce, struct stat *st)
+{
+ struct checkout costate;
+
+ memset(costate, 0, sizeof(costate));
+ costate.base_dir = ;
+ costate.refresh_cache = 1;
+ if (checkout_entry(ce, costate, NULL) || lstat(ce-name, st))
+ return error(_(cannot checkout %s), ce-name);
+ return 0;
+}
+
static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
{
struct strbuf buf = STRBUF_INIT;
@@ -3163,13 +3175,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
}
*ce = active_cache[pos];
if (stat_ret 0) {
- struct checkout costate;
- /* checkout */
- memset(costate, 0, sizeof(costate));
- costate.base_dir = ;
- costate.refresh_cache = 1;
- if (checkout_entry(*ce, costate, NULL) ||
- lstat(old_name, st))
+ if (checkout_target(*ce, st))
return -1;
}
if (!cached verify_index_match(*ce, st))
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 06/19] apply: split load_preimage() helper function out
Given a patch for a single path, the function apply_data() reads the
preimage in core, and applies the change represented in the patch.
Separate out the first part that reads the preimage into a separate
helper function load_preimage().
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 487e403..4d2546f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3046,10 +3046,10 @@ static int checkout_target(struct cache_entry *ce,
struct stat *st)
return 0;
}
-static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
+static int load_preimage(struct image *image,
+struct patch *patch, struct stat *st, struct
cache_entry *ce)
{
struct strbuf buf = STRBUF_INIT;
- struct image image;
size_t len;
char *img;
struct patch *tpatch;
@@ -3086,7 +3086,16 @@ static int apply_data(struct patch *patch, struct stat
*st, struct cache_entry *
}
img = strbuf_detach(buf, len);
- prepare_image(image, img, len, !patch-is_binary);
+ prepare_image(image, img, len, !patch-is_binary);
+ return 0;
+}
+
+static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
+{
+ struct image image;
+
+ if (load_preimage(image, patch, st, ce) 0)
+ return -1;
if (apply_fragments(image, patch) 0)
return -1; /* note with --reject this succeeds. */
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 07/19] apply: refactor previous patch logic
The code to grab the result of application of a previous patch in the
input was mixed with error message generation for a case where a later
patch tries to modify contents of a path that has been removed.
The same code is duplicated elsewhere in the code. Introduce a helper
to clarify what is going on.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 82 +++--
1 file changed, 56 insertions(+), 26 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 4d2546f..5642433 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3046,22 +3046,50 @@ static int checkout_target(struct cache_entry *ce,
struct stat *st)
return 0;
}
+static struct patch *previous_patch(struct patch *patch, int *gone)
+{
+ struct patch *previous;
+
+ *gone = 0;
+ if (patch-is_copy || patch-is_rename)
+ return NULL; /* git patches do not depend on the order */
+
+ previous = in_fn_table(patch-old_name);
+ if (!previous)
+ return NULL;
+
+ if (to_be_deleted(previous))
+ return NULL; /* the deletion hasn't happened yet */
+
+ if (was_deleted(previous))
+ *gone = 1;
+
+ return previous;
+}
+
+/*
+ * We are about to apply patch; populate the image with the
+ * current version we have, from the working tree or from the index,
+ * depending on the situation e.g. --cached/--index. If we are
+ * applying a non-git patch that incrementally updates the tree,
+ * we read from the result of a previous diff.
+ */
static int load_preimage(struct image *image,
struct patch *patch, struct stat *st, struct
cache_entry *ce)
{
struct strbuf buf = STRBUF_INIT;
size_t len;
char *img;
- struct patch *tpatch;
+ struct patch *previous;
+ int status;
- if (!(patch-is_copy || patch-is_rename)
- (tpatch = in_fn_table(patch-old_name)) != NULL
!to_be_deleted(tpatch)) {
- if (was_deleted(tpatch)) {
- return error(_(patch %s has been renamed/deleted),
- patch-old_name);
- }
+ previous = previous_patch(patch, status);
+ if (status)
+ return error(_(path %s has been renamed/deleted),
+patch-old_name);
+ if (previous) {
/* We have a patched copy in memory; use that. */
- strbuf_add(buf, tpatch-result, tpatch-resultsize);
+ strbuf_add(buf, previous-result, previous-resultsize);
} else if (cached) {
if (read_file_or_gitlink(ce, buf))
return error(_(read of %s failed), patch-old_name);
@@ -3143,39 +3171,41 @@ static int verify_index_match(struct cache_entry *ce,
struct stat *st)
return ce_match_stat(ce, st,
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
}
+/*
+ * If patch that we are looking at modifies or deletes what we have,
+ * we would want it not to lose any local modification we have, either
+ * in the working tree or in the index.
+ *
+ * This also decides if a non-git patch is a creation patch or a
+ * modification to an existing empty file. We do not check the state
+ * of the current tree for a creation patch in this function; the caller
+ * check_patch() separately makes sure (and errors out otherwise) that
+ * the path the patch creates does not exist in the current tree.
+ */
static int check_preimage(struct patch *patch, struct cache_entry **ce, struct
stat *st)
{
const char *old_name = patch-old_name;
- struct patch *tpatch = NULL;
- int stat_ret = 0;
+ struct patch *previous = NULL;
+ int stat_ret = 0, status;
unsigned st_mode = 0;
- /*
-* Make sure that we do not have local modifications from the
-* index when we are looking at the index. Also make sure
-* we have the preimage file to be patched in the work tree,
-* unless --cached, which tells git to apply only in the index.
-*/
if (!old_name)
return 0;
assert(patch-is_new = 0);
+ previous = previous_patch(patch, status);
- if (!(patch-is_copy || patch-is_rename)
- (tpatch = in_fn_table(old_name)) != NULL !to_be_deleted(tpatch))
{
- if (was_deleted(tpatch))
- return error(_(%s: has been deleted/renamed),
old_name);
- st_mode = tpatch-new_mode;
+ if (status)
+ return error(_(path %s has been renamed/deleted), old_name);
+ if (previous) {
+ st_mode = previous-new_mode;
} else if (!cached) {
stat_ret = lstat(old_name, st);
if (stat_ret errno != ENOENT)
return error(_(%s: %s), old_name, strerror(errno));
}
- if (to_be_deleted(tpatch))
-
[PATCH v4 09/19] apply: move check_to_create_blob() closer to its sole caller
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 46 +++---
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index ee1a960..cd25e63 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3159,29 +3159,6 @@ static int apply_data(struct patch *patch, struct stat
*st, struct cache_entry *
return 0;
}
-static int check_to_create_blob(const char *new_name, int ok_if_exists)
-{
- struct stat nst;
- if (!lstat(new_name, nst)) {
- if (S_ISDIR(nst.st_mode) || ok_if_exists)
- return 0;
- /*
-* A leading component of new_name might be a symlink
-* that is going to be removed with this patch, but
-* still pointing at somewhere that has the path.
-* In such a case, path new_name does not exist as
-* far as git is concerned.
-*/
- if (has_symlink_leading_path(new_name, strlen(new_name)))
- return 0;
-
- return error(_(%s: already exists in working directory),
new_name);
- }
- else if ((errno != ENOENT) (errno != ENOTDIR))
- return error(%s: %s, new_name, strerror(errno));
- return 0;
-}
-
static int verify_index_match(struct cache_entry *ce, struct stat *st)
{
if (S_ISGITLINK(ce-ce_mode)) {
@@ -3272,6 +3249,29 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
return 0;
}
+static int check_to_create_blob(const char *new_name, int ok_if_exists)
+{
+ struct stat nst;
+ if (!lstat(new_name, nst)) {
+ if (S_ISDIR(nst.st_mode) || ok_if_exists)
+ return 0;
+ /*
+* A leading component of new_name might be a symlink
+* that is going to be removed with this patch, but
+* still pointing at somewhere that has the path.
+* In such a case, path new_name does not exist as
+* far as git is concerned.
+*/
+ if (has_symlink_leading_path(new_name, strlen(new_name)))
+ return 0;
+
+ return error(_(%s: already exists in working directory),
new_name);
+ }
+ else if ((errno != ENOENT) (errno != ENOTDIR))
+ return error(%s: %s, new_name, strerror(errno));
+ return 0;
+}
+
/*
* Check and apply the patch in-core; leave the result in patch-result
* for the caller to write it out to the final destination.
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 08/19] apply: further split load_preimage()
load_preimage() is very specific to grab the current contents for
the path given by patch-old_name. Split the logic that grabs the
contents for a path out of it into a separate load_patch_target()
function.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 59 ++---
1 file changed, 40 insertions(+), 19 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 5642433..ee1a960 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3067,6 +3067,31 @@ static struct patch *previous_patch(struct patch *patch,
int *gone)
return previous;
}
+#define SUBMODULE_PATCH_WITHOUT_INDEX 1
+
+static int load_patch_target(struct strbuf *buf,
+struct cache_entry *ce,
+struct stat *st,
+const char *name,
+unsigned expected_mode)
+{
+ if (cached) {
+ if (read_file_or_gitlink(ce, buf))
+ return error(_(read of %s failed), name);
+ } else if (name) {
+ if (S_ISGITLINK(expected_mode)) {
+ if (ce)
+ return read_file_or_gitlink(ce, buf);
+ else
+ return SUBMODULE_PATCH_WITHOUT_INDEX;
+ } else {
+ if (read_old_data(st, name, buf))
+ return error(_(read of %s failed), name);
+ }
+ }
+ return 0;
+}
+
/*
* We are about to apply patch; populate the image with the
* current version we have, from the working tree or from the index,
@@ -3090,26 +3115,22 @@ static int load_preimage(struct image *image,
if (previous) {
/* We have a patched copy in memory; use that. */
strbuf_add(buf, previous-result, previous-resultsize);
- } else if (cached) {
- if (read_file_or_gitlink(ce, buf))
+ } else {
+ status = load_patch_target(buf, ce, st,
+ patch-old_name, patch-old_mode);
+ if (status 0)
+ return status;
+ else if (status == SUBMODULE_PATCH_WITHOUT_INDEX) {
+ /*
+* There is no way to apply subproject
+* patch without looking at the index.
+* NEEDSWORK: shouldn't this be flagged
+* as an error???
+*/
+ free_fragment_list(patch-fragments);
+ patch-fragments = NULL;
+ } else if (status) {
return error(_(read of %s failed), patch-old_name);
- } else if (patch-old_name) {
- if (S_ISGITLINK(patch-old_mode)) {
- if (ce) {
- read_file_or_gitlink(ce, buf);
- } else {
- /*
-* There is no way to apply subproject
-* patch without looking at the index.
-* NEEDSWORK: shouldn't this be flagged
-* as an error???
-*/
- free_fragment_list(patch-fragments);
- patch-fragments = NULL;
- }
- } else {
- if (read_old_data(st, patch-old_name, buf))
- return error(_(read of %s failed),
patch-old_name);
}
}
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line 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 11/19] apply: accept -3/--3way command line option
Begin teaching the three-way merge fallback logic git am -3 uses
to the underlying git apply. It only implements the command line
parsing part, and does not do anything interesting yet, other than
making sure that --reject and --3way are not given together, and
making --3way imply --index.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 25 +++--
t/t4117-apply-reject.sh | 8
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 7379687..51b695b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -46,6 +46,7 @@ static int apply_with_reject;
static int apply_verbosely;
static int allow_overlap;
static int no_add;
+static int threeway;
static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned int p_context = UINT_MAX;
@@ -3139,6 +3140,12 @@ static int load_preimage(struct image *image,
return 0;
}
+static int try_threeway(struct image *image, struct patch *patch,
+ struct stat *st, struct cache_entry *ce)
+{
+ return -1; /* for now */
+}
+
static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
{
struct image image;
@@ -3146,8 +3153,11 @@ static int apply_data(struct patch *patch, struct stat
*st, struct cache_entry *
if (load_preimage(image, patch, st, ce) 0)
return -1;
- if (apply_fragments(image, patch) 0)
- return -1; /* note with --reject this succeeds. */
+ if (apply_fragments(image, patch) 0) {
+ /* Note: with --reject, apply_fragments() returns 0 */
+ if (!threeway || try_threeway(image, patch, st, ce) 0)
+ return -1;
+ }
patch-result = image.buf;
patch-resultsize = image.len;
add_to_fn_table(patch);
@@ -4079,6 +4089,8 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
apply a patch without touching the working tree),
OPT_BOOLEAN(0, apply, force_apply,
also apply the patch (use with
--stat/--summary/--check)),
+ OPT_BOOL('3', 3way, threeway,
+attempt three-way merge if a patch does not apply),
OPT_FILENAME(0, build-fake-ancestor, fake_ancestor,
build a temporary index based on embedded index
information),
{ OPTION_CALLBACK, 'z', NULL, NULL, NULL,
@@ -4127,6 +4139,15 @@ int cmd_apply(int argc, const char **argv, const char
*prefix_)
argc = parse_options(argc, argv, prefix, builtin_apply_options,
apply_usage, 0);
+ if (apply_with_reject threeway)
+ die(--reject and --3way cannot be used together.);
+ if (cached threeway)
+ die(--cached and --3way cannot be used together.);
+ if (threeway) {
+ if (is_not_gitdir)
+ die(_(--3way outside a repository));
+ check_index = 1;
+ }
if (apply_with_reject)
apply = apply_verbosely = 1;
if (!force_apply (diffstat || numstat || summary || check ||
fake_ancestor))
diff --git a/t/t4117-apply-reject.sh b/t/t4117-apply-reject.sh
index e9ccd16..8e15ecb 100755
--- a/t/t4117-apply-reject.sh
+++ b/t/t4117-apply-reject.sh
@@ -46,6 +46,14 @@ test_expect_success setup '
cat file1 saved.file1
'
+test_expect_success 'apply --reject is incompatible with --3way' '
+ test_when_finished cat saved.file1 file1
+ git diff patch.0
+ git checkout file1
+ test_must_fail git apply --reject --3way patch.0
+ git diff --exit-code
+'
+
test_expect_success 'apply without --reject should fail' '
if git apply patch.1
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line 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 12/19] apply: fall back on three-way merge
Grab the preimage blob the patch claims to be based on out of the object
store, apply the patch, and then call three-way-merge function. This step
still does not plug the actual three-way merge logic yet, but we are
getting there.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 46 +-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 51b695b..5a7201f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3140,10 +3140,54 @@ static int load_preimage(struct image *image,
return 0;
}
+static int three_way_merge(struct image *image,
+ char *path,
+ const unsigned char *base,
+ const unsigned char *ours,
+ const unsigned char *theirs)
+{
+ return -1; /* for now */
+}
+
static int try_threeway(struct image *image, struct patch *patch,
struct stat *st, struct cache_entry *ce)
{
- return -1; /* for now */
+ unsigned char pre_sha1[20], post_sha1[20], our_sha1[20];
+ struct strbuf buf = STRBUF_INIT;
+ size_t len;
+ char *img;
+ struct image tmp_image;
+
+ /* No point falling back to 3-way merge in these cases */
+ if (patch-is_new || patch-is_delete ||
+ S_ISGITLINK(patch-old_mode) || S_ISGITLINK(patch-new_mode))
+ return -1;
+
+ /* Preimage the patch was prepared for */
+ if (get_sha1(patch-old_sha1_prefix, pre_sha1) ||
+ read_blob_object(buf, pre_sha1, patch-old_mode))
+ return error(repository lacks the necessary blob to fall back
on 3-way merge.);
+ img = strbuf_detach(buf, len);
+ prepare_image(tmp_image, img, len, 1);
+ /* Apply the patch to get the post image */
+ if (apply_fragments(tmp_image, patch) 0) {
+ clear_image(tmp_image);
+ return -1;
+ }
+ /* post_sha1[] is theirs */
+ write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, post_sha1);
+ clear_image(tmp_image);
+
+ /* our_sha1[] is ours */
+ if (load_preimage(tmp_image, patch, st, ce))
+ return error(cannot read the current contents of '%s',
+patch-old_name);
+ write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_sha1);
+ clear_image(tmp_image);
+
+ /* in-core three-way merge between post and our using pre as base */
+ return three_way_merge(image,
+ patch-new_name, pre_sha1, our_sha1, post_sha1);
}
static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 13/19] apply: plug the three-way merge logic in
When a patch does not apply to what we have, but we know the preimage the
patch was made against, we apply the patch to the preimage to compute what
the patch author wanted the result to look like, and attempt a three-way
merge between the result and our version, using the intended preimage as
the base version.
When we are applying the patch using the index, we would additionally need
to add the object names of these three blobs involved in the merge, which
is not yet done in this step, but we add a field to struct patch so that
later write-out step can use it.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 53 ++---
1 file changed, 50 insertions(+), 3 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 5a7201f..d84958b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -16,6 +16,8 @@
#include dir.h
#include diff.h
#include parse-options.h
+#include xdiff-interface.h
+#include ll-merge.h
/*
* --check turns on checking that the working tree matches the
@@ -194,12 +196,16 @@ struct patch {
unsigned int is_copy:1;
unsigned int is_rename:1;
unsigned int recount:1;
+ unsigned int conflicted_threeway:1;
struct fragment *fragments;
char *result;
size_t resultsize;
char old_sha1_prefix[41];
char new_sha1_prefix[41];
struct patch *next;
+
+ /* three-way fallback result */
+ unsigned char threeway_stage[3][20];
};
static void free_fragment_list(struct fragment *list)
@@ -3146,7 +3152,29 @@ static int three_way_merge(struct image *image,
const unsigned char *ours,
const unsigned char *theirs)
{
- return -1; /* for now */
+ mmfile_t base_file, our_file, their_file;
+ mmbuffer_t result = { NULL };
+ int status;
+
+ read_mmblob(base_file, base);
+ read_mmblob(our_file, ours);
+ read_mmblob(their_file, theirs);
+ status = ll_merge(result, path,
+ base_file, base,
+ our_file, ours,
+ their_file, theirs, NULL);
+ free(base_file.ptr);
+ free(our_file.ptr);
+ free(their_file.ptr);
+ if (status 0 || !result.ptr) {
+ free(result.ptr);
+ return -1;
+ }
+ clear_image(image);
+ image-buf = result.ptr;
+ image-len = result.size;
+
+ return status;
}
static int try_threeway(struct image *image, struct patch *patch,
@@ -3155,6 +3183,7 @@ static int try_threeway(struct image *image, struct patch
*patch,
unsigned char pre_sha1[20], post_sha1[20], our_sha1[20];
struct strbuf buf = STRBUF_INIT;
size_t len;
+ int status;
char *img;
struct image tmp_image;
@@ -3167,6 +3196,9 @@ static int try_threeway(struct image *image, struct patch
*patch,
if (get_sha1(patch-old_sha1_prefix, pre_sha1) ||
read_blob_object(buf, pre_sha1, patch-old_mode))
return error(repository lacks the necessary blob to fall back
on 3-way merge.);
+
+ fprintf(stderr, Falling back to three-way merge...\n);
+
img = strbuf_detach(buf, len);
prepare_image(tmp_image, img, len, 1);
/* Apply the patch to get the post image */
@@ -3186,8 +3218,23 @@ static int try_threeway(struct image *image, struct
patch *patch,
clear_image(tmp_image);
/* in-core three-way merge between post and our using pre as base */
- return three_way_merge(image,
- patch-new_name, pre_sha1, our_sha1, post_sha1);
+ status = three_way_merge(image, patch-new_name,
+pre_sha1, our_sha1, post_sha1);
+ if (status 0) {
+ fprintf(stderr, Failed to fall back on three-way merge...\n);
+ return status;
+ }
+
+ if (status) {
+ patch-conflicted_threeway = 1;
+ hashcpy(patch-threeway_stage[0], pre_sha1);
+ hashcpy(patch-threeway_stage[1], our_sha1);
+ hashcpy(patch-threeway_stage[2], post_sha1);
+ fprintf(stderr, Applied patch to '%s' with conflicts.\n,
patch-new_name);
+ } else {
+ fprintf(stderr, Applied patch to '%s' cleanly.\n,
patch-new_name);
+ }
+ return 0;
}
static int apply_data(struct patch *patch, struct stat *st, struct cache_entry
*ce)
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 14/19] apply: move verify_index_match() higher
We will be adding another caller of this function in a later patch.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index d84958b..682852c 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3074,6 +3074,16 @@ static struct patch *previous_patch(struct patch *patch,
int *gone)
return previous;
}
+static int verify_index_match(struct cache_entry *ce, struct stat *st)
+{
+ if (S_ISGITLINK(ce-ce_mode)) {
+ if (!S_ISDIR(st-st_mode))
+ return -1;
+ return 0;
+ }
+ return ce_match_stat(ce, st,
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
+}
+
#define SUBMODULE_PATCH_WITHOUT_INDEX 1
static int load_patch_target(struct strbuf *buf,
@@ -3260,16 +3270,6 @@ static int apply_data(struct patch *patch, struct stat
*st, struct cache_entry *
return 0;
}
-static int verify_index_match(struct cache_entry *ce, struct stat *st)
-{
- if (S_ISGITLINK(ce-ce_mode)) {
- if (!S_ISDIR(st-st_mode))
- return -1;
- return 0;
- }
- return ce_match_stat(ce, st,
CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);
-}
-
/*
* If patch that we are looking at modifies or deletes what we have,
* we would want it not to lose any local modification we have, either
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 15/19] apply: --3way with add/add conflict
When a patch wants to create a path, but we already have it in our
current state, pretend as if the patch and we independently added
the same path and cause add/add conflict, so that the user can
resolve it just like git merge in the same situation.
For that purpose, implement load_current() in terms of the
load_patch_target() helper introduced earlier to read the current
contents from the path given by patch-new_name (patch-old_name is
NULL for a creation patch).
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 70 ++---
1 file changed, 62 insertions(+), 8 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 682852c..24efb3f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -197,6 +197,7 @@ struct patch {
unsigned int is_rename:1;
unsigned int recount:1;
unsigned int conflicted_threeway:1;
+ unsigned int direct_to_threeway:1;
struct fragment *fragments;
char *result;
size_t resultsize;
@@ -3187,6 +3188,48 @@ static int three_way_merge(struct image *image,
return status;
}
+/*
+ * When directly falling back to add/add three-way merge, we read from
+ * the current contents of the new_name. In no cases other than that
+ * this function will be called.
+ */
+static int load_current(struct image *image, struct patch *patch)
+{
+ struct strbuf buf = STRBUF_INIT;
+ int status, pos;
+ size_t len;
+ char *img;
+ struct stat st;
+ struct cache_entry *ce;
+ char *name = patch-new_name;
+ unsigned mode = patch-new_mode;
+
+ if (!patch-is_new)
+ die(BUG: patch to %s is not a creation, patch-old_name);
+
+ pos = cache_name_pos(name, strlen(name));
+ if (pos 0)
+ return error(_(%s: does not exist in index), name);
+ ce = active_cache[pos];
+ if (lstat(name, st)) {
+ if (errno != ENOENT)
+ return error(_(%s: %s), name, strerror(errno));
+ if (checkout_target(ce, st))
+ return -1;
+ }
+ if (verify_index_match(ce, st))
+ return error(_(%s: does not match index), name);
+
+ status = load_patch_target(buf, ce, st, name, mode);
+ if (status 0)
+ return status;
+ else if (status)
+ return -1;
+ img = strbuf_detach(buf, len);
+ prepare_image(image, img, len, !patch-is_binary);
+ return 0;
+}
+
static int try_threeway(struct image *image, struct patch *patch,
struct stat *st, struct cache_entry *ce)
{
@@ -3198,13 +3241,15 @@ static int try_threeway(struct image *image, struct
patch *patch,
struct image tmp_image;
/* No point falling back to 3-way merge in these cases */
- if (patch-is_new || patch-is_delete ||
+ if (patch-is_delete ||
S_ISGITLINK(patch-old_mode) || S_ISGITLINK(patch-new_mode))
return -1;
/* Preimage the patch was prepared for */
- if (get_sha1(patch-old_sha1_prefix, pre_sha1) ||
- read_blob_object(buf, pre_sha1, patch-old_mode))
+ if (patch-is_new)
+ write_sha1_file(, 0, blob_type, pre_sha1);
+ else if (get_sha1(patch-old_sha1_prefix, pre_sha1) ||
+read_blob_object(buf, pre_sha1, patch-old_mode))
return error(repository lacks the necessary blob to fall back
on 3-way merge.);
fprintf(stderr, Falling back to three-way merge...\n);
@@ -3221,9 +3266,15 @@ static int try_threeway(struct image *image, struct
patch *patch,
clear_image(tmp_image);
/* our_sha1[] is ours */
- if (load_preimage(tmp_image, patch, st, ce))
- return error(cannot read the current contents of '%s',
-patch-old_name);
+ if (patch-is_new) {
+ if (load_current(tmp_image, patch))
+ return error(cannot read the current contents of '%s',
+patch-new_name);
+ } else {
+ if (load_preimage(tmp_image, patch, st, ce))
+ return error(cannot read the current contents of '%s',
+patch-old_name);
+ }
write_sha1_file(tmp_image.buf, tmp_image.len, blob_type, our_sha1);
clear_image(tmp_image);
@@ -3254,7 +3305,8 @@ static int apply_data(struct patch *patch, struct stat
*st, struct cache_entry *
if (load_preimage(image, patch, st, ce) 0)
return -1;
- if (apply_fragments(image, patch) 0) {
+ if (patch-direct_to_threeway ||
+ apply_fragments(image, patch) 0) {
/* Note: with --reject, apply_fragments() returns 0 */
if (!threeway || try_threeway(image, patch, st, ce) 0)
return -1;
@@ -3431,7 +3483,9 @@ static int
[PATCH v4 16/19] apply: register conflicted stages to the index
Now we have all the necessary logic to fall back on three-way merge when
the patch does not cleanly apply, insert the conflicted entries to the
index as appropriate. This obviously triggers only when the --index
option is used.
When we fall back to three-way merge and some of the merges fail, just
like the case where the --reject option was specified and we had to
write some *.rej files out for unapplicable patches, exit the command
with non-zero status without showing the diffstat and summary. Otherwise
they would make the list of problematic paths scroll off the display.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 66 +++
t/t4108-apply-threeway.sh | 78 +++
2 files changed, 138 insertions(+), 6 deletions(-)
create mode 100755 t/t4108-apply-threeway.sh
diff --git a/builtin/apply.c b/builtin/apply.c
index 24efb3f..dc52c94 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3288,7 +3288,10 @@ static int try_threeway(struct image *image, struct
patch *patch,
if (status) {
patch-conflicted_threeway = 1;
- hashcpy(patch-threeway_stage[0], pre_sha1);
+ if (patch-is_new)
+ hashclr(patch-threeway_stage[0]);
+ else
+ hashcpy(patch-threeway_stage[0], pre_sha1);
hashcpy(patch-threeway_stage[1], our_sha1);
hashcpy(patch-threeway_stage[2], post_sha1);
fprintf(stderr, Applied patch to '%s' with conflicts.\n,
patch-new_name);
@@ -3852,6 +3855,32 @@ static void create_one_file(char *path, unsigned mode,
const char *buf, unsigned
die_errno(_(unable to write file '%s' mode %o), path, mode);
}
+static void add_conflicted_stages_file(struct patch *patch)
+{
+ int stage, namelen;
+ unsigned ce_size, mode;
+ struct cache_entry *ce;
+
+ if (!update_index)
+ return;
+ namelen = strlen(patch-new_name);
+ ce_size = cache_entry_size(namelen);
+ mode = patch-new_mode ? patch-new_mode : (S_IFREG | 0644);
+
+ remove_file_from_cache(patch-new_name);
+ for (stage = 1; stage 4; stage++) {
+ if (is_null_sha1(patch-threeway_stage[stage - 1]))
+ continue;
+ ce = xcalloc(1, ce_size);
+ memcpy(ce-name, patch-new_name, namelen);
+ ce-ce_mode = create_ce_mode(mode);
+ ce-ce_flags = create_ce_flags(namelen, stage);
+ hashcpy(ce-sha1, patch-threeway_stage[stage - 1]);
+ if (add_cache_entry(ce, ADD_CACHE_OK_TO_ADD) 0)
+ die(_(unable to add cache entry for %s),
patch-new_name);
+ }
+}
+
static void create_file(struct patch *patch)
{
char *path = patch-new_name;
@@ -3862,7 +3891,11 @@ static void create_file(struct patch *patch)
if (!mode)
mode = S_IFREG | 0644;
create_one_file(path, mode, buf, size);
- add_index_file(path, mode, buf, size);
+
+ if (patch-conflicted_threeway)
+ add_conflicted_stages_file(patch);
+ else
+ add_index_file(path, mode, buf, size);
}
/* phase zero is to remove, phase one is to create */
@@ -3964,6 +3997,7 @@ static int write_out_results(struct patch *list)
int phase;
int errs = 0;
struct patch *l;
+ struct string_list cpath = STRING_LIST_INIT_DUP;
for (phase = 0; phase 2; phase++) {
l = list;
@@ -3972,12 +4006,28 @@ static int write_out_results(struct patch *list)
errs = 1;
else {
write_out_one_result(l, phase);
- if (phase == 1 write_out_one_reject(l))
- errs = 1;
+ if (phase == 1) {
+ if (write_out_one_reject(l))
+ errs = 1;
+ if (l-conflicted_threeway) {
+ string_list_append(cpath,
l-new_name);
+ errs = 1;
+ }
+ }
}
l = l-next;
}
}
+
+ if (cpath.nr) {
+ struct string_list_item *item;
+
+ sort_string_list(cpath);
+ for_each_string_list_item(item, cpath)
+ fprintf(stderr, U %s\n, item-string);
+ string_list_clear(cpath, 0);
+ }
+
return errs;
}
@@ -4100,8 +4150,12 @@ static int apply_patch(int fd, const char *filename, int
options)
!apply_with_reject)
exit(1);
- if (apply write_out_results(list))
[PATCH v4 18/19] apply: document --3way option
Signed-off-by: Junio C Hamano gits...@pobox.com --- Documentation/git-apply.txt | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt index afd2c9a..634b84e 100644 --- a/Documentation/git-apply.txt +++ b/Documentation/git-apply.txt @@ -9,7 +9,7 @@ git-apply - Apply a patch to files and/or to the index SYNOPSIS [verse] -'git apply' [--stat] [--numstat] [--summary] [--check] [--index] +'git apply' [--stat] [--numstat] [--summary] [--check] [--index] [--3way] [--apply] [--no-add] [--build-fake-ancestor=file] [-R | --reverse] [--allow-binary-replacement | --binary] [--reject] [-z] [-pn] [-Cn] [--inaccurate-eof] [--recount] [--cached] @@ -72,6 +72,15 @@ OPTIONS cached data, apply the patch, and store the result in the index without using the working tree. This implies `--index`. +-3:: +--3way:: + When the patch does not apply cleanly, fall back on 3-way merge if + the patch records the identity of blobs it is supposed to apply to, + and we have those blobs available locally, possibly leaving the + conflict markers in the files in the working tree for the user to + resolve. This option implies the `--index` option, and is incompatible + with the `--reject` and the `--cached` options. + --build-fake-ancestor=file:: Newer 'git diff' output has embedded 'index information' for each blob to help identify the original version that -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 17/19] apply: allow rerere() to work on --3way results
Signed-off-by: Junio C Hamano gits...@pobox.com --- builtin/apply.c | 3 +++ t/t4108-apply-threeway.sh | 25 + 2 files changed, 28 insertions(+) diff --git a/builtin/apply.c b/builtin/apply.c index dc52c94..cd68862 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -18,6 +18,7 @@ #include parse-options.h #include xdiff-interface.h #include ll-merge.h +#include rerere.h /* * --check turns on checking that the working tree matches the @@ -4026,6 +4027,8 @@ static int write_out_results(struct patch *list) for_each_string_list_item(item, cpath) fprintf(stderr, U %s\n, item-string); string_list_clear(cpath, 0); + + rerere(0); } return errs; diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index 475dfb5..e6d4da6 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -75,4 +75,29 @@ test_expect_success 'apply with --3way' ' test_cmp expect.diff actual.diff ' +test_expect_success 'apply with --3way with rerere enabled' ' + git config rerere.enabled true + + # Merging side should be similar to applying this patch + git diff ...side P.diff + + # The corresponding conflicted merge + git reset --hard + git checkout master^0 + test_must_fail git merge --no-commit side + + # Manually resolve and record the resolution + create_file 1 two 3 4 five six 7 one + git rerere + cat one expect + + # should fail to apply + git reset --hard + git checkout master^0 + test_must_fail git apply --index --3way P.diff + + # but rerere should have replayed the recorded resolution + test_cmp expect one +' + test_done -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 19/19] apply: tests for the --3way option
Signed-off-by: Junio C Hamano gits...@pobox.com --- t/t4108-apply-threeway.sh | 54 +++ 1 file changed, 54 insertions(+) diff --git a/t/t4108-apply-threeway.sh b/t/t4108-apply-threeway.sh index e6d4da6..fa5d4ef 100755 --- a/t/t4108-apply-threeway.sh +++ b/t/t4108-apply-threeway.sh @@ -100,4 +100,58 @@ test_expect_success 'apply with --3way with rerere enabled' ' test_cmp expect one ' +test_expect_success 'apply -3 with add/add conflict setup' ' + git reset --hard + + git checkout -b adder + create_file 1 2 3 4 5 6 7 three + create_file 1 2 3 4 5 6 7 four + git add three four + git commit -m add three and four + + git checkout -b another adder^ + create_file 1 2 3 4 5 6 7 three + create_file 1 2 3 four 5 6 7 four + git add three four + git commit -m add three and four + + # Merging another should be similar to applying this patch + git diff adder...another P.diff + + git checkout adder^0 + test_must_fail git merge --no-commit another + git ls-files -s expect.ls + git diff HEAD | sanitize_conflicted_diff expect.diff +' + +test_expect_success 'apply -3 with add/add conflict' ' + # should fail to apply ... + git reset --hard + git checkout adder^0 + test_must_fail git apply --index --3way P.diff + # ... and leave conflicts in the index and in the working tree + git ls-files -s actual.ls + git diff HEAD | sanitize_conflicted_diff actual.diff + + # The result should resemble the corresponding merge + test_cmp expect.ls actual.ls + test_cmp expect.diff actual.diff +' + +test_expect_success 'apply -3 with add/add conflict (dirty working tree)' ' + # should fail to apply ... + git reset --hard + git checkout adder^0 + echo four + cat four four.save + cat three three.save + git ls-files -s expect.ls + test_must_fail git apply --index --3way P.diff + # ... and should not touch anything + git ls-files -s actual.ls + test_cmp expect.ls actual.ls + test_cmp four.save four + test_cmp three.save three +' + test_done -- 1.7.11.1.294.g68a9409 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 10/19] apply: move already exists logic to check_to_create()
The check_to_create_blob() function used to check only the case
where we are applying to the working tree. Rename the function to
check_to_create() and make it also responsible for checking the case
where we apply to the index. Also make its caller responsible for
issuing an error message.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
builtin/apply.c | 40 +---
1 file changed, 29 insertions(+), 11 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index cd25e63..7379687 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3249,9 +3249,21 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
return 0;
}
-static int check_to_create_blob(const char *new_name, int ok_if_exists)
+
+#define EXISTS_IN_INDEX 1
+#define EXISTS_IN_WORKTREE 2
+
+static int check_to_create(const char *new_name, int ok_if_exists)
{
struct stat nst;
+
+ if (check_index
+ cache_name_pos(new_name, strlen(new_name)) = 0
+ !ok_if_exists)
+ return EXISTS_IN_INDEX;
+ if (cached)
+ return 0;
+
if (!lstat(new_name, nst)) {
if (S_ISDIR(nst.st_mode) || ok_if_exists)
return 0;
@@ -3265,10 +3277,10 @@ static int check_to_create_blob(const char *new_name,
int ok_if_exists)
if (has_symlink_leading_path(new_name, strlen(new_name)))
return 0;
- return error(_(%s: already exists in working directory),
new_name);
- }
- else if ((errno != ENOENT) (errno != ENOTDIR))
+ return EXISTS_IN_WORKTREE;
+ } else if ((errno != ENOENT) (errno != ENOTDIR)) {
return error(%s: %s, new_name, strerror(errno));
+ }
return 0;
}
@@ -3316,15 +3328,21 @@ static int check_patch(struct patch *patch)
if (new_name
((0 patch-is_new) | (0 patch-is_rename) | patch-is_copy)) {
- if (check_index
- cache_name_pos(new_name, strlen(new_name)) = 0
- !ok_if_exists)
+ int err = check_to_create(new_name, ok_if_exists);
+
+ switch (err) {
+ case 0:
+ break; /* happy */
+ case EXISTS_IN_INDEX:
return error(_(%s: already exists in index),
new_name);
- if (!cached) {
- int err = check_to_create_blob(new_name, ok_if_exists);
- if (err)
- return err;
+ break;
+ case EXISTS_IN_WORKTREE:
+ return error(_(%s: already exists in working
directory),
+new_name);
+ default:
+ return err;
}
+
if (!patch-new_mode) {
if (0 patch-is_new)
patch-new_mode = S_IFREG | 0644;
--
1.7.11.1.294.g68a9409
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git commit --amend --only -- nevertheless commits staged changes
When using git commit --amend --only --message message --, I'd expect to have just the commit message of my last commit changed, according to the man page: --only Make a commit only from the paths specified on the command line, disregarding any contents that have been staged so far. [...] If this option is specified together with --amend, then no paths need to be specified, which can be used to amend the last commit without committing changes that have already been staged. However, all staged changes are committed as well. So looks like either the man page or Git is wrong here!? Tested with 1.7.10.msysgit.1. -Marc -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] A better way of handling upstream information in git-branch
Hello all, This stems from comments made by Junio and Jonathan about my proposed changes to --set-upstream. This should probably have a few tests, but I'd like to hear comments about the code and documentation first. The third patch is the one I'm not so confident about. It would be simpler to remove the whole branch.foo configuration, but that wouldn't be very safe, as we may have more things there (either the future git or some external tool). Carlos Martín Nieto (3): branch: introduce --set-upstream-to branch: suggest how to undo a --set-upstream when given one branch branch: add --unset-upstream option Documentation/git-branch.txt | 14 +++- builtin/branch.c | 50 +++--- 2 files changed, 60 insertions(+), 4 deletions(-) -- 1.7.10.2.1.g8c77c3c -- To unsubscribe from this list: send the line 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] branch: suggest how to undo a --set-upstream when given one branch
This interface is error prone, and a better one (--set-upstream-to)
exists. Suggest how to fix a --set-upstream invocation in case the
user only gives one argument, which makes it likely that he meant to
do the opposite, like with
git branch --set-upstream origin/master
when they meant one of
git branch --set-upstream origin/master master
git branch --set-upstream-to origin/master
Signed-off-by: Carlos Martín Nieto c...@elego.de
---
builtin/branch.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index c886fc0..5551227 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
info and making sure new_upstream is correct */
create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet,
BRANCH_TRACK_OVERRIDE);
} else if (argc 0 argc = 2) {
+ struct branch *branch = branch_get(argv[0]);
+ const char *old_upstream = NULL;
+ int branch_existed = 0;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
+
+ /* Save what argv[0] was pointing to so we can give
+ the --set-upstream-to hint */
+ if (branch_has_merge_config(branch))
+ old_upstream = shorten_unambiguous_ref(branch-merge[0]-dst,
0);
+
+ branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, 0, quiet, track);
+
+ if (argc == 1) {
+ printf(If you wanted to make '%s' track '%s', do
this:\n, head, argv[0]);
+ if (branch_existed)
+ printf( $ git branch --set-upstream '%s'
'%s'\n, argv[0], old_upstream);
+ else
+ printf( $ git branch -d '%s'\n, argv[0]);
+
+ printf( $ git branch --set-upstream-to '%s'\n,
argv[0]);
+ }
+
} else
usage_with_options(builtin_branch_usage, options);
--
1.7.10.2.1.g8c77c3c
--
To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Carlos Martín Nieto c...@elego.de writes:
diff --git a/builtin/branch.c b/builtin/branch.c
index 0e060f2..c886fc0 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -713,6 +713,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
int verbose = 0, abbrev = -1, detached = 0;
int reflog = 0, edit_description = 0;
int quiet = 0;
+ const char *new_upstream = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
struct commit_list *with_commit = NULL;
@@ -726,6 +727,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
BRANCH_TRACK_EXPLICIT),
OPT_SET_INT( 0, set-upstream, track, change upstream info,
BRANCH_TRACK_OVERRIDE),
+ OPT_STRING('u', set-upstream-to, new_upstream, upstream,
change the upstream info),
OPT__COLOR(branch_use_color, use colored output),
OPT_SET_INT('r', remotes, kinds, act on remote-tracking
branches,
REF_REMOTE_BRANCH),
@@ -794,10 +796,10 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, builtin_branch_usage,
0);
- if (!delete !rename !edit_description argc == 0)
+ if (!delete !rename !edit_description !new_upstream argc ==
0)
list = 1;
- if (!!delete + !!rename + !!force_create + !!list 1)
+ if (!!delete + !!rename + !!force_create + !!list + !!new_upstream 1)
usage_with_options(builtin_branch_usage, options);
It probably is an error to have track and new_upstream together.
The remainder of [Patch 1/3] looked entirely sensible, including the
proposed log message (modulo missing sign-off).
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: What's cooking in git.git (Jul 2012, #01; Tue, 3)
Junio C Hamano wrote: [...] * rj/platform-pread-may-be-thread-unsafe (2012-06-26) 1 commit (merged to 'next' on 2012-06-28 at ce5f79f) + index-pack: Disable threading on cygwin On Cygwin, the platform pread(3) is not thread safe, just like our own compat/ emulation, and cannot be used in the index-pack program. I've been using this patch (applied directly on top of v1.7.11) for the last two weeks without problem. (So I guess you could declare it tested!) Are you planning on an v1.7.11.2 anytime soon? If so, could I request that you include this patch in the release. (I am mildly surprised at the lack of complaints from cygwin users, but it would be nice to be able to point them to a release if they start flooding in :-P ). ATB, Ramsay Jones -- To unsubscribe from this list: send the line 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] branch: suggest how to undo a --set-upstream when given one branch
Carlos Martín Nieto c...@elego.de writes:
This interface is error prone, and a better one (--set-upstream-to)
exists. Suggest how to fix a --set-upstream invocation in case the
user only gives one argument, which makes it likely that he meant to
do the opposite, like with
git branch --set-upstream origin/master
when they meant one of
git branch --set-upstream origin/master master
git branch --set-upstream-to origin/master
Signed-off-by: Carlos Martín Nieto c...@elego.de
The new code does not seem to depend on the value of track (which
is set by either -t or --set-upstream) in any way. Shouldn't it be
done only when it is set to track-override?
Doesn't git branch [-f] frotz without any other argument trigger
the warning?
builtin/branch.c | 22 ++
1 file changed, 22 insertions(+)
diff --git a/builtin/branch.c b/builtin/branch.c
index c886fc0..5551227 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
info and making sure new_upstream is correct */
create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet,
BRANCH_TRACK_OVERRIDE);
} else if (argc 0 argc = 2) {
+ struct branch *branch = branch_get(argv[0]);
+ const char *old_upstream = NULL;
+ int branch_existed = 0;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
+
+ /* Save what argv[0] was pointing to so we can give
+the --set-upstream-to hint */
+ if (branch_has_merge_config(branch))
+ old_upstream = shorten_unambiguous_ref(branch-merge[0]-dst,
0);
+
+ branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, 0, quiet, track);
+
+ if (argc == 1) {
+ printf(If you wanted to make '%s' track '%s', do
this:\n, head, argv[0]);
+ if (branch_existed)
+ printf( $ git branch --set-upstream '%s'
'%s'\n, argv[0], old_upstream);
+ else
+ printf( $ git branch -d '%s'\n, argv[0]);
+
+ printf( $ git branch --set-upstream-to '%s'\n,
argv[0]);
+ }
+
} else
usage_with_options(builtin_branch_usage, options);
--
To unsubscribe from this list: send the line 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] branch: add --unset-upstream option
Carlos Martín Nieto c...@elego.de writes:
We have ways of setting the upstream information, but if we want to
unset it, we need to resort to modifying the configuration manually.
Teach branch an --unset-upstream option that unsets this information.
---
create_branch() uses install_branch_config() which may also set
branch.foo.rebase, so this version might leave some configuration
laying around.
I wonder if deleting the whole branch.foo section would be better. Can
we be sure that nothing else shows up there?
branch.foo.$unknown may not be related to upstream at all, so that
will not fly. Besides, we already have unknown=description, no?
If you are removing the branch foo, it would make sense, though.
+ } else if (unset_upstream) {
+ struct branch *branch = branch_get(argv[0]);
+ struct strbuf buf = STRBUF_INIT;
+
+ strbuf_addf(buf, branch.%s.remote, branch-name);
+ git_config_set_multivar(buf.buf, NULL, NULL, 1);
This part makes sense, as --set-upstream is about setting the
value of branch.foo.remote to 'origin' or whatever.
+ strbuf_reset(buf);
+ strbuf_addf(buf, branch.%s.merge, branch-name);
+ git_config_set_multivar(buf.buf, NULL, NULL, 1);
This also makes sense because branch.foo.merge names a ref in the
context of the remote. A branch may have integrated with the dev
branch at origin repository; when it is modified to slurp changes
from central repository from now on, there is nothing that says
that the branch dev at this different remote corresponds to the
dev branch at the original origin repository (such a branch may
not even exist at the new central repository). There is no point
leaving the branch.foo.merge configuration behind when you unset
the upstream information.
+ strbuf_release(buf);
} else if (argc 0 argc = 2) {
struct branch *branch = branch_get(argv[0]);
const char *old_upstream = NULL;
--
To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Hi,
Carlos Martín Nieto wrote:
The existing --set-uptream option can cause confusion, as it uses the
usual branch convention of assuming a starting point of HEAD if none
is specified, causing
git branch --set-upstream origin/master
to create a new local branch 'origin/master' that tracks the current
branch. As --set-upstream already exists, we can't simply change its
behaviour. To work around this, introduce --set-upstream-to which
accepts a compulsory argument
Thanks. A part of me really dislikes this --set-upstream-to which
is named more awkwardly than the deprecated mistake it replaces,
though.
Here's a patch on top to play with that names the new option
--set-upstream=. Untested.
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
---
diff --git i/Documentation/git-branch.txt w/Documentation/git-branch.txt
index f572913f..57935a64 100644
--- i/Documentation/git-branch.txt
+++ w/Documentation/git-branch.txt
@@ -49,7 +49,7 @@ branch so that 'git pull' will appropriately merge from
the remote-tracking branch. This behavior may be changed via the global
`branch.autosetupmerge` configuration flag. That setting can be
overridden by using the `--track` and `--no-track` options, and
-changed later using `git branch --set-upstream-to`.
+changed later using `git branch --set-upstream`.
With a `-m` or `-M` option, oldbranch will be renamed to newbranch.
If oldbranch had a corresponding reflog, it is renamed to match
@@ -174,11 +174,13 @@ start-point is either a local or remote-tracking branch.
like `--track` would when creating the branch, except that where
branch points to is not changed.
--u upstream::
---set-upstream-to=upstream::
+--set-upstream=upstream::
Set up branchname's tracking information so upstream is
considered branchname's upstream branch. If no branch is
specified it defaults to the current branch.
++
+If no argument is attached, for historical reasons the meaning is
+different. See above.
--edit-description::
Open an editor and edit the text to explain what the branch is
diff --git i/builtin/branch.c w/builtin/branch.c
index c886fc06..0d705790 100644
--- i/builtin/branch.c
+++ w/builtin/branch.c
@@ -669,6 +669,31 @@ static int opt_parse_merge_filter(const struct option
*opt, const char *arg, int
return 0;
}
+struct set_upstream_params {
+ enum branch_track *track;
+ const char **new_upstream;
+};
+static int parse_opt_set_upstream(const struct option *opt, const char *arg,
int unset)
+{
+ struct set_upstream_params *o = opt-value;
+
+ if (unset) {/* --no-set-upstream */
+ *o-track = BRANCH_TRACK_NEVER;
+ *o-new_upstream = NULL;
+ return 0;
+ }
+
+ *o-track = BRANCH_TRACK_OVERRIDE;
+ if (!arg) /* --set-upstream branchname start-point */
+ *o-new_upstream = NULL;
+ else/* --set-upstream=upstream branchname */
+ *o-new_upstream = arg;
+ return 0;
+}
+#define OPT_SET_UPSTREAM(s, l, v) \
+ { OPTION_CALLBACK, (s), (l), (v), upstream, change upstream info, \
+ PARSE_OPT_OPTARG, parse_opt_set_upstream }
+
static const char edit_description[] = BRANCH_DESCRIPTION;
static int edit_branch_description(const char *branch_name)
@@ -716,6 +741,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
const char *new_upstream = NULL;
enum branch_track track;
int kinds = REF_LOCAL_BRANCH;
+ struct set_upstream_params set_upstream_args = { track, new_upstream
};
struct commit_list *with_commit = NULL;
struct option options[] = {
@@ -725,9 +751,7 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
OPT__QUIET(quiet, suppress informational messages),
OPT_SET_INT('t', track, track, set up tracking mode (see
git-pull(1)),
BRANCH_TRACK_EXPLICIT),
- OPT_SET_INT( 0, set-upstream, track, change upstream info,
- BRANCH_TRACK_OVERRIDE),
- OPT_STRING('u', set-upstream-to, new_upstream, upstream,
change the upstream info),
+ OPT_SET_UPSTREAM(0, set-upstream, set_upstream_args),
OPT__COLOR(branch_use_color, use colored output),
OPT_SET_INT('r', remotes, kinds, act on remote-tracking
branches,
REF_REMOTE_BRANCH),
--
To unsubscribe from this list: send the line 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] branch: suggest how to undo a --set-upstream when given one branch
Hi,
Quick nitpicks.
Carlos Martín Nieto wrote:
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
info and making sure new_upstream is correct */
create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet,
BRANCH_TRACK_OVERRIDE);
} else if (argc 0 argc = 2) {
+ struct branch *branch = branch_get(argv[0]);
+ const char *old_upstream = NULL;
+ int branch_existed = 0;
+
if (kinds != REF_LOCAL_BRANCH)
die(_(-a and -r options to 'git branch' do not make
sense with a branch name));
+
+ /* Save what argv[0] was pointing to so we can give
+the --set-upstream-to hint */
+ if (branch_has_merge_config(branch))
+ old_upstream = shorten_unambiguous_ref(branch-merge[0]-dst,
0);
Whitespace is odd here. Maybe this case could be factored out as a
new function to make room on the right margin and make cmd_branch()
easier to read straight through.
+
+ branch_existed = ref_exists(branch-refname);
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
force_create, reflog, 0, quiet, track);
+
+ if (argc == 1) {
+ printf(If you wanted to make '%s' track '%s', do
this:\n, head, argv[0]);
+ if (branch_existed)
+ printf( $ git branch --set-upstream '%s'
'%s'\n, argv[0], old_upstream);
+ else
+ printf( $ git branch -d '%s'\n, argv[0]);
+
+ printf( $ git branch --set-upstream-to '%s'\n,
argv[0]);
Message should go on stderr and be guarded with an advice option (see
advice.c).
Like this:
const char *arg;
...
if (argc != 1 || !advice_old_fashioned_set_upstream)
return 0; /* ok. */
arg = argv[0];
advise(If you wanted to make '%s' track '%s', do this:,
head, arg);
if (branch_existed)
advise( $ git branch --set-upstream-to='%s' '%s',
old_upstream, arg);
else
advise( $ git branch -d '%s', arg);
advise( $ git branch --set-upstream-to='%s', arg);
If an argument contains single-quotes, the quoting will be wrong, but
that's probably not worth worrying about.
Hope that helps,
Jonathan
--
To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Junio C Hamano wrote: I am not super excited about it either, but at least it is a vast improvement compared to the older one, with which it was entirely unclear if we are setting the value of upstream *to* what is given as an option, or setting the upstream *for* what is given on the command line. Ah, do you mean that --set-upstream is meant to have usage like git remote set-url and co? git remote set-url remote url git branch --set-upstream branch upstream That's a reasonable stance, and it seems possible to get used to it. In that case, we should just teach --set-upstream not to create new branches, and people will get used to it. The immediate problem that seems to trip people up is that it is very tempting to run git branch --set-upstream junio/master in an attempt to change what is upstream to the current branch, and the result is some other completely counterintuitive thing. I suspect the order of arguments to --set-upstream is a red herring, as long as it errors out when the arguments are switched to help people catch mistakes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git commit --amend --only -- nevertheless commits staged changes
On Tue, Jul 10, 2012 at 12:41:13PM +0200, Marc Strapetz wrote: When using git commit --amend --only --message message --, I'd expect to have just the commit message of my last commit changed, according to the man page: --only Make a commit only from the paths specified on the command line, disregarding any contents that have been staged so far. [...] If this option is specified together with --amend, then no paths need to be specified, which can be used to amend the last commit without committing changes that have already been staged. However, all staged changes are committed as well. So looks like either the man page or Git is wrong here!? I think git has a bug. As far as I can tell, this has never worked as the documentation advertised. We originally forbid the use of --only without paths as nonsensical. This was loosened by 6a74642 (git-commit --amend: two fixes., 2006-04-20) to let --amend --only --, but I don't think it even worked then. Using this test: git init repo cd repo echo foo one foo echo bar one bar git add . git commit -m one echo foo two foo echo bar two bar git add foo GIT_EDITOR=true git commit --amend -o git cat-file -p HEAD:foo git cat-file -p HEAD:bar I always get: foo two bar one i.e., we accidentally amend the commit with the staged contents in the index. I get the same results for 6a74642 and on. If you switch the commit to -o bar, it does work properly (you get the updated bar, but the staged foo in the index is ignored). -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: git commit --amend --only -- nevertheless commits staged changes
Marc Strapetz marc.strap...@syntevo.com writes:
When using git commit --amend --only --message message --, I'd
expect to have just the commit message of my last commit changed,
according to the man page:
--only Make a commit only from the paths specified on the command line,
disregarding any contents that have been staged so far. [...] If this
option is specified together with --amend, then no paths need to be
specified, which can be used to amend the last commit without committing
changes that have already been staged.
However, all staged changes are committed as well. So looks like either
the man page or Git is wrong here!?
I do not think the combination with --amend, --only and no paths
ever worked. We rejected such a combination before 6a74642c5, which
merely made us to accept the combination but I do not think the
commit did anything to re-read the tree from the HEAD being amended
to the index.
Something like this, but I haven't thought about what other things
it may break.
builtin/commit.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..59ef5e1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -184,6 +184,9 @@ static int list_paths(struct string_list *list, const char
*with_tree,
int i;
char *m;
+ if (!pattern)
+ return 0;
+
for (i = 0; pattern[i]; i++)
;
m = xcalloc(1, i);
@@ -345,7 +348,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
* and create commit from the_index.
* We still need to refresh the index here.
*/
- if (!pathspec || !*pathspec) {
+ if (!(only amend) (!pathspec || !*pathspec)) {
fd = hold_locked_index(index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git commit --amend --only -- nevertheless commits staged changes
On Tue, Jul 10, 2012 at 04:30:52PM -0400, Jeff King wrote:
On Tue, Jul 10, 2012 at 01:14:32PM -0700, Junio C Hamano wrote:
I do not think the combination with --amend, --only and no paths
ever worked. We rejected such a combination before 6a74642c5, which
merely made us to accept the combination but I do not think the
commit did anything to re-read the tree from the HEAD being amended
to the index.
Something like this, but I haven't thought about what other things
it may break.
Our emails just crossed. I came to the exact same conclusion, and just
wrote almost the exact same patch.
Here it is with a test and commit message. I believe this fix could also
make:
git commit --allow-empty --only --
work if we removed the --only does not make sense without paths check.
But I seriously doubt that anybody cares, given that --only is the
default (i.e., just omitting it already does what you want there,
whether you have pathspecs or not).
-- 8 --
Subject: [PATCH] commit: fix --amend --only with no pathspec
When we do not have any pathspec, we typically disallow an
explicit --only, because it makes no sense (your commit
would, by definition, be empty). But since 6a74642
(git-commit --amend: two fixes., 2006-04-20), we have
allowed --amend --only with the intent that it would amend
the commit, ignoring any contents staged in the index.
However, while that commit allowed the combination, we never
actually implemented the logic to make it work. The current
code notices that we have no pathspec and assumes we want to
do an as-is commit (i.e., the --only is ignored).
Instead, we must make sure to follow the partial-commit
code-path. We also need to tweak the list_paths function to
handle a NULL pathspec.
Signed-off-by: Jeff King p...@peff.net
---
builtin/commit.c | 5 -
t/t7501-commit.sh | 10 ++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index f43eaaf..3c3385c 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -184,6 +184,9 @@ static int list_paths(struct string_list *list, const char
*with_tree,
int i;
char *m;
+ if (!pattern)
+ return 0;
+
for (i = 0; pattern[i]; i++)
;
m = xcalloc(1, i);
@@ -345,7 +348,7 @@ static char *prepare_index(int argc, const char **argv,
const char *prefix,
* and create commit from the_index.
* We still need to refresh the index here.
*/
- if (!pathspec || !*pathspec) {
+ if (!only (!pathspec || !*pathspec)) {
fd = hold_locked_index(index_lock, 1);
refresh_cache_or_die(refresh_flags);
if (active_cache_changed) {
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index b20ca0e..9f8d423 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -108,6 +108,16 @@ test_expect_success 'amend commit' '
EDITOR=./editor git commit --amend
'
+test_expect_success 'amend --only ignores staged contents' '
+ test_when_finished git reset --hard
+ cp file file.expect
+ echo changed file
+ git add file
+ git commit --no-edit --amend --only
+ git cat-file blob HEAD:file file.actual
+ test_cmp file.expect file.actual
+'
+
test_expect_success 'set up editor' '
cat editor -\EOF
#!/bin/sh
--
1.7.11.35.gbaf554e.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 1/3] branch: introduce --set-upstream-to
Jonathan Nieder jrnie...@gmail.com writes: The immediate problem that seems to trip people up is that it is very tempting to run git branch --set-upstream junio/master I think we have discussed this already a few days ago. See my comment in the earlier thread before this round. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git clone fails with error: RPC failed; result=22, HTTP code = 401
On Fri, Jul 06, 2012 at 02:04:10PM +0200, Ilya Ruprecht wrote: Location /git/repo1.git # read access Limit GET require ldap-group repo.writers require ldap-group repo.readers /Limit # write access Limit GET PUT POST DELETE PROPPATCH MKCOL COPY MOVE LOCK UNLOCK require ldap-group repo.writers /Limit As you noticed, this will not do what you want. Git's smart-http protocol uses POST requests to send the list of heads during ref negotiation. So even a fetch request will require both GETs and POSTs. The right way to restrict reading versus writing over smart-http is to check which git service is being requested (confusingly, git-upload-pack is for clones and fetches, and git-receive-pack is for pushes; the names are based on what the _server_ is doing). There is an example in the git-http-backend documentation, which uses a LocationMatch along with a require directive. -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 1/3] branch: introduce --set-upstream-to
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: The immediate problem that seems to trip people up is that it is very tempting to run git branch --set-upstream junio/master I think we have discussed this already a few days ago. See my comment in the earlier thread before this round. You wrote[*]: | I think it was a mistake that nobody noticed that it is likely that | the operation most often will be done for the current branch and the | usual give me one branch name to operate on, or I'll operate on the | current branch command line convention of git branch commannd is | not a good fit for it, when set upstream feature was added with which I completely agree. You then moved on to | and [someone should have] | suggested an alternative syntax that avoids the mistake you quoted | above, perhaps something like: | | git branch --set-upstream-to=origin/master [HEAD] with which I disagree. As far as I can tell, nobody really thought very hard about what --set-upstream would do when passed only one argument. It should have been made to error out and only later change if someone had an idea about how to make it useful. Luckily we have a way out. Any example transition plan looks like the following. DAY 1. $ git branch --set-upstream origin/master Branch origin/master set up to track local branch debian-sid. hint: If you intended to make the current branch track hint: origin/master, you can recover with the following commands: hint: $ git branch -d origin/master hint: $ git branch --set-upstream master origin/master $ DAY 2. $ git branch --set-upstream origin/master Branch origin/master set up to track local branch debian-sid. warning: using --set-upstream when creating a new branch is deprecated hint: use --track instead hint: hint: If you intended to make the current branch track hint: origin/master, you can recover with the following commands: hint: $ git branch -d origin/master hint: $ git branch --set-upstream master origin/master $ DAY 3. $ git branch --set-upstream origin/master fatal: no such branch origin/master $ DAY 4. $ git branch --set-upstream origin/master usage: git branch --set-upstream branchname upstream $ [*] http://thread.gmane.org/gmane.comp.version-control.git/201040/focus=201051 -- To unsubscribe from this list: send the line 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] branch: suggest how to undo a --set-upstream when given one branch
Jonathan Nieder jrnie...@gmail.com writes: Message should go on stderr and be guarded with an advice option (see advice.c). Like this: const char *arg; ... if (argc != 1 || !advice_old_fashioned_set_upstream) return 0; /* ok. */ arg = argv[0]; advise(If you wanted to make '%s' track '%s', do this:, head, arg); if (branch_existed) advise( $ git branch --set-upstream-to='%s' '%s', old_upstream, arg); else advise( $ git branch -d '%s', arg); advise( $ git branch --set-upstream-to='%s', arg); If an argument contains single-quotes, the quoting will be wrong, but that's probably not worth worrying about. In principle, I would agree that this is a kind of thing that falls into the advice categiry, but with the fact that we plan to deprecate --set-upstream, combined with the fact that [PATCH 1/3] introduced the new option --set-upstream-to together with a short and sweet -u synonym already at this point in the series, I think it is better to leave them emitted unconditionally to the standard error stream, in order to train users away from using the old option that has its arguments wrong (the option does not take an argument it should, and makes the command line to look as if it takes two branch arguments in the wrong order). Actually, we should probably add the deprecation warning in this commit. -- To unsubscribe from this list: send the line 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 2/2] stash: invoke rerere in case of conflict
'stash apply' directly calls a backend merge function
which does not automatically invoke rerere. This
confuses mergetool when leftover rerere state is left
behind from previous merges.
Invoke rerere explicitly when we encounter a conflict
during stash apply
Change the test for this flaw to expect success.
Signed-off-by: Phil Hord ho...@cisco.com
Mentored-by: Junio C Hamano gits...@pobox.com
---
git-stash.sh | 1 +
t/t7610-mergetool.sh | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index 4e2c7f8..bbefdf6 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -469,6 +469,7 @@ apply_stash () {
else
# Merge conflict; keep the exit status from merge-recursive
status=$?
+ git rerere
if test -n $INDEX_OPTION
then
gettextln Index was not unstashed. 2
diff --git a/t/t7610-mergetool.sh b/t/t7610-mergetool.sh
index 725f316..6fa0c70 100755
--- a/t/t7610-mergetool.sh
+++ b/t/t7610-mergetool.sh
@@ -203,7 +203,7 @@ test_expect_success 'mergetool skips resolved paths when
rerere is active' '
git reset --hard
'
-test_expect_failure 'conflicted stash sets up rerere' '
+test_expect_success 'conflicted stash sets up rerere' '
git config rerere.enabled true
git checkout stash1
echo Conflicting stash content file11
--
1.7.11.1.213.gb567ea5.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] branch: suggest how to undo a --set-upstream when given one branch
Junio C Hamano wrote: I think it is better to leave them emitted unconditionally to the standard error stream, in order to train users away from using the old option that has its arguments wrong (the option does not take an argument it should, and makes the command line to look as if it takes two branch arguments in the wrong order). I thought we already discussed that that is a side-issue? The option is a mode option for the command, like -m, -d, or --edit-description. I genuinely don't think the order of options it takes is counter-intuitive. The second argument defaulting to HEAD and the behavior of creating the branch named by the first argument when it does not exist are quite counter-intuitive. Transitioning to a different argument order seems like it would just make the command more complicated. After the transition, there are two options to explain, and during the transition, it is easy to make scripts with gratuitous incompatibilities that won't work on older systems. Where is my thinking going wrong? Jonathan -- To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Jonathan Nieder jrnie...@gmail.com writes: [someone should have] | suggested an alternative syntax that avoids the mistake you quoted | above, perhaps something like: | | git branch --set-upstream-to=origin/master [HEAD] with which I disagree. You can think of it this way. git branch can not only _create_ a new branch (or list existing ones, but that is another entirely different mode), but also can be used to set attributes to an existing branch. Imagine a new option, say --set-description, to replace branch.frotz.description, for example. It would be used like this: $ git branch --set-description='add frotz feature' frotz to set the description for the 'frotz' branch (i.e. the above would set branch.frotz.description), and we default to HEAD if 'frotz' is missing from the command line. git branch --option [branch] is about manipulating the branch, and we default the target of manipulation to HEAD. upstream is just another kind of attribute for the branch being manipulated, whose value happens to be a branch name. The mistake was that --set-upstream was coded by piggybacking the existing --track implementation where a new branch was created, and in that codepath, git branch name1 [name2] creates name1 while defaulting a missing name2 to HEAD. Creating a new branch that is forked from the current HEAD is an often useful thing to do, so defaulting a missing name2 (aka start-point) to HEAD is very sensible, but reconfiguring a named branch name1 to integrate with the current branch is much less useful than the other way around. One major reason why it is so is because you would more likely set any branch to integrate with a remote tracking branch (rather than a local branch) and by definition your HEAD cannot be a remote tracking branch. It makes it worse that you would often want to reconfigure the current branch; for the purpose of reconfiguring a branch name1 to integrate with something else name2, it is much more likely that you want a missing name1 to default to HEAD, not the other way around to default a missing name2 to HEAD, which is useful for branch creation. But switching which missing argument gets default based on what options are used is insane. If the very original create this new branch starting at that point were spelled like this $ git branch [--start-point=name2] name1 and a missing name2 defaulted to HEAD, it probably would have been better. It would have made it very unlikely to tempt anybody to hack the --set-upstream option into the system with the wrong parameter order if such a command line convention was in place. If anything, it could be a sensible longer-term direction to a more intuitive UI to deprecate the two-name format and make the creation to be specified with an explicit --start-point option with an argument (which defaults to HEAD), but I think that falls into the if I were reinventing git without existing userbase in 2005 category and it is too late for that. -- To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Junio C Hamano wrote: You can think of it this way. git branch can not only _create_ a new branch (or list existing ones, but that is another entirely different mode), but also can be used to set attributes to an existing branch. Imagine a new option, say --set-description, to replace branch.frotz.description, for example. It would be used like this: $ git branch --set-description='add frotz feature' frotz That's the same question. You say that it would be used like that. I say that it would be more intuitive, given how git remote, git config, and other commands other than update-index --chmod that set attributes already work, for it to be used like this: git branch --set-description frotz 'add frotz feature' Notice how similar that is to git remote set-head origin master. It would just be the consistent thing to do. The truth is that neither one of us is right. Both conventions could work, and which one is more intuitive will vary from person to person. The convention used for plain git branch is copy(target, source) That matches memcpy() and is the opposite of what cp uses. Oh well. The convention used for git remote add is method(this, args...) It's generally pretty natural. The convention used for git update-index --chmod is action(parameters)(files...) That matches chmod so it was probably a good choice. Hoping that clarifies, Jonathan -- To unsubscribe from this list: send the line 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] branch: introduce --set-upstream-to
Jonathan Nieder jrnie...@gmail.com writes: The truth is that neither one of us is right. Both conventions could work, and which one is more intuitive will vary from person to person. It is not just person-to-person, I think. In short, you are saying that, assuming that missing start and branch are given a sane default values (namely HEAD), the syntax: git branch branch [start] git branch --set-upstream-jrn [branch] upstream is easier to understand, while I think git branch branch [start] git branch --set-upstream-to=upstream [branch] so that omitted things can come uniformly at the end (of course, unless the --option=argument in the middle is omitted, that is) makes things more consistent. I do not think it is productive to keep agreeing that we disagree and continuing to talk between ourselves without waiting for others to catch up, so I'll stop here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] git-svn: don't create master if another head exists
Junio C Hamano gits...@pobox.com wrote: Marcin Owsiany mar...@owsiany.pl writes: This makes my idea to do the same to my something else instead of master much less attractive. In fact I don't think such behaviour would be useful. I think with the suggested patch git-svn works as I would like it to: - creates master at initial checkout - consistent with git clone - using a different tracking-like branch is possible with dcommit - does not re-create master on fetch - so the annoying part is gone Any comments? Not from me. Even though I'd love to hear Eric's opinion, your I don't think such behaviour would be useful. gave me an impression that you would justify the change in a different way (i.e. a rewrite of proposed log message) or tweak the patch (i.e. a modified behaviour), or perhaps both, in your re-roll, the ball was in your court, and we were waiting for such a rerolled patch. Sorry, I keep forgetting this topic. But yes, I thought you would tweak your patch. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] branch: introduce --set-upstream-to
Junio C Hamano wrote: In short, you are saying that, assuming that missing start and branch are given a sane default values (namely HEAD), the syntax: git branch branch [start] git branch --set-upstream-jrn [branch] upstream is easier to understand I didn't propose allowing the branch argument to be omitted, actually. It would be clearest, _especially_ because one argument currently means something different, to make that error out. Sorry for the lack of clarity. One more detail I didn't mention before: I think a convenience feature git branch --set-upstream-to upstream that takes exactly one argument and means git branch --set-upstream HEAD upstream would be fine. Having a second command to do the same thing as --set-upstream does (or adding new --set-other-things commands that use this proposed convention where the value comes before the key) and migrating awkwardly to it is what I object to. Clearer? Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
