Re: [PATCHv2 1/8] t2024: Add tests verifying current DWIM behavior of 'git checkout branch'
On Sat, Apr 20, 2013 at 10:44 PM, Jonathan Nieder jrnie...@gmail.com wrote: Johan Herland wrote: The DWIM mode of checkout allows you to run git checkout foo when there is no existing local ref or path called foo and there is exactly one remote with a remote-tracking branch called foo. Thanks for testing this. I'm surprised no one suggested a test since v1.7.0-rc0~51^2~6 (2009-10-18). Maybe it would also be worthwhile to also test --no-guess? (c.f. 46148dd7, 2009-10-18) [...] Sane? Yes. Thanks! Will incorporate your suggestions into the next iteration. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFD/PATCH 0/5] Improving the search for remote-tracking branches
Hi, The following patch series has three uncontroversial patches (I hope) to improve the DWIM behavivor git checkout foo, followed by two more experimental patches meant to trigger discussion about how Git decides on what is a remote-tracking branch, and what is not: The first three patches concern the DWIMming of git checkout foo into git checkout -b foo --track refs/remotes/$remote/foo. This DWIMming depends on finding exactly _one_ remote $remote having a remote- tracking branch called foo. Currently we find remote-tracking branches by pattern-matching refs against refs/remotes/*/foo, but instead we should look at the configured refspecs to find the real set of remote- tracking branch candidates. This fixes the DWIM behavior when the user has tweaked refspecs to place remote-tracking branches elsewhere (e.g. refs/remotes/$remote/heads/foo). The argument is explained more closely in patch #3. The remaining two patches tries to apply the same argument to the -t/--track argument to checkout/branch: This option takes a remote- tracking branch (or a local branch) as argument, but validating this argument is currently done by a simple match against refs/remotes/* (or refs/heads/*). By the same reasoning as above, we should instead look at the configured refspecs to determine whether the argument is a valid remote-tracking branch. However, as shown in the final patch, this would break a few tests where we currently try to --track refs that are not proper remote-tracking branches (in that they do not correspond to a refspec of some remote). - Should we simply fix these tests (t3200.39, t7201.24, t9114.*), and _require_ that all remote-tracking branches must have corresponding refspecs configured? - Are there valid real-world use cases that would be broken by such a requirement? - Is it preferable to stop after patch #4, and accept the superset of refs/remotes/* and refspec-matching refs as valid remote-tracking branches? Have fun! :) ...Johan Johan Herland (5): t2024: Add tests verifying current DWIM behavior of 'git checkout branch' t2024: Show failure to use refspec when DWIMming remote branch names checkout: Use remote refspecs when DWIMming tracking branches branch.c: Look up refspecs to validate tracking branches RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream Documentation/git-checkout.txt | 6 +- branch.c | 17 +- builtin/checkout.c | 42 +++--- t/t2024-checkout-dwim.sh | 116 +++ t/t3200-branch.sh| 2 +- t/t7201-co.sh| 2 +- t/t9114-git-svn-dcommit-merge.sh | 8 +-- 7 files changed, 163 insertions(+), 30 deletions(-) create mode 100755 t/t2024-checkout-dwim.sh -- 1.8.1.3.704.g33f7d4f -- 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
[RFD/PATCH 1/5] t2024: Add tests verifying current DWIM behavior of 'git checkout branch'
The DWIM mode of checkout allows you to run git checkout foo when there is
no existing local ref or path called foo, and there is exactly one remote
with a remote-tracking branch called foo. Git will then automatically
create a new local branch called foo using the remote-tracking foo as
its starting point and configured upstream.
Signed-off-by: Johan Herland jo...@herland.net
---
t/t2024-checkout-dwim.sh | 66
1 file changed, 66 insertions(+)
create mode 100755 t/t2024-checkout-dwim.sh
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
new file mode 100755
index 000..5650d21
--- /dev/null
+++ b/t/t2024-checkout-dwim.sh
@@ -0,0 +1,66 @@
+#!/bin/sh
+
+test_description='checkout branch
+
+Ensures that checkout on an unborn branch does what the user expects'
+
+. ./test-lib.sh
+
+# Arguments: branch remote remote-tracking
+#
+# Verify that we have checked out branch, and that it is at the same
+# commit as remote-tracking, and that has appropriate tracking config
+# setup against remote
+test_tracking_branch() {
+ branch=$1
+ remote=$2
+ remote_track=$3
+ test refs/heads/$branch = $(git rev-parse --symbolic-full-name
HEAD)
+ test $(git rev-parse --verify HEAD) = $(git rev-parse --verify
$remote_track)
+ test $remote = $(git config branch.$branch.remote)
+ test refs/heads/$branch = $(git config branch.$branch.merge)
+}
+
+test_expect_success 'setup' '
+ (git init repo_a
+cd repo_a
+test_commit a_master
+git checkout -b foo
+test_commit a_foo
+git checkout -b bar
+test_commit a_bar
+ )
+ (git init repo_b
+cd repo_b
+test_commit b_master
+git checkout -b foo
+test_commit b_foo
+git checkout -b baz
+test_commit b_baz
+ )
+ git remote add repo_a repo_a
+ git remote add repo_b repo_b
+ git config remote.repo_b.fetch \
+ +refs/heads/*:refs/remotes/other_b/*
+ git fetch --all
+'
+
+test_expect_success 'checkout of non-existing branch fails' '
+ test_must_fail git checkout xyzzy
+'
+
+test_expect_success 'checkout of branch from multiple remotes fails' '
+ test_must_fail git checkout foo
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #1' '
+ git checkout bar
+ test_tracking_branch bar repo_a refs/remotes/repo_a/bar
+'
+
+test_expect_success 'checkout of branch from a single remote succeeds #2' '
+ git checkout baz
+ test_tracking_branch baz repo_b refs/remotes/other_b/baz
+'
+
+test_done
--
1.8.1.3.704.g33f7d4f
--
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
[RFD/PATCH 2/5] t2024: Show failure to use refspec when DWIMming remote branch names
When using git checkout foo to DWIM the creation of local foo from some
existing upstream foo, we assume conventional refspecs as created by git
clone or git remote add, and fail to work correctly if the current
refspecs do not follow the conventional refs/remotes/$remote/* pattern.
Signed-off-by: Johan Herland jo...@herland.net
---
t/t2024-checkout-dwim.sh | 52 +++-
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index 5650d21..36bf52f 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -22,6 +22,7 @@ test_tracking_branch() {
}
test_expect_success 'setup' '
+ test_commit my_master
(git init repo_a
cd repo_a
test_commit a_master
@@ -49,7 +50,7 @@ test_expect_success 'checkout of non-existing branch fails' '
test_must_fail git checkout xyzzy
'
-test_expect_success 'checkout of branch from multiple remotes fails' '
+test_expect_success 'checkout of branch from multiple remotes fails #1' '
test_must_fail git checkout foo
'
@@ -63,4 +64,53 @@ test_expect_success 'checkout of branch from a single remote
succeeds #2' '
test_tracking_branch baz repo_b refs/remotes/other_b/baz
'
+test_expect_success 'setup more remotes with unconventional refspecs' '
+ git checkout master
+ git branch -D bar
+ git branch -D baz
+ test $(git rev-parse --verify HEAD) = $(git rev-parse --verify
my_master)
+ (git init repo_c
+cd repo_c
+test_commit c_master
+git checkout -b bar
+test_commit c_bar
+git checkout -b spam
+test_commit c_spam
+ )
+ (git init repo_d
+cd repo_d
+test_commit d_master
+git checkout -b baz
+test_commit f_baz
+git checkout -b eggs
+test_commit c_eggs
+ )
+ git remote add repo_c repo_c
+ git config remote.repo_c.fetch \
+ +refs/heads/*:refs/remotes/extra_dir/repo_c/extra_dir/*
+ git fetch repo_c
+ git remote add repo_d repo_d
+ git config remote.repo_d.fetch \
+ +refs/heads/*:refs/repo_d/*
+ git fetch repo_d
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #2' '
+ test_must_fail git checkout bar
+'
+
+test_expect_failure 'checkout of branch from multiple remotes fails #3' '
+ test_must_fail git checkout baz
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #3' '
+ git checkout spam
+ test_tracking_branch spam repo_c
refs/remotes/extra_dir/repo_c/extra_dir/spam
+'
+
+test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+ git checkout eggs
+ test_tracking_branch eggs repo_d refs/repo_d/eggs
+'
+
test_done
--
1.8.1.3.704.g33f7d4f
--
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
[RFD/PATCH 3/5] checkout: Use remote refspecs when DWIMming tracking branches
The DWIM mode of checkout allows you to run git checkout foo when there is
no existing local ref or path called foo, and there is exactly one remote
with a remote-tracking branch called foo. Git will then automatically
create a new local branch called foo using the remote-tracking foo as
its starting point and configured upstream.
However, the current code hardcodes the assumption that all remote-tracking
branches are located at refs/remotes/$remote/*, and that git checkout foo
must find exactly one ref matching refs/remotes/*/foo to succeed.
This approach fails if a user has customized the refspec of a given remote to
place remote-tracking branches elsewhere.
The better way to find a tracking branch is to use the fetch refspecs for the
configured remotes to deduce the available candidate remote-tracking branches
corresponding to a remote branch of the requested name, and if exactly one of
these exists (and points to a valid SHA1), then that is the remote-tracking
branch we will use.
For example, in the case of git checkout foo, we map refs/heads/foo
through each remote's refspec to find the available candidate remote-tracking
branches, and if exactly one of these candidates exist in our local repo, then
we have found the upstream for the new local branch foo.
This fixes most of the failing tests introduced in the previous patch.
Signed-off-by: Johan Herland jo...@herland.net
---
Documentation/git-checkout.txt | 6 +++---
builtin/checkout.c | 42 ++
t/t2024-checkout-dwim.sh | 6 +++---
3 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 8edcdca..bf0c99c 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -131,9 +131,9 @@ entries; instead, unmerged entries are ignored.
--track in linkgit:git-branch[1] for details.
+
If no '-b' option is given, the name of the new branch will be
-derived from the remote-tracking branch. If remotes/ or refs/remotes/
-is prefixed it is stripped away, and then the part up to the
-next slash (which would be the nickname of the remote) is removed.
+derived from the remote-tracking branch, by looking at the local part of
+the refspec configured for the corresponding remote, and then stripping
+the initial part up to the *.
This would tell us to use hack as the local branch when branching
off of origin/hack (or remotes/origin/hack, or even
refs/remotes/origin/hack). If the given name has no slash, or the above
diff --git a/builtin/checkout.c b/builtin/checkout.c
index f8033f4..d6f9c01 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -822,38 +822,40 @@ static int git_checkout_config(const char *var, const
char *value, void *cb)
}
struct tracking_name_data {
- const char *name;
- char *remote;
+ /* const */ char *src_ref;
+ char *dst_ref;
+ unsigned char *dst_sha1;
int unique;
};
-static int check_tracking_name(const char *refname, const unsigned char *sha1,
- int flags, void *cb_data)
+static int check_tracking_name(struct remote *remote, void *cb_data)
{
struct tracking_name_data *cb = cb_data;
- const char *slash;
-
- if (prefixcmp(refname, refs/remotes/))
- return 0;
- slash = strchr(refname + 13, '/');
- if (!slash || strcmp(slash + 1, cb-name))
+ struct refspec query;
+ memset(query, 0, sizeof(struct refspec));
+ query.src = cb-src_ref;
+ if (remote_find_tracking(remote, query) ||
+ get_sha1(query.dst, cb-dst_sha1))
return 0;
- if (cb-remote) {
+ if (cb-dst_ref) {
cb-unique = 0;
return 0;
}
- cb-remote = xstrdup(refname);
+ cb-dst_ref = xstrdup(query.dst);
return 0;
}
-static const char *unique_tracking_name(const char *name)
+static const char *unique_tracking_name(const char *name, unsigned char *sha1)
{
- struct tracking_name_data cb_data = { NULL, NULL, 1 };
- cb_data.name = name;
- for_each_ref(check_tracking_name, cb_data);
+ struct tracking_name_data cb_data = { NULL, NULL, NULL, 1 };
+ char src_ref[PATH_MAX];
+ snprintf(src_ref, PATH_MAX, refs/heads/%s, name);
+ cb_data.src_ref = src_ref;
+ cb_data.dst_sha1 = sha1;
+ for_each_remote(check_tracking_name, cb_data);
if (cb_data.unique)
- return cb_data.remote;
- free(cb_data.remote);
+ return cb_data.dst_ref;
+ free(cb_data.dst_ref);
return NULL;
}
@@ -916,8 +918,8 @@ static int parse_branchname_arg(int argc, const char **argv,
if (dwim_new_local_branch_ok
!check_filename(NULL, arg)
argc == 1) {
- const char *remote = unique_tracking_name(arg);
- if (!remote
[RFD/PATCH 4/5] branch.c: Look up refspecs to validate tracking branches
The current code for validating tracking branches (e.g. the argument to the
-t/--track option) hardcodes refs/heads/* and refs/remotes/* as the potential
locations for tracking branches. This works well with the conventional
refspecs created by git clone or git remote add, but does not work if the
user tweaks the refspecs to place remote-tracking branches outside
refs/remotes/*.
This patch adds explicit checking of the refspecs for each remote to determine
whether a candidate tracking branch is indeed a valid remote-tracking branch,
even if placed outside refs/heads/* and refs/remotes/*.
This new check is added as a fallback after checking for match against
refs/heads/* and refs/remotes/*, so the code will not be run in the common
case.
This patch also fixes the last remaining test failure in t2024-checkout-dwim.
Signed-off-by: Johan Herland jo...@herland.net
---
branch.c | 18 +-
t/t2024-checkout-dwim.sh | 2 +-
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/branch.c b/branch.c
index 6ae6a4c..c9f9dec 100644
--- a/branch.c
+++ b/branch.c
@@ -197,6 +197,21 @@ int validate_new_branchname(const char *name, struct
strbuf *ref,
return 1;
}
+static int check_tracking_branch(struct remote *remote, void *cb_data)
+{
+ char *tracking_branch = cb_data;
+ struct refspec query;
+ memset(query, 0, sizeof(struct refspec));
+ query.dst = tracking_branch;
+ return !(remote_find_tracking(remote, query) ||
+prefixcmp(query.src, refs/heads/));
+}
+
+static int validate_remote_tracking_branch(char *ref)
+{
+ return !for_each_remote(check_tracking_branch, ref);
+}
+
static const char upstream_not_branch[] =
N_(Cannot setup tracking information; starting point '%s' is not a branch.);
static const char upstream_missing[] =
@@ -259,7 +274,8 @@ void create_branch(const char *head,
case 1:
/* Unique completion -- good, only if it is a real branch */
if (prefixcmp(real_ref, refs/heads/)
- prefixcmp(real_ref, refs/remotes/)) {
+ prefixcmp(real_ref, refs/remotes/)
+ validate_remote_tracking_branch(real_ref)) {
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
else
diff --git a/t/t2024-checkout-dwim.sh b/t/t2024-checkout-dwim.sh
index fc6edc9..a8f0a90 100755
--- a/t/t2024-checkout-dwim.sh
+++ b/t/t2024-checkout-dwim.sh
@@ -108,7 +108,7 @@ test_expect_success 'checkout of branch from a single
remote succeeds #3' '
test_tracking_branch spam repo_c
refs/remotes/extra_dir/repo_c/extra_dir/spam
'
-test_expect_failure 'checkout of branch from a single remote succeeds #4' '
+test_expect_success 'checkout of branch from a single remote succeeds #4' '
git checkout eggs
test_tracking_branch eggs repo_d refs/repo_d/eggs
'
--
1.8.1.3.704.g33f7d4f
--
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
[RFD/PATCH 5/5] RFD: Disallow out-of-refspec refs within refs/remotes/* to be used as upstream
The previous patch adds validation of upstream remote-tracking branches by
parsing the configured refspecs, and making sure that the candidate upstream
(if not already matching refs/heads/* or refs/remotes/*) is indeed a
remote-tracking branch according to some remote's refspec. For a
default/conventional setup, this check would automatically also cover
everything within refs/remotes/*, meaning that the preceding check for
refs/remotes/* is redundant (although quicker than the validation against
refspecs). One could also argue that not everything inside refs/remotes/*
should be automatically acceptable as an upstream, if one were to keep
other (non-branch) type of remote-tracking refs there.
This patch removes the simple check for refs/remotes/*, to make sure that
_only_ validated remote-tracking branches (in addition to local branches)
are allowed as upstreams.
However, this means that for unconventional setups that place refs within
refs/remotes/* without configuring a corresponding refspec, those refs will
no longer be usable as upstreams. This breaks a few existing tests, which
are marked as test_expect_failure by this patch, to make them easy to find.
---
branch.c | 1 -
t/t3200-branch.sh| 2 +-
t/t7201-co.sh| 2 +-
t/t9114-git-svn-dcommit-merge.sh | 8
4 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/branch.c b/branch.c
index c9f9dec..beaf11d 100644
--- a/branch.c
+++ b/branch.c
@@ -274,7 +274,6 @@ void create_branch(const char *head,
case 1:
/* Unique completion -- good, only if it is a real branch */
if (prefixcmp(real_ref, refs/heads/)
- prefixcmp(real_ref, refs/remotes/)
validate_remote_tracking_branch(real_ref)) {
if (explicit_tracking)
die(_(upstream_not_branch), start_name);
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index d969f0e..41d293d 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -317,7 +317,7 @@ test_expect_success 'test tracking setup (non-wildcard,
matching)' '
test $(git config branch.my4.merge) = refs/heads/master
'
-test_expect_success 'test tracking setup (non-wildcard, not matching)' '
+test_expect_failure 'test tracking setup (non-wildcard, not matching)' '
git config remote.local.url .
git config remote.local.fetch refs/heads/s:refs/remotes/local/s
(git show-ref -q refs/remotes/local/master || git fetch local)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index be9672e..7267ee2 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -429,7 +429,7 @@ test_expect_success 'detach a symbolic link HEAD' '
test z$(git rev-parse --verify refs/heads/master) = z$here
'
-test_expect_success \
+test_expect_failure \
'checkout with --track fakes a sensible -b name' '
git update-ref refs/remotes/origin/koala/bear renamer
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index 3077851..ef274fd 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -45,7 +45,7 @@ test_expect_success 'setup svn repository' '
)
'
-test_expect_success 'setup git mirror and merge' '
+test_expect_failure 'setup git mirror and merge' '
git svn init $svnrepo -t tags -T trunk -b branches
git svn fetch
git checkout --track -b svn remotes/trunk
@@ -67,7 +67,7 @@ test_expect_success 'setup git mirror and merge' '
test_debug 'gitk --all sleep 1'
-test_expect_success 'verify pre-merge ancestry'
+test_expect_failure 'verify pre-merge ancestry'
test x\`git rev-parse --verify refs/heads/svn^2\` = \
x\`git rev-parse --verify refs/heads/merge\`
git cat-file commit refs/heads/svn^ | grep '^friend$'
@@ -79,7 +79,7 @@ test_expect_success 'git svn dcommit merges'
test_debug 'gitk --all sleep 1'
-test_expect_success 'verify post-merge ancestry'
+test_expect_failure 'verify post-merge ancestry'
test x\`git rev-parse --verify refs/heads/svn\` = \
x\`git rev-parse --verify refs/remotes/trunk \`
test x\`git rev-parse --verify refs/heads/svn^2\` = \
@@ -87,7 +87,7 @@ test_expect_success 'verify post-merge ancestry'
git cat-file commit refs/heads/svn^ | grep '^friend$'
-test_expect_success 'verify merge commit message'
+test_expect_failure 'verify merge commit message'
git rev-list --pretty=raw -1 refs/heads/svn | \
grep \Merge branch 'merge' into svn\
--
1.8.1.3.704.g33f7d4f
--
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: put THEIR commits AFTER my commits with a single rebase command
On Thu, Apr 18, 2013 at 7:18 AM, Ilya Basin basini...@gmail.com wrote: I asked this on stackoverflow, but no reply. http://stackoverflow.com/questions/15971244/git-put-their-commits-after-my-commits-with-a-single-rebase-command Suppose master and origin/master diverged. I'm on master and I want to put the commits from origin/master after my commits and then push --force. A---B---C origin/master / D---E---F---G *master desired result: A---B---C origin/master / D---E---F---G---A'---B'---C' *master Note that if other people are working on top of origin/master, then what you are proposing is quite rude to them, since they must now manually rebase their own work on top of your rebased history. Rewriting public history is generally considered evil. Variant 1: git branch -f tmp git reset --hard origin/master git rebase tmp This variant is bad, because 'git reset --hard' checks out some files and 'git rebase' rewrites them again before applying commits. It's a redundant job. Variant 2: git branch -f tmp origin/master git rebase --onto master master tmp git branch -f master git checkout master Too many commands. I want to do this with just one command. And I want to stay be on branch master in case of rebase conflicts. git cherry-pick master..origin/master ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Revert graph.c: mark private file-scope symbols as static
On Mon, Mar 4, 2013 at 1:03 AM, John Keeping j...@keeping.me.uk wrote: This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb. CGit uses these symbols to output the correct HTML around graph elements. Making these symbols private means that CGit cannot be updated to use Git 1.8.0 or newer, so let's not do that. On top of the revert, also add comments so that we avoid reintroducing this problem in the future and suggest to those modifying this API that they might want to discuss it with the CGit developers. Signed-off-by: John Keeping j...@keeping.me.uk Still-Acked-by: Johan Herland jo...@herland.net ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Revert graph.c: mark private file-scope symbols as static
On Sat, Mar 2, 2013 at 1:46 PM, John Keeping j...@keeping.me.uk wrote: This reverts commit ba35480439d05b8f6cca50527072194fe3278bbb. CGit uses these symbols to output the correct HTML around graph elements. Making these symbols private means that CGit cannot be updated to use Git 1.8.0 or newer, so let's not do that. Signed-off-by: John Keeping j...@keeping.me.uk --- I realise that Git isn't a library so making the API useful for outside projects isn't a priority, but making these two methods public makes life a lot easier for CGit. Additionally, it seems that Johan added graph_set_column_colors specifically so that CGit should use it - there's no value to having that as a method just for its use in graph.c and he was the author of CGit commit 268b34a (ui-log: Colorize commit graph, 2010-11-15). Correct, Acked-by: Johan Herland jo...@herland.net Have fun! :) ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git Notes - Search Functionality
On Wed, Feb 27, 2013 at 4:34 PM, krishna chaitanya kurnala kkc...@gmail.com wrote: Hello All I working on Git Notes. I want to know if there is an easy way to obtain a list of all namespaces(For eg., git notes --ref=namespace ... ) with notes objects in a specific git repository. The notes namespaces are stored as regular refs under refs/notes/*. Here's one way to list the available namespaces: git for-each-ref refs/notes/ Hope this helps, ...Johan We can easily create, edit, merge git notes with commands if we know the namespaces and/or the sha. But, for example, Has anyone tried to search for a string in a git notes objects for that project etc? The closest i can think of is using some options with git logs, for example, git log --show-notes=* --format=%H %N etc. Appreciate your time. thanks Krishna Chaitanya -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/5] Use string_lists when processing notes
On Tue, Nov 6, 2012 at 8:56 AM, Michael Haggerty mhag...@alum.mit.edu wrote: On 11/04/2012 10:05 PM, Johan Herland wrote: On Sun, Nov 4, 2012 at 8:07 AM, Michael Haggerty mhag...@alum.mit.edu wrote: This simplifies the code. Also, sort lines all at once (O(N lg N)) rather than insertion sorting as lines are processed (O(N^2)) and fix the handling of empty values in GIT_NOTES_DISPLAY_REF and GIT_NOTES_REWRITE_REF. Michael Haggerty (5): string_list: add a function string_list_remove_empty_items() Initialize sort_uniq_list using named constant combine_notes_cat_sort_uniq(): sort and dedup lines all at once notes: fix handling of colon-separated values string_list_add_refs_from_colon_sep(): use string_list_split() Series looks good to me. Acked-by: Johan Herland jo...@herland.net Thanks for reviewing the series. Was it intentional that you didn't CC the mailing list? No, not at all. Sorry. Here we go again, with the appropriate CC: Acked-by: Johan Herland jo...@herland.net Have fun! :) ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Fix remaining issue when deleting symrefs
The following two patches are based on rs/branch-del-symref, and fixes the remaining failure to delete a packed ref through a symref. The first patch demonstrates the bug with a testcase, and the second patch fixes the bug. Feel free to squash the two patches into one, if you prefer to keep both the testcase and subsequent fix in a single commit. Have fun! :) ...Johan Johan Herland (2): t1400-update-ref: Add test verifying bug with symrefs in delete_ref() Fix failure to delete a packed ref through a symref refs.c| 2 +- t/t1400-update-ref.sh | 18 ++ 2 files changed, 19 insertions(+), 1 deletion(-) -- 1.7.12.1.609.g5cd6968 -- 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/2] t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD' to delete refs/heads/master), we currently fail to remove the packed version of that ref. This testcase demonstrates the bug. Signed-off-by: Johan Herland jo...@herland.net --- t/t1400-update-ref.sh | 18 ++ 1 file changed, 18 insertions(+) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 4fd83a6..f7ec203 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -74,6 +74,24 @@ test_expect_success delete $m (by HEAD) ' ' rm -f .git/$m +test_expect_success \ + create $m (by HEAD) \ + git update-ref HEAD $A +test $A' = $(cat .git/'$m')' +test_expect_success \ + pack refs \ + git pack-refs --all +test_expect_success \ + move $m (by HEAD) \ + git update-ref HEAD $B $A +test $B' = $(cat .git/'$m')' +test_expect_failure delete $m (by HEAD) should remove both packed and loose $m ' + git update-ref -d HEAD $B + ! grep $m .git/packed-refs + ! test -f .git/$m +' +rm -f .git/$m + cp -f .git/HEAD .git/HEAD.orig test_expect_success delete symref without dereference ' git update-ref --no-deref -d HEAD -- 1.7.12.1.609.g5cd6968 -- 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/2] Fix failure to delete a packed ref through a symref
When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD' to delete refs/heads/master), we would remove the loose ref, but a packed version of the same ref would remain, the end result being that instead of deleting refs/heads/master we would appear to reset it to its state as of the last repack. This patch fixes the issue, by making sure we pass the correct ref name when invoking repack_without_ref() from within delete_ref(). Signed-off-by: Johan Herland jo...@herland.net --- refs.c| 2 +- t/t1400-update-ref.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/refs.c b/refs.c index 726c53c..6cec1c8 100644 --- a/refs.c +++ b/refs.c @@ -1779,7 +1779,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) * packed one. Also, if it was not loose we need to repack * without it. */ - ret |= repack_without_ref(refname); + ret |= repack_without_ref(lock-ref_name); unlink_or_warn(git_path(logs/%s, lock-ref_name)); invalidate_ref_cache(NULL); diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index f7ec203..e415ee0 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -85,7 +85,7 @@ test_expect_success \ move $m (by HEAD) \ git update-ref HEAD $B $A test $B' = $(cat .git/'$m')' -test_expect_failure delete $m (by HEAD) should remove both packed and loose $m ' +test_expect_success delete $m (by HEAD) should remove both packed and loose $m ' git update-ref -d HEAD $B ! grep $m .git/packed-refs ! test -f .git/$m -- 1.7.12.1.609.g5cd6968 -- 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
[RFC/PATCH] git symbolic-ref --delete $symref
Teach symbolic-ref to delete symrefs by adding the -d/--delete option to
git-symbolic-ref. Both proper and dangling symrefs are deleted by this
option, but other refs - or anything else that is not a symref - is not.
The symref deletion is performed by first verifying that we are given a
proper symref, and then invoking delete_ref() on it with the REF_NODEREF
flag.
Signed-off-by: Johan Herland jo...@herland.net
---
Here is a quick first attempt at implementing the 'git symbolic-ref -d'
you've been mentioning in the symref deletion thread.
Documentation/git-symbolic-ref.txt | 10 +-
builtin/symbolic-ref.c | 33 +++--
t/t1401-symbolic-ref.sh| 30 ++
3 files changed, 62 insertions(+), 11 deletions(-)
diff --git a/Documentation/git-symbolic-ref.txt
b/Documentation/git-symbolic-ref.txt
index 981d3a8..ef68ad2 100644
--- a/Documentation/git-symbolic-ref.txt
+++ b/Documentation/git-symbolic-ref.txt
@@ -3,13 +3,14 @@ git-symbolic-ref(1)
NAME
-git-symbolic-ref - Read and modify symbolic refs
+git-symbolic-ref - Read, modify and delete symbolic refs
SYNOPSIS
[verse]
'git symbolic-ref' [-m reason] name ref
'git symbolic-ref' [-q] [--short] name
+'git symbolic-ref' --delete [-q] name
DESCRIPTION
---
@@ -21,6 +22,9 @@ argument to see which branch your working tree is on.
Given two arguments, creates or updates a symbolic ref name to
point at the given branch ref.
+Given `--delete` and an additional argument, deletes the given
+symbolic ref.
+
A symbolic ref is a regular file that stores a string that
begins with `ref: refs/`. For example, your `.git/HEAD` is
a regular file whose contents is `ref: refs/heads/master`.
@@ -28,6 +32,10 @@ a regular file whose contents is `ref: refs/heads/master`.
OPTIONS
---
+-d::
+--delete::
+ Delete the symbolic ref name.
+
-q::
--quiet::
Do not issue an error message if the name is not a
diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c
index 9e92828..f481959 100644
--- a/builtin/symbolic-ref.c
+++ b/builtin/symbolic-ref.c
@@ -5,12 +5,11 @@
static const char * const git_symbolic_ref_usage[] = {
N_(git symbolic-ref [options] name [ref]),
+ N_(git symbolic-ref -d [-q] name),
NULL
};
-static int shorten;
-
-static void check_symref(const char *HEAD, int quiet)
+static int check_symref(const char *HEAD, int quiet, int shorten, int print)
{
unsigned char sha1[20];
int flag;
@@ -22,20 +21,24 @@ static void check_symref(const char *HEAD, int quiet)
if (!quiet)
die(ref %s is not a symbolic ref, HEAD);
else
- exit(1);
+ return 1;
+ }
+ if (print) {
+ if (shorten)
+ refname = shorten_unambiguous_ref(refname, 0);
+ puts(refname);
}
- if (shorten)
- refname = shorten_unambiguous_ref(refname, 0);
- puts(refname);
+ return 0;
}
int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
{
- int quiet = 0;
+ int quiet = 0, delete = 0, shorten = 0, ret = 0;
const char *msg = NULL;
struct option options[] = {
OPT__QUIET(quiet,
N_(suppress error message for non-symbolic (detached)
refs)),
+ OPT_BOOL('d', delete, delete, N_(delete symbolic ref)),
OPT_BOOL(0, short, shorten, N_(shorten ref output)),
OPT_STRING('m', NULL, msg, N_(reason), N_(reason of the
update)),
OPT_END(),
@@ -46,9 +49,19 @@ int cmd_symbolic_ref(int argc, const char **argv, const char
*prefix)
git_symbolic_ref_usage, 0);
if (msg !*msg)
die(Refusing to perform update with empty message);
+
+ if (delete) {
+ if (argc != 1)
+ usage_with_options(git_symbolic_ref_usage, options);
+ ret = check_symref(argv[0], 1, 0, 0);
+ if (ret)
+ die(Cannot delete %s, not a symbolic ref, argv[0]);
+ return delete_ref(argv[0], NULL, REF_NODEREF);
+ }
+
switch (argc) {
case 1:
- check_symref(argv[0], quiet);
+ ret = check_symref(argv[0], quiet, shorten, 1);
break;
case 2:
if (!strcmp(argv[0], HEAD)
@@ -59,5 +72,5 @@ int cmd_symbolic_ref(int argc, const char **argv, const char
*prefix)
default:
usage_with_options(git_symbolic_ref_usage, options);
}
- return 0;
+ return ret;
}
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
index 2c96551..36378b0 100755
--- a/t/t1401-symbolic-ref.sh
+++ b/t/t1401-symbolic-ref.sh
@@ -33,4 +33,34 @@ test_expect_success 'symbolic-ref refuses bare sha1' '
'
reset_to_sane
[PATCH 0/5] Fixing problem with deleting symrefs
I see that Rene Scharfe has also worked on the same issue, while I was preparing these patches... On Mon, Oct 15, 2012 at 11:29 AM, Junio C Hamano jch2...@gmail.com wrote: Even though update-ref deferences a symref when it updates one to point at a new object, I personally don't think update-ref -d that derefs makes any sense. I'd rather see it error out when given a symref, with or without --no-deref option. I'm not sure. We have multiple testcases that directly test deleting a ref through a symref (e.g. t1400), so supporting this seems like a concious decision. Erroring out when given a symref will break the following testcases: - t1400 (git update-ref -d) - t3310 t3311 (git notes merge --abort is broken) - t5505 (git remote set-head --delete and renaming a remote is broken) But even if it did, removing a ref pointed by a symref should really remove it, and forgetting to remove a leftover entry in packed-ref has no excuse bug. I'd say what you observed is a double bug. Patch #1 - #2 fixes the 2nd bug (removing through a symref should remove both loose and packed versions of the ref). Patch #3 fixes an associated problem where deleting a symref would remove the dereferenced ref's reflog instead of the symref's reflog. Patch #4 - #5 introduces solution A presented in my previous mail (i.e. 'git branch -d' never dereferences symrefs). Johan Herland (5): t1400-update-ref: Add test verifying bug with symrefs in delete_ref() delete_ref(): Fix deletion of refs through symbolic refs delete_ref(): Remove the correct reflog when deleting symrefs Add tests for using symbolic refs as branch name aliases. branch -d: Fix failure to remove branch aliases (symrefs) builtin/branch.c| 2 +- refs.c | 35 +++--- t/t1400-update-ref.sh | 18 +++ t/t3220-symbolic-ref-as-branch-alias.sh | 53 + 4 files changed, 90 insertions(+), 18 deletions(-) create mode 100755 t/t3220-symbolic-ref-as-branch-alias.sh -- 1.7.12.1.609.g5cd6968 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD' to delete refs/heads/master), we currently fail to remove the packed version of that ref. This testcase demonstrates the bug. Signed-off-by: Johan Herland jo...@herland.net --- t/t1400-update-ref.sh | 18 ++ 1 file changed, 18 insertions(+) diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 4fd83a6..f7ec203 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -74,6 +74,24 @@ test_expect_success delete $m (by HEAD) ' ' rm -f .git/$m +test_expect_success \ + create $m (by HEAD) \ + git update-ref HEAD $A +test $A' = $(cat .git/'$m')' +test_expect_success \ + pack refs \ + git pack-refs --all +test_expect_success \ + move $m (by HEAD) \ + git update-ref HEAD $B $A +test $B' = $(cat .git/'$m')' +test_expect_failure delete $m (by HEAD) should remove both packed and loose $m ' + git update-ref -d HEAD $B + ! grep $m .git/packed-refs + ! test -f .git/$m +' +rm -f .git/$m + cp -f .git/HEAD .git/HEAD.orig test_expect_success delete symref without dereference ' git update-ref --no-deref -d HEAD -- 1.7.12.1.609.g5cd6968 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] delete_ref(): Fix deletion of refs through symbolic refs
When deleting a ref through a symref (e.g. using 'git update-ref -d HEAD'
to delete refs/heads/master), the packed version of that ref would not
be deleted, because delete_ref() would pass the symref name (as opposed
to the dereferenced ref name) to repack_without_ref().
This patch revamps the logic in delete_ref() to make it easier to read,
and to clarify how it operates when given a symref.
Signed-off-by: Johan Herland jo...@herland.net
---
refs.c| 33 +
t/t1400-update-ref.sh | 2 +-
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/refs.c b/refs.c
index da74a2b..df4fe20 100644
--- a/refs.c
+++ b/refs.c
@@ -1751,34 +1751,35 @@ static int repack_without_ref(const char *refname)
int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
struct ref_lock *lock;
- int err, i = 0, ret = 0, flag = 0;
+ int err, delete_symref, ret = 0, flag = 0;
lock = lock_ref_sha1_basic(refname, sha1, 0, flag);
if (!lock)
return 1;
- if (!(flag REF_ISPACKED) || flag REF_ISSYMREF) {
- /* loose */
- const char *path;
- if (!(delopt REF_NODEREF)) {
- i = strlen(lock-lk-filename) - 5; /* .lock */
- lock-lk-filename[i] = 0;
- path = lock-lk-filename;
- } else {
- path = git_path(%s, refname);
- }
- err = unlink_or_warn(path);
+ /* The following variables are at play here:
+* - refname may be a symref (in this case lock-orig_ref_name holds
+*the symref name, and lock-ref_name holds the dereferenced name)
+* - The dereferenced ref name (lock-ref_name) may be a loose ref, a
+*packed ref, or both.
+* - If REF_NODEREF is enabled - and refname is a symref, we should
+*delete the symref; otherwise delete the dereferenced ref.
+*/
+ delete_symref = (flag REF_ISSYMREF delopt REF_NODEREF);
+ refname = delete_symref ? lock-orig_ref_name : lock-ref_name;
+
+ if (!(flag REF_ISPACKED) || delete_symref) {
+ /* loose */
+ err = unlink_or_warn(git_path(%s, refname));
if (err errno != ENOENT)
ret = 1;
-
- if (!(delopt REF_NODEREF))
- lock-lk-filename[i] = '.';
}
/* removing the loose one could have resurrected an earlier
* packed one. Also, if it was not loose we need to repack
* without it.
*/
- ret |= repack_without_ref(refname);
+ if (!delete_symref)
+ ret |= repack_without_ref(refname);
unlink_or_warn(git_path(logs/%s, lock-ref_name));
invalidate_ref_cache(NULL);
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index f7ec203..e415ee0 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -85,7 +85,7 @@ test_expect_success \
move $m (by HEAD) \
git update-ref HEAD $B $A
test $B' = $(cat .git/'$m')'
-test_expect_failure delete $m (by HEAD) should remove both packed and loose
$m '
+test_expect_success delete $m (by HEAD) should remove both packed and loose
$m '
git update-ref -d HEAD $B
! grep $m .git/packed-refs
! test -f .git/$m
--
1.7.12.1.609.g5cd6968
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] Add tests for using symbolic refs as branch name aliases.
A branch name alias is an alternative name for a branch, that is in most respects equivalent to using the proper branch name. It is implemented as a symbolic ref from the alias to the proper branch name. Currently branch aliases work well up to the point where you try to delete them (with git branch -d), at which point they incorrectly delete the proper branch name instead of the alias. This is reflected in these tests, and will be fixed in a subsequent patch. Signed-off-by: Johan Herland jo...@herland.net --- t/t3220-symbolic-ref-as-branch-alias.sh | 53 + 1 file changed, 53 insertions(+) create mode 100755 t/t3220-symbolic-ref-as-branch-alias.sh diff --git a/t/t3220-symbolic-ref-as-branch-alias.sh b/t/t3220-symbolic-ref-as-branch-alias.sh new file mode 100755 index 000..39f3a33 --- /dev/null +++ b/t/t3220-symbolic-ref-as-branch-alias.sh @@ -0,0 +1,53 @@ +#!/bin/sh + +test_description='Using a symbolic ref as a branch name alias + +This test uses refs/heads/alias as a symbolic ref to refs/heads/master, and +verifies that it works as a branch name alias, namely: + - commits on master are automatically reflected on alias + - creating or deleting alias does not affect master + - the alias is not broken by git gc +' +. ./test-lib.sh + +test_expect_success 'prepare a trivial repository' ' + echo Hello A + git add A + git commit -m Hello + git rev-parse --verify HEAD expect +' + +test_expect_success 'create symref: refs/heads/alias - refs/heads/master' ' + git symbolic-ref refs/heads/alias refs/heads/master + git rev-parse --verify master master + git rev-parse --verify alias alias + test_cmp expect master + test_cmp expect alias +' + +test_expect_success 'git gc does not change alias symref' ' + git gc + git rev-parse --verify master master + git rev-parse --verify alias alias + test_cmp expect master + test_cmp expect alias +' + +test_expect_success 'commits are reflected through alias symref' ' + echo World A + git commit -am A + git rev-parse --verify HEAD expect + git rev-parse --verify master master + git rev-parse --verify alias alias + test_cmp expect master + test_cmp expect alias +' + +test_expect_failure 'branch -d deletes the alias symref' ' + git branch -d alias + git rev-parse --verify master master + test_must_fail git rev-parse --verify alias + test_cmp expect master +' + +test_done -- 1.7.12.1.609.g5cd6968 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] branch -d: Fix failure to remove branch aliases (symrefs)
With refs/heads/alias set up as a symref to refs/heads/master,
'git branch -d alias' should remove refs/heads/alias and not
refs/heads/master.
Signed-off-by: Johan Herland jo...@herland.net
---
builtin/branch.c| 2 +-
t/t3220-symbolic-ref-as-branch-alias.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index ffd2684..31af114 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -221,7 +221,7 @@ static int delete_branches(int argc, const char **argv, int
force, int kinds,
continue;
}
- if (delete_ref(name, sha1, 0)) {
+ if (delete_ref(name, sha1, REF_NODEREF)) {
error(remote_branch
? _(Error deleting remote branch '%s')
: _(Error deleting branch '%s'),
diff --git a/t/t3220-symbolic-ref-as-branch-alias.sh
b/t/t3220-symbolic-ref-as-branch-alias.sh
index 39f3a33..8ebec7a 100755
--- a/t/t3220-symbolic-ref-as-branch-alias.sh
+++ b/t/t3220-symbolic-ref-as-branch-alias.sh
@@ -43,7 +43,7 @@ test_expect_success 'commits are reflected through alias
symref' '
test_cmp expect alias
'
-test_expect_failure 'branch -d deletes the alias symref' '
+test_expect_success 'branch -d deletes the alias symref' '
git branch -d alias
git rev-parse --verify master master
test_must_fail git rev-parse --verify alias
--
1.7.12.1.609.g5cd6968
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] delete_ref(): Remove the correct reflog when deleting symrefs
When deleting a symref (e.g. HEAD), we would incorrectly remove the reflog of the dereferenced ref (e.g. .git/logs/refs/heads/master), insted of the symref's reflog (e.g. .git/logs/HEAD). This patch ensures that we remove the reflog that corresponds to the removed (sym)ref. Signed-off-by: Johan Herland jo...@herland.net --- refs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/refs.c b/refs.c index df4fe20..f2508bf 100644 --- a/refs.c +++ b/refs.c @@ -1781,7 +1781,7 @@ int delete_ref(const char *refname, const unsigned char *sha1, int delopt) if (!delete_symref) ret |= repack_without_ref(refname); - unlink_or_warn(git_path(logs/%s, lock-ref_name)); + unlink_or_warn(git_path(logs/%s, refname)); invalidate_ref_cache(NULL); unlock_ref(lock); return ret; -- 1.7.12.1.609.g5cd6968 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
BUG when trying to delete symbolic refs
Hi, At $dayjob we renamed a branch, and for a grace period, we kept the old name as a symref/alias to the new name, to give our users a window for switching. This has worked well, until we tried to remove the symref/alias. The following script demonstrates what we discovered: $ git --version git version 1.8.0.rc2.249.g6cc8227 $ git init symref_delete_test/ Initialized empty Git repository in .../symref_delete_test/.git/ $ cd symref_delete_test/ $ echo foo foo git add foo git commit -m foo [master (root-commit) c7ae77e] foo 1 file changed, 1 insertion(+) create mode 100644 foo $ git gc Counting objects: 3, done. Writing objects: 100% (3/3), done. Total 3 (delta 0), reused 0 (delta 0) $ cat .git/packed-refs # pack-refs with: peeled c7ae77e537138bee3f722e57e1af87a7011466cb refs/heads/master $ echo bar foo git commit -am bar [master 7451bf0] bar 1 file changed, 1 insertion(+), 1 deletion(-) $ git symbolic-ref refs/heads/alias refs/heads/master $ git rev-parse master 7451bf08b7aacedc9e88a9fa37a6c1f701071bbe $ git rev-parse alias 7451bf08b7aacedc9e88a9fa37a6c1f701071bbe $ git branch -d alias Deleted branch alias (was 7451bf0). $ git rev-parse master c7ae77e537138bee3f722e57e1af87a7011466cb $ git rev-parse alias c7ae77e537138bee3f722e57e1af87a7011466cb $ cat .git/packed-refs # pack-refs with: peeled c7ae77e537138bee3f722e57e1af87a7011466cb refs/heads/master $ ls .git/refs/heads/ alias Basically, there is a master branch, and an alias symref to master. When we naively try to delete the symref with git branch -d alias, it ends up: - NOT deleting the alias symref - DELETING the master loose ref - NOT deleting the master packed ref So, from the user perspective, git branch -d alias ends up resetting master (and alias) back to the last time we happened to run git gc. Needless to say, this is not quite what we had in mind... AFAICS, there may be three possible acceptable outcomes when we run git branch -d alias in the above scenario: A. The symbolic ref is deleted. This is obviously what we expected... B. The command fails because alias is a symref. This would be understandable if we don't want to teach branch -d about symrefs. But then, the error message should ideally explain which command we should use to remove the symref. C. The master ref (BOTH loose and packed versions of it) is deleted. This would be less helpful for us, but Git would at least be internally consistent (in that the symref would be resolved, and the command would become git branch -d master). Obviously, I would advocate for option A. What say you? Have fun! :) ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[BUG?] Path limiting in git log when run from another subdir
The following works in the git.git repo: $ cd t $ git log -- /Documentation [...] but the following does not: $ cd t $ git log -- /Documentation/RelNotes fatal: Could not switch to '/Documentation': No such file or directory Is this the intended behavior? ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] Path limiting in git log when run from another subdir
On Mon, Sep 24, 2012 at 3:30 PM, Nguyen Thai Ngoc Duy pclo...@gmail.com wrote: In any case, what is the _preferred_ way to path-limit git log to Documentation/RelNotes, when my cwd is t/? If you want worktree root no matter where you stand, use git log -- :/Documentation/RelNotes. The idea is : starts some magic in path handling, but for now there is only :/. Or if you don't like magic, `git rev-parse --git-dir` should give you worktree's root to start with. Ah, ok, that's exactly what I wanted. Then I guess cd t git log -- /Documentation should have never worked in the first place, and I question about git log -- /Documentation/RelNotes was misguided. Thanks for clearing up my confusion. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] notes get-ref: --expand expands the output notes ref.
On Thu, Sep 6, 2012 at 9:47 PM, Junio C Hamano gits...@pobox.com wrote: W. Trevor King wk...@tremily.us writes: On Wed, Sep 05, 2012 at 10:23:54PM -0700, Junio C Hamano wrote: Really? Would git log --expand master be useful? I'm clearly not an expert on this, but isn't that what git show-ref master is for? But then can't you say the same against git notes get-ref --expand with git show-ref refs/remotes/origin/notes/foobla? My primary point is that I think it is a UI mistake if the DWIM ignores the user input that directly names that can be interpreted without DWIMming. When the user wants to avoid ambiguity, it should always be possible to spell it out without having to worry about the DWIM code to get in the way. The problem with the current notes.c:expand_notes_ref() is that it does not allow you to do that, and if you fixed that problem, the user never has to ask what does this expand to, no? Perhaps something like this. Note that this only deals with an existing ref, and that is semi-deliberate (because I am not yet convinced that it is a good idea to let any ref outside refs/notes/ to be created to hold a notes tree). (sorry for the late reply; I was away this weekend) This patch looks like an acceptable solution to me. Especially since it prevents the notes code from accidentally creating new notes trees outside refs/notes/*. That said, we should check for more cases of hardcoded refs/notes/ that potentially needs changing as well. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Add --expand to 'git notes get-ref'
On Wed, Sep 5, 2012 at 2:48 PM, W. Trevor King wk...@tremily.us wrote: The second commit makes the expansion less strict about the location of note refs. In his initial mail introducing 'git notes', Johan says that note refs should live under 'refs/notes' [1]. This seems like a good place for local notes, but note refs from remote repos should probably live somewhere else (e.g. 'refs/remote-notes/' or 'refs/remotes/name/notes/'. In the initial thread there are a few messages talking about looking up reverse mappings under 'refs/notes/', but this seems to all have been before the 'refs/notes/namespace/' stage. If I'm missing a good reason to keep everything under 'refs/notes/', feel free to ignore the second patch. This has been discussed a couple of times on this list, but it never resulted in any actual changes. Read up on this thread to get some context: http://thread.gmane.org/gmane.comp.version-control.git/160503 ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] notes get-ref: --expand expands the output notes ref.
On Wed, Sep 5, 2012 at 2:52 PM, W. Trevor King wk...@tremily.us wrote: From: W. Trevor King wk...@tremily.us Useful for debugging refs that don't seem to be expanding correctly. Signed-off-by: W. Trevor King wk...@tremily.us Acked-by: Johan Herland jo...@herland.net -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Add --expand to 'git notes get-ref'
On Wed, Sep 5, 2012 at 7:53 PM, W. Trevor King wk...@tremily.us wrote: On Wed, Sep 05, 2012 at 05:58:37PM +0200, Johan Herland wrote: On Wed, Sep 5, 2012 at 2:48 PM, W. Trevor King wk...@tremily.us wrote: If I'm missing a good reason to keep everything under 'refs/notes/', feel free to ignore the second patch. This has been discussed a couple of times on this list, but it never resulted in any actual changes. Read up on this thread to get some context: http://thread.gmane.org/gmane.comp.version-control.git/160503 Thanks for the pointer, it looks like there are a bunch of good ideas. I assume the lack of changes was due to nobody having the time/inclination to implement http://article.gmane.org/gmane.comp.version-control.git/160655 Yes, I think time/inclination probably has a lot to do with it. Also, I forgot to link to the largest mailing list thread that discusses these changes in _much_ more detail: http://thread.gmane.org/gmane.comp.version-control.git/165799/focus=165885 As you can see, the discussion quickly balloons into something much larger than a simple discussion of where remote notes should be located... That said, your patch 2/2 might be an acceptable stopgap measure for the time being, and then we can reevaluate it if/when we implement a larger ref restructuring. ...Johan -- Johan Herland, jo...@herland.net www.herland.net -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
