Re: [PATCH] fetch: Print full url in header
On Thu, Jan 09, 2014 at 12:07:38PM -0800, Junio C Hamano wrote: Having said all that, the difference between the full URL shown by remote --verbose (which is used to interact with the remote in this repository) and the abbreviated URL (which is shown by fetch and is designed to be sharable with others with a simple cutpaste) matters only when there are a pair of ambiguously configured repositories (e.g. there are two repositories git://host/a.git/ and git://host/a/.git) that serve different things and you are debugging the situation. And to me, remote --verbose looks more or less a debugging aid, nothing more. So another alternative that may be to leave everything as-is. Thanks. I like the alterantive option of leave everything as-is, especially after the arguments you've presented. There is still the problem of the logic that has been duplicated. I think it should be put in a function, but if you are ok with leaving it duplicated that is fine by me. if (raw_url) url = transport_anonymize_url(raw_url); else url = xstrdup(foreign); url_len = strlen(url); for (i = url_len - 1; url[i] == '/' 0 = i; i--) ; url_len = i + 1; if (4 i !strncmp(.git, url + i - 3, 4)) url_len = i - 3; Thanks, Tom Miller -- To unsubscribe from this list: send the line 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] fetch: Print full url in header
Do not remove / and .git from the end of the header url when
fetching. This affects the output of fetch and fetch --prune
making the header url more consistent with remote --verbose.
Add tests to verify that fetch and fetch --prune do not strip the
trailing characters from the header url.
Output before this patch:
$ git fetch remote-with-dot-git-and-slash
From https://github.com/git/git
a155a5f..5512ac5 maint - upstream/maint
Output after this patch:
$ git fetch remote-with-dot-git-and-slash
From https://github.com/git/git.git/
a155a5f..5512ac5 maint - upstream/maint
Signed-off-by: Tom Miller jacker...@gmail.com
---
This patch should be based on the tip of next because it is dependent
on the code from tm/fetch-prune.
Initially I thought I would stop anonymizing the header url as well.
Then I ran across this commit.
commit 47abd85ba06ed7209d1caa3e5ac7cc6b232bece4
Author: Andreas Ericsson a...@op5.se
Date: Fri Apr 17 10:20:11 2009 +0200
fetch: Strip usernames from url's before storing them
When pulling from a remote, the full URL including username
is by default added to the commit message. Since it adds
very little value but could be used by malicious people to
glean valid usernames (with matching hostnames), we're far
better off just stripping the username before storing the
remote URL locally.
Note that this patch has no lasting visible effect when
git pull does not create a merge commit. It simply
alters what gets written to .git/FETCH_HEAD, which is used
by git merge to automagically create its messages.
Signed-off-by: Andreas Ericsson a...@op5.se
Signed-off-by: Junio C Hamano gits...@pobox.com
After reading this and trying different things with the code. I believe
it would make sense to continue to anonymize the url for output to the
terminal. I found if someone is using HTTP basic authentication and has
the username and password in the url. Then, one could unexpectedly
compromise their credentials during a fetch. I do not believe that
anyone should store their credentials in this way, but it is better safe
than sorry.
I also chose to continue to strip the trailing characters for the
FETCH_HEAD file. I wanted the input of the mailing list to see if we
should also stop striping the trailing characters off of the url written
to FETCH_HEAD? If so, I'll do it as a seperate patch.
builtin/fetch.c | 15 +++
t/t5510-fetch.sh | 29 -
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 025bc3e..01df749 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -664,8 +664,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
*what ? what : HEAD);
if (note.len) {
if (verbosity = 0 !shown_url) {
- fprintf(stderr, _(From %.*s\n),
- url_len, url);
+ fprintf(stderr, _(From %s\n), url);
shown_url = 1;
}
if (verbosity = 0)
@@ -723,7 +722,7 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
const char *raw_url)
{
- int url_len, i, result = 0;
+ int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
char *url;
const char *dangling_msg = dry_run
@@ -735,19 +734,11 @@ static int prune_refs(struct refspec *refs, int
ref_count, struct ref *ref_map,
else
url = xstrdup(foreign);
- url_len = strlen(url);
- for (i = url_len - 1; url[i] == '/' 0 = i; i--)
- ;
-
- url_len = i + 1;
- if (4 i !strncmp(.git, url + i - 3, 4))
- url_len = i - 3;
-
for (ref = stale_refs; ref; ref = ref-next) {
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
if (verbosity = 0 !shown_url) {
- fprintf(stderr, _(From %.*s\n), url_len, url);
+ fprintf(stderr, _(From %s\n), url);
shown_url = 1;
}
if (verbosity = 0) {
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 12674ac..882bfa1 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,15 +614,34 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack
[PATCH V3 2/2] fetch --prune: Run prune before fetching
When we have a remote-tracking branch named frotz/nitfol from a
previous fetch, and the upstream now has a branch named frotz. Prior
to this patch fetch would fail to remove frotz/nitfol with a git
fetch --prune from the upstream. git would inform the user to use git
remote prune to fix the problem.
This patch changes the way fetch --prune works by moving the pruning
operation before the fetching operation. Instead of warning the user of
a conflict, it autmatically fixes it.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
Acked-by: Junio C Hamano gits...@pobox.com
---
I did change the commit message according to Junio's suggestion in the
first patch.
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1b81cf9..09825c8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -863,11 +863,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -883,6 +878,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 87e896d..12674ac 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -626,4 +626,18 @@ test_expect_success 'fetch --prune prints the remotes url'
'
test_cmp expect actual
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.2.231.g4834e63.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3 1/2] fetch --prune: Always print header url
If fetch --prune is run with no new refs to fetch, but it has refs
to prune. Then, the header url is not printed as it would if there were
new refs to fetch.
Output before this patch:
$ git fetch --prune remote-with-no-new-refs
x [deleted] (none) - origin/world
Output after this patch:
$ git fetch --prune remote-with-no-new-refs
From https://github.com/git/git
x [deleted] (none) - origin/test
Signed-off-by: Tom Miller jacker...@gmail.com
---
I decided it is not worth writing a function to format the header url
that is printed by fetch. Instead, I will duplicate the formatting logic
for now because I plan on following up this patch set with another patch
to stop striping the trailing / and .git from the url. When that
patch is finished it will remove the majority of the duplicated logic.
builtin/fetch.c | 32 +++-
t/t5510-fetch.sh | 12
2 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1e7d617..1b81cf9 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
+static int shown_url = 0;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
{
FILE *fp;
struct commit *commit;
- int url_len, i, shown_url = 0, rc = 0;
+ int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
@@ -708,17 +709,36 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
return ret;
}
-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
+ const char *raw_url)
{
- int result = 0;
+ int url_len, i, result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
+ char *url;
const char *dangling_msg = dry_run
? _( (%s will become dangling))
: _( (%s has become dangling));
+ if (raw_url)
+ url = transport_anonymize_url(raw_url);
+ else
+ url = xstrdup(foreign);
+
+ url_len = strlen(url);
+ for (i = url_len - 1; url[i] == '/' 0 = i; i--)
+ ;
+
+ url_len = i + 1;
+ if (4 i !strncmp(.git, url + i - 3, 4))
+ url_len = i - 3;
+
for (ref = stale_refs; ref; ref = ref-next) {
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
+ if (verbosity = 0 !shown_url) {
+ fprintf(stderr, _(From %.*s\n), url_len, url);
+ shown_url = 1;
+ }
if (verbosity = 0) {
fprintf(stderr, x %-*s %-*s - %s\n,
TRANSPORT_SUMMARY(_([deleted])),
@@ -726,6 +746,7 @@ static int prune_refs(struct refspec *refs, int ref_count,
struct ref *ref_map)
warn_dangling_symref(stderr, dangling_msg, ref-name);
}
}
+ free(url);
free_refs(stale_refs);
return result;
}
@@ -854,11 +875,12 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (ref_count) {
- prune_refs(refs, ref_count, ref_map);
+ prune_refs(refs, ref_count, ref_map, transport-url);
} else {
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr,
- ref_map);
+ ref_map,
+ transport-url);
}
}
free_refs(ref_map);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..87e896d 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'fetch --prune prints the remotes url' '
+ git branch goodbye
+ git clone . only-prunes
+ git branch -D goodbye
+ (
+ cd only-prunes
+ git fetch --prune origin 21 | head -n1 ../actual
+ )
+ echo From ${D}/. expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.2.231.g4834e63.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message
[PATCH V2 1/2] fetch --prune: Always print header url
If fetch --prune is run with no new refs to fetch, but it has refs
to prune. Then, the header url is not printed as it would if there were
new refs to fetch.
Output before this patch:
$ git fetch --prune remote-with-no-new-refs
x [deleted] (none) - origin/world
Output after this patch:
$ git fetch --prune remote-with-no-new-refs
From https://github.com/git/git.git
x [deleted] (none) - origin/test
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 22 ++
t/t5510-fetch.sh | 12
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 1e7d617..e6dc2d6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,7 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
+static int shown_url = 0;
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -535,7 +536,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
{
FILE *fp;
struct commit *commit;
- int url_len, i, shown_url = 0, rc = 0;
+ int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
@@ -708,17 +709,28 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
return ret;
}
-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
+ const char *raw_url)
{
int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
+ char *url;
const char *dangling_msg = dry_run
? _( (%s will become dangling))
: _( (%s has become dangling));
+ if (raw_url)
+ url = transport_anonymize_url(raw_url);
+ else
+ url = xstrdup(foreign);
+
for (ref = stale_refs; ref; ref = ref-next) {
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
+ if (verbosity = 0 !shown_url) {
+ fprintf(stderr, _(From %s\n), url);
+ shown_url = 1;
+ }
if (verbosity = 0) {
fprintf(stderr, x %-*s %-*s - %s\n,
TRANSPORT_SUMMARY(_([deleted])),
@@ -726,6 +738,7 @@ static int prune_refs(struct refspec *refs, int ref_count,
struct ref *ref_map)
warn_dangling_symref(stderr, dangling_msg, ref-name);
}
}
+ free(url);
free_refs(stale_refs);
return result;
}
@@ -854,11 +867,12 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (ref_count) {
- prune_refs(refs, ref_count, ref_map);
+ prune_refs(refs, ref_count, ref_map, transport-url);
} else {
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr,
- ref_map);
+ ref_map,
+ transport-url);
}
}
free_refs(ref_map);
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..08a4841 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,16 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'fetch --prune prints the remotes full url' '
+ git branch goodbye
+ git clone . only-prunes
+ git branch -D goodbye
+ (
+ cd only-prunes
+ git fetch --prune origin 21 | head -n1 ../actual
+ )
+ echo From ${D}/. expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.2.194.g00457d4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2 2/2] fetch --prune: Run prune before fetching
When we have a remote-tracking branch named frotz/nitfol from a
previous fetch, and the upstream now has a branch named frotz. Prior
to this patch fetch would fail to remove frotz/nitfol with a git
fetch --prune from the upstream. git would inform the user to use git
remote prune to fix the problem.
This patch changes the way fetch --prune works by moving the pruning
operation before the fetching operation. Instead of warning the user of
a conflict, it autmatically fixes it.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e6dc2d6..ef3f0bb 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -855,11 +855,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -875,6 +870,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 08a4841..3e64af4 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -626,4 +626,18 @@ test_expect_success 'fetch --prune prints the remotes full
url' '
test_cmp expect actual
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.2.194.g00457d4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] builtin/fetch.c: Add pretty_url() and print_url()
In order to fix branchname DF conflicts during `fetch --prune`, the way
the header is output to the screen needs to be refactored. Here is an
exmaple of the output with the line in question denoted by '':
$ git fetch --prune --dry-run upstream
From https://github.com/git/git
a155a5f..5512ac5 maint - upstream/maint
d7aced9..7794a68 master - upstream/master
523f7c4..3e57c29 next - upstream/next
+ 462f102...0937cdf pu - upstream/pu (forced update)
e24105a..5d352bc todo - upstream/todo
* [new tag] v1.8.5.2 - v1.8.5.2
* [new tag] v1.8.5.2 - v1.8.5.2
pretty_url():
This function when passed a transport url will anonymize the transport
of the url. It will strip a trailing '/'. It will also strip a trailing
'.git'. It will return the newly formated url for use. I do not believe
there is a need for stripping the trailing '/' and '.git' from a url,
but it was already there and I wanted to make as little changes as
possible.
print_url():
This function will convert a transport url to a pretty url using
pretty_url(). Then it will print out the pretty url to stderr as
indicated above in the example output. It uses a global variable
named gshown_url' to prevent this header for being printed twice.
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 60 -
1 file changed, 42 insertions(+), 18 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 3d978eb..b3145f6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -44,6 +44,42 @@ static struct transport *gtransport;
static struct transport *gsecondary;
static const char *submodule_prefix = ;
static const char *recurse_submodules_default;
+static int gshown_url = 0;
+
+static char *pretty_url(const char *raw_url) {
+ if (raw_url) {
+ int url_len, i;
+ char *pretty_url, *url;
+
+ url = transport_anonymize_url(raw_url);
+
+ url_len = strlen(url);
+ for (i = url_len - 1; url[i] == '/' 0 = i; i--)
+ ;
+ url_len = i + 1;
+ if (4 i !strncmp(.git, url + i - 3, 4))
+ url_len = i - 3;
+
+ pretty_url = xcalloc(1, 1 + url_len);
+ memcpy(pretty_url, url, url_len);
+
+ free(url);
+ return pretty_url;
+ }
+ return xstrdup(foreign);
+}
+
+static void print_url(const char *raw_url) {
+ if (!gshown_url) {
+ char *url = pretty_url(raw_url);
+
+ fprintf(stderr, _(From %s\n), url);
+
+ gshown_url = 1;
+ free(url);
+ }
+}
+
static int option_parse_recurse_submodules(const struct option *opt,
const char *arg, int unset)
@@ -535,7 +571,7 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
{
FILE *fp;
struct commit *commit;
- int url_len, i, shown_url = 0, rc = 0;
+ int url_len, i, rc = 0;
struct strbuf note = STRBUF_INIT;
const char *what, *kind;
struct ref *rm;
@@ -546,10 +582,8 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
if (!fp)
return error(_(cannot open %s: %s\n), filename,
strerror(errno));
- if (raw_url)
- url = transport_anonymize_url(raw_url);
- else
- url = xstrdup(foreign);
+ url = pretty_url(raw_url);
+ url_len = strlen(url);
rm = ref_map;
if (check_everything_connected(iterate_ref_map, 0, rm)) {
@@ -606,13 +640,6 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
what = rm-name;
}
- url_len = strlen(url);
- for (i = url_len - 1; url[i] == '/' 0 = i; i--)
- ;
- url_len = i + 1;
- if (4 i !strncmp(.git, url + i - 3, 4))
- url_len = i - 3;
-
strbuf_reset(note);
if (*what) {
if (*kind)
@@ -651,13 +678,10 @@ static int store_updated_refs(const char *raw_url, const
char *remote_name,
REFCOL_WIDTH,
*what ? what : HEAD);
if (note.len) {
- if (verbosity = 0 !shown_url) {
- fprintf(stderr, _(From %.*s\n),
- url_len, url);
- shown_url = 1;
- }
- if (verbosity = 0
[PATCH 3/3] fetch --prune: Repair branchname DF conflicts
When a branchname DF conflict occurs during a fetch, --prune should
be able to fix it. When fetching with --prune, the fetching process
happens before pruning causing the branchname DF conflict to persist
and report an error. This patch prunes before fetching, thus
correcting DF conflicts during a fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e50b697..845c687 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded' '
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack 3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
+ git branch -D dir/file
+ git branch dir
+ (
+ cd prune-df-conflict
+ git fetch --prune
+ git rev-parse origin/dir ../actual
+ )
+ git rev-parse dir expect
+ test_cmp expect actual
+'
+
test_done
--
1.8.5.1.163.gd7aced9
--
To unsubscribe from this list: send the line 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] fetch --prune: Always print header url
If fetch --prune is run with no new refs to fetch, but it has refs
to prune. Then, the header url is not printed as it would if there were
new refs to fetch. the following is example output showing this
behavior:
$ git fetch --prune --dry-run origin
x [deleted] (none) - origin/world
After this patch the output of fetch --prune should look like this:
$ git fetch --prune --dry-run origin
From https://github.com/git/git
x [deleted] (none) - origin/test
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b3145f6..e50b697 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -732,7 +732,8 @@ static int fetch_refs(struct transport *transport, struct
ref *ref_map)
return ret;
}
-static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map)
+static int prune_refs(struct refspec *refs, int ref_count, struct ref *ref_map,
+ const char *raw_url)
{
int result = 0;
struct ref *ref, *stale_refs = get_stale_heads(refs, ref_count,
ref_map);
@@ -744,6 +745,7 @@ static int prune_refs(struct refspec *refs, int ref_count,
struct ref *ref_map)
if (!dry_run)
result |= delete_ref(ref-name, NULL, 0);
if (verbosity = 0) {
+ print_url(raw_url);
fprintf(stderr, x %-*s %-*s - %s\n,
TRANSPORT_SUMMARY(_([deleted])),
REFCOL_WIDTH, _((none)),
prettify_refname(ref-name));
@@ -878,11 +880,12 @@ static int do_fetch(struct transport *transport,
* don't care whether --tags was specified.
*/
if (ref_count) {
- prune_refs(refs, ref_count, ref_map);
+ prune_refs(refs, ref_count, ref_map, transport-url);
} else {
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr,
- ref_map);
+ ref_map,
+ transport-url);
}
}
free_refs(ref_map);
--
1.8.5.1.163.gd7aced9
--
To unsubscribe from this list: send the line 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] builtin/fetch.c: Add pretty_url() and print_url()
On Wed, Dec 18, 2013 at 3:47 PM, Junio C Hamano gits...@pobox.com wrote: Tom Miller jacker...@gmail.com writes: In order to fix branchname DF conflicts during `fetch --prune`, the way the header is output to the screen needs to be refactored. Here is an exmaple of the output with the line in question denoted by '': $ git fetch --prune --dry-run upstream From https://github.com/git/git a155a5f..5512ac5 maint - upstream/maint d7aced9..7794a68 master - upstream/master 523f7c4..3e57c29 next - upstream/next + 462f102...0937cdf pu - upstream/pu (forced update) e24105a..5d352bc todo - upstream/todo * [new tag] v1.8.5.2 - v1.8.5.2 * [new tag] v1.8.5.2 - v1.8.5.2 pretty_url(): This function when passed a transport url will anonymize the transport of the url. It will strip a trailing '/'. It will also strip a trailing '.git'. It will return the newly formated url for use. I do not believe there is a need for stripping the trailing '/' and '.git' from a url, but it was already there and I wanted to make as little changes as possible. OK. I tend to agree that stripping the trailing part is probably not a good idea and we would want to remove that but that definitely should be done as a separate step, or even as a separate series on top of this one. I think that removing the trailing part will greatly reduce the complexity to the point were it is unnecessary to have pretty_url(). My goal with extracting this function is to isolate the complexity of formatting the url to a single spot. I am thinking along the lines of the following commit order: 1. Remove the remove trailing part 2. Add print_url() 3. Always print url when pruning 4. Reverse order of prune and fetch print_url(): This function will convert a transport url to a pretty url using pretty_url(). Then it will print out the pretty url to stderr as indicated above in the example output. It uses a global variable named gshown_url' to prevent this header for being printed twice. Gaah. What is that 'g' doing there? Please don't do that meaningless naming. I am not familiar with C conventions and I was trying to stay consistent. I saw other global variables starting with 'g' and made an assumption. It will use the original name in the upcoming patches. I do not think the change to introduce such a global variable belongs to this refactoring step. The current caller can decide itself if it called that function, and if you are going to introduce new callers in later steps, they can coordinate among themselves, no? I agree, there is no reason for introducing it in this step. Thanks for pointing that out. -- To unsubscribe from this list: send the line 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] fetch --prune: Repair branchname DF conflicts
On Wed, Dec 18, 2013 at 3:54 PM, Junio C Hamano gits...@pobox.com wrote:
Tom Miller jacker...@gmail.com writes:
When a branchname DF conflict occurs during a fetch,
You may have started with a specific case in which you want to
change the behaviour of current Git, so it may be clear what you
meant by branchname DF conflict, but that is true for nobody other
than you who will read this log message. Introducing new lingo is
OK as long as it is necessary, but in a case like this, where you
have to describe what situation you are trying to address anyway,
I do not think you need to add a new word to our vocabulary.
When we have a remote-tracking branch frotz/nitfol from a
previous fetch, and the upstream now has branch frotz, we
used to fail to remove frotz/nitfol and recreate frotz with
git fetch --prune from the upstream.
or something like that?
I did not intend to introduce new lingo. I did some searching through
history to see if something like this had been worked on before and
I found a commit by Jeff King that introduced me the the idea of
DF conflicts
commit fa250759794ab98e6edfbbf2f6aa2cb912e535eb
Author: Jeff King p...@peff.net
Date: Mon May 25 06:40:54 2009 -0400
fetch: report ref storage DF errors more accurately
When we fail to store a fetched ref, we recommend that the
user try running git prune to remove up any old refs that
have been deleted by the remote, which would clear up any DF
conflicts. However, ref storage might fail for other
reasons (e.g., permissions problems) in which case the
advice is useless and misleading.
This patch detects when there is an actual DF situation and
only issues the advice when one is found.
Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
I have no issue with rewording the it to be more clear and to try to
remove any new lingo.
But what should happen when we do not give --prune to git fetch in
such a situation? Should it fail, because we still have frotz/nitfol
and we cannot create frotz without losing it?
You talk about this to some extent in an email from 2009. I have linked
it below for your review.
http://article.gmane.org/gmane.comp.version-control.git/132276
In my opinion, if I supply --prune to fetch I expect it to be
destructive. It should be noted that the reflog can *not* be used
to recover pruned branches from a remote.
--prune should
be able to fix it. When fetching with --prune, the fetching process
happens before pruning causing the branchname DF conflict to persist
and report an error. This patch prunes before fetching, thus
correcting DF conflicts during a fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
Tested-by: Thomas Rast t...@thomasrast.ch
I wasn't following previous threads closely (was there a previous
thread???); has this iteration been already tested by trast?
There was a previous thread, but I was just looking for feed back on this
as a WIP. Should I have replied to it with this patchset?
Here is a link to the previous thread.
http://thread.gmane.org/gmane.comp.version-control.git/238530
The commit below should be the same patch he tested. The test was added
by him, and I made it part of this commit. Did I do this wrong?
---
builtin/fetch.c | 10 +-
t/t5510-fetch.sh | 14 ++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index e50b697..845c687 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -868,11 +868,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* We only prune based on refspecs specified
@@ -888,6 +883,11 @@ static int do_fetch(struct transport *transport,
transport-url);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 5d4581d..a981125 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -614,4 +614,18 @@ test_expect_success 'all boundary commits are excluded'
'
test_bundle_object_count .git/objects/pack/pack-${pack##pack}.pack
3
'
+test_expect_success 'branchname D/F conflict resolved by --prune' '
+ git branch dir/file
+ git clone . prune-df-conflict
[PATCH/WIP] Repair DF conflicts during fetch.
I encountered a directory/file conflict when running `git fetch --prune origin`. I figured passing --prune would automatically fix DF conflicts. After looking in the code I found that prune is called after fetching. It seemed to be intentional according historical commits. I made this patch to change it, which seems to work as I expected it to. This patch doesn't have any tests and it breaks the output when it does prune branches. I'm looking for guidance to help with fixing the broken output. I tried to figure out a way to do it on my own but I realize that I don't have the expertise with the codebase or C. Thanks, for any help that I may recieve in advaned this is my first time posting. If I have submitted this wrong I applogize and look forward to any advice that I may recieve in correcting my mistakes. Tom Miller (1): Repair DF conflicts during fetch. builtin/fetch.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) -- 1.8.5.rc3.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/WIP] Repair DF conflicts during fetch.
When a DF conflict occurs during a fetch, --prune should be able to fix
it. When fetching with --prune, the fetching process happens before
pruning causing the DF conflict to persist and report an error. This
patch prunes before fetching, thus correcting DF conflicts during a
fetch.
Signed-off-by: Tom Miller jacker...@gmail.com
---
builtin/fetch.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index bd7a101..f7959d0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -824,11 +824,6 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT autotags)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, 1);
- if (fetch_refs(transport, ref_map)) {
- free_refs(ref_map);
- retcode = 1;
- goto cleanup;
- }
if (prune) {
/*
* If --tags was specified, pretend that the user gave us
@@ -857,6 +852,11 @@ static int do_fetch(struct transport *transport,
prune_refs(transport-remote-fetch,
transport-remote-fetch_refspec_nr, ref_map);
}
}
+ if (fetch_refs(transport, ref_map)) {
+ free_refs(ref_map);
+ retcode = 1;
+ goto cleanup;
+ }
free_refs(ref_map);
/* if neither --no-tags nor --tags was specified, do automated tag
--
1.8.5.rc3.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
