* Re: [PATCH 2/2] Fix failure to delete a packed ref through a symref
From: René Scharfe @ 2012-10-21 17:46 UTC (permalink / raw)
To: Johan Herland; +Cc: gitster, git, vmiklos
In-Reply-To: <1350816032-16312-3-git-send-email-johan@herland.net>
Am 21.10.2012 12:40, schrieb Johan Herland:
> 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 <johan@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);
Looks reasonable.
FWIW, this is independent of 547d058f in next (refs: lock symref that is
to be deleted, not its target), which only affects behaviour when
REF_NODEREF is set, while this one here only makes a difference with
symrefs and REF_NODEREF unset.
René
^ permalink raw reply
* Re: Subtree in Git
From: Herman van Rink @ 2012-10-21 15:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: dag, greened, Hilco Wijbenga, Git Users
In-Reply-To: <7vbofwgwso.fsf@alter.siamese.dyndns.org>
On 10/21/2012 08:32 AM, Junio C Hamano wrote:
> Herman van Rink <rink@initfour.nl> writes:
>
>> Junio, Could you please consider merging the single commit from my
>> subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates
> In general, in areas like contrib/ where there is a volunteer area
> maintainer, unless the change something ultra-urgent (e.g. serious
> security fix) and the area maintainer is unavailable, I'm really
> reluctant to bypass and take a single patch that adds many things
> that are independent from each other.
Who do you see as volunteer area maintainer for contrib/subtree?
My best guess would be Dave. And he already indicated earlier in the
thread to be ok with the combined patch as long as you are ok with it.
>
> Especially not immediately before tagging 1.8.0 final.
Sure, we've waited this long... I don't mind waiting one more release cycle.
--
Met vriendelijke groet / Regards,
Herman van Rink
Initfour websolutions
^ permalink raw reply
* Re: In search of a version control system
From: Sitaram Chamarty @ 2012-10-21 13:25 UTC (permalink / raw)
To: Drew Northup
Cc: Matthieu Moy, David Aguilar, Ramkumar Ramachandra,
Maurice Francois, git@vger.kernel.org
In-Reply-To: <CAM9Z-nnQjhehJEpyhyTY=rsK_f=2gTOUMVTycn7-Lao8Gog0pA@mail.gmail.com>
On Sun, Oct 21, 2012 at 5:50 PM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Tue, Oct 9, 2012 at 1:58 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> David Aguilar <davvid@gmail.com> writes:
>>
>>> I would advise against the file locking, though. You ain't gonna need
>>> it ;-)
>>
>> What do you suggest to merge Word files?
>
> If the files are in the DOCX format you can just expand them as zip
> archives and diff what's inside of them. The text in particular is
> stored as XML.
You also need a merge driver that at least splits the "all in one
single very very long line" XML into different lines in some way. I
don't think git can merge even text files if everything is on one line
in each file. And even if you do this I don't think the result will
be a valid ODT etc file.
All in all, I prefer the locking that David mentioned [1]. And if
your users cannot be trained to check first (as that URL describes),
then you probably have to use a CVCS that supports some stronger form
of locking.
[1]: http://sitaramc.github.com/gitolite/locking.html
^ permalink raw reply
* Re: In search of a version control system
From: Nguyen Thai Ngoc Duy @ 2012-10-21 13:16 UTC (permalink / raw)
To: Drew Northup
Cc: Matthieu Moy, David Aguilar, Ramkumar Ramachandra,
Maurice Francois, git@vger.kernel.org
In-Reply-To: <CAM9Z-nnQjhehJEpyhyTY=rsK_f=2gTOUMVTycn7-Lao8Gog0pA@mail.gmail.com>
On Sun, Oct 21, 2012 at 7:20 PM, Drew Northup <n1xim.email@gmail.com> wrote:
> On Tue, Oct 9, 2012 at 1:58 AM, Matthieu Moy
> <Matthieu.Moy@grenoble-inp.fr> wrote:
>> David Aguilar <davvid@gmail.com> writes:
>>
>>> I would advise against the file locking, though. You ain't gonna need
>>> it ;-)
>>
>> What do you suggest to merge Word files?
>
> If the files are in the DOCX format you can just expand them as zip
> archives and diff what's inside of them. The text in particular is
> stored as XML.
An XML diff means nothing to a regular user, and manually "merging"
this way can produce unreadable Words files. So no, it's still not an
answer.
--
Duy
^ permalink raw reply
* Re: In search of a version control system
From: Drew Northup @ 2012-10-21 12:20 UTC (permalink / raw)
To: Matthieu Moy
Cc: David Aguilar, Ramkumar Ramachandra, Maurice Francois,
git@vger.kernel.org
In-Reply-To: <vpq4nm4rxsy.fsf@grenoble-inp.fr>
On Tue, Oct 9, 2012 at 1:58 AM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> David Aguilar <davvid@gmail.com> writes:
>
>> I would advise against the file locking, though. You ain't gonna need
>> it ;-)
>
> What do you suggest to merge Word files?
If the files are in the DOCX format you can just expand them as zip
archives and diff what's inside of them. The text in particular is
stored as XML.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: git fetch documentation problem or bug
From: Drew Northup @ 2012-10-21 12:15 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Angelo Borsotti, Carlos Martín Nieto, Matthieu Moy,
Nguyễn Thái Ngọc
In-Reply-To: <7vpq4silou.fsf@alter.siamese.dyndns.org>
On Mon, Oct 8, 2012 at 7:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> In other words, you can do this from the command line if you want
>> to do the update.
>>
>> $ git fetch origin master:refs/remotes/origin/master
>
> Now having said all that, we should probably revisit this and
> possibly other issues and for the ones we can reach concensus, start
> coding after 1.8.0 final.
> * "git fetch $from $branch..." to update tracking branches
>
> This is the topic in this thread.
>
> I personally do not think the downside of breaking backward
> compatibility is too bad. If we do this only when we already are
> configured to keep remote tracking branch for branch $branch from
> remote $from (it has to be given as a nickname, not URL that happens
> to have an entry in the configuration), then a promiscuous fetch
> that grabs from a URL (or a nickname that is configured not to keep
> tracking branches) will not change any behaviour, and when you want
> to keep your remote tracking branch intact while doing one-shot
> fetch for whatever reason, you can say "git fetch $from $branch:" to
> explicitly decline copying.
How are we supposed to remember those are different?
"git fetch $from $branch..."
VS
"git fetch $from $branch:"
I strongly prefer EXPLICITLY setting tracking than expecting some
extreme syntactic nuance to quietly do it for me now and confuse the
heck out of me later.
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: looking for suggestions for managing a tree of server configs
From: Drew Northup @ 2012-10-21 11:58 UTC (permalink / raw)
To: david; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.DEB.2.02.1210201931130.31862@asgard.lang.hm>
On Sat, Oct 20, 2012 at 10:34 PM, <david@lang.hm> wrote:
> On Sat, 20 Oct 2012, Drew Northup wrote:
>> On Sun, Oct 14, 2012 at 12:57 AM, <david@lang.hm> wrote:
>>> On Sat, 13 Oct 2012, Junio C Hamano wrote:
>>>> david@lang.hm writes:
>>>>>
>>>>> today I have just a single git tree covering everything, and I make a
>>>>> commit each time one of the per-server directories is updated, and
>>>>> again when the top-level stuff is created.
>>>>
>>>> if a large portion of the configuration for these servers are
>>>> shared, it might not be a bad idea to have a canonical "gold-master"
>>>> configuration branch, to which the shared updates are applied, with
>>>> a branch per server that forks from that canonical branch to keep
>>>> the machine specific tweaks
>>>
>>> In an ideal world yes, but right now these machines are updated by many
>>> different tools (unforuntantly including 'vi'), so
>>> these directories aren't the config to be pushed out to the boxes, it's
>>> instead an archived 'what is', the result of changes from all the tools.
So you need to save what is there before pulling changes from the
master. That's no different from doing development work on an active
code base.
>> David,
>> Is there any particular reason you aren't using etckeeper?
>>
> not really, I've thought of that as a tool for managing a single system.
> Some of the data in configs is sensitive (and much of it is not in /etc),
> but I guess I should be able to work around those issues.
.gitignore and symlinks have been employed at times to deal with that.
> How can I sanely organize all these different, but similar sets of files on
> the central server?
The reason I asked about etckeeper is that you could, with proper
security in place, push those up to branches in a shared repository
(set up using gitolite, for instance) and not loose information about
the files in the process. This would allow you to make your changes on
one system using vi or whatever else is convenient, push the change up
to the shared repo, cherry-pick it into the other branches (using a
full check-out of all of the branches someplace safe as a workspace),
and pull that change out to the other systems.
If you are just looking to gather configuration information in the
large and don't want to engage in any shared management schemes (which
may involve symlinks in seemingly odd places to /etc and such) you may
wish to look at the System Configuration Collector [1] [2] which is a
nicely organized tool designed specifically to gather just the
important (and not highly confidential) information about common
software on a server and present it (and changes to it) to the admin
in a sensible manner. It is outside of the "Git Universe" but it does
what it sounds like you are doing now (if not what you wish to be
doing).
(1) http://www.qnh.eu/scc/
(2) http://sourceforge.net/projects/sysconfcollect/
--
-Drew Northup
--------------------------------------------------------------
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* [RFC/PATCH] git symbolic-ref --delete $symref
From: Johan Herland @ 2012-10-21 11:32 UTC (permalink / raw)
To: gitster; +Cc: git, Johan Herland
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 <johan@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
+test_expect_success 'symbolic-ref deletes HEAD' '
+ git symbolic-ref -d HEAD &&
+ test_path_is_file .git/refs/heads/foo &&
+ test_path_is_missing .git/HEAD
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref deletes dangling HEAD' '
+ git symbolic-ref HEAD refs/heads/missing &&
+ git symbolic-ref -d HEAD &&
+ test_path_is_missing .git/refs/heads/missing &&
+ test_path_is_missing .git/HEAD
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref fails to delete missing FOO' '
+ echo "fatal: Cannot delete FOO, not a symbolic ref" >expect &&
+ test_must_fail git symbolic-ref -d FOO >actual 2>&1 &&
+ test_cmp expect actual
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref fails to delete real ref' '
+ echo "fatal: Cannot delete refs/heads/foo, not a symbolic ref" >expect &&
+ test_must_fail git symbolic-ref -d refs/heads/foo >actual 2>&1 &&
+ test_path_is_file .git/refs/heads/foo &&
+ test_cmp expect actual
+'
+reset_to_sane
+
test_done
--
1.7.12.1.609.g5cd6968
^ permalink raw reply related
* [PATCH 1/2] t1400-update-ref: Add test verifying bug with symrefs in delete_ref()
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <1350816032-16312-1-git-send-email-johan@herland.net>
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 <johan@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
^ permalink raw reply related
* [PATCH 2/2] Fix failure to delete a packed ref through a symref
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <1350816032-16312-1-git-send-email-johan@herland.net>
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 <johan@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
^ permalink raw reply related
* [PATCH 0/2] Fix remaining issue when deleting symrefs
From: Johan Herland @ 2012-10-21 10:40 UTC (permalink / raw)
To: gitster; +Cc: git, rene.scharfe, vmiklos, Johan Herland
In-Reply-To: <7vpq4flb1c.fsf@alter.siamese.dyndns.org>
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
^ permalink raw reply
* Links broken in ref docs.
From: Mike Norman @ 2012-10-21 7:31 UTC (permalink / raw)
To: git
Many links on scm-git.org/docs simply reload the page.
For example, all of Sharing and Updating section simply reload the
docs page. And tons others. Must be a broken link or routing problem.
Repros on FF 14.0.1 and Chrome. Good luck!
IRC was useless.
Also, fuck your stupid email policies. :)
^ permalink raw reply
* Re: git config --unset does not remove section
From: Junio C Hamano @ 2012-10-21 7:09 UTC (permalink / raw)
To: Angelo Borsotti; +Cc: git
In-Reply-To: <CAB9Jk9C_b_PScm3vEdvxsjqNHpTbz7OTTdFeFey7Sdb+M5gYFw@mail.gmail.com>
Angelo Borsotti <angelo.borsotti@gmail.com> writes:
> Another git config foo.bar true changes the config file into:
>
> [foo]
> [foo]
> bar = true
>
> Having two sections with the same name clutters the config file.
True. It would be nice if the removal of the last element removes
the section header as well.
^ permalink raw reply
* Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Junio C Hamano @ 2012-10-21 6:51 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> This patch series has the side effect that all of the directories
> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
> resolve any symlinks that are present in their paths. It is
> admittedly odd that a feature intended to avoid accessing expensive
> directories would now *intentionally* access directories near the
> expensive ones. In the above scenario this shouldn't be a problem,
> because /home would be the directory listed in
> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
> expensive.
Interesting observation. In the last sentence, "accessing /home"
does not exactly mean accessing /home, but accessing / to learn
about "home" in it, no?
> But there might be other scenarios for which this patch
> series causes a performance regression.
Yeah, after merging this to 'next', we should ask people who care
about CEILING to test it sufficiently.
Thanks for rerolling.
^ permalink raw reply
* git config --unset does not remove section
From: Angelo Borsotti @ 2012-10-21 6:46 UTC (permalink / raw)
To: git
Hello,
git config --unset does not remove the section in the configuration
file when the removed key is the last one, leaving then an empty
section that is lo longer used. E.g.:
git config foo.bar true
creates:
[foo]
bar = true
then:
git config --unset foo.bar
changes the section into:
[foo]
Another git config foo.bar true changes the config file into:
[foo]
[foo]
bar = true
Having two sections with the same name clutters the config file.
-Angelo Borsotti
^ permalink raw reply
* Re: Subtree in Git
From: Junio C Hamano @ 2012-10-21 6:32 UTC (permalink / raw)
To: Herman van Rink; +Cc: dag, greened, Hilco Wijbenga, Git Users
In-Reply-To: <50830374.9090308@initfour.nl>
Herman van Rink <rink@initfour.nl> writes:
> Junio, Could you please consider merging the single commit from my
> subtree-updates branch? https://github.com/helmo/git/tree/subtree-updates
In general, in areas like contrib/ where there is a volunteer area
maintainer, unless the change something ultra-urgent (e.g. serious
security fix) and the area maintainer is unavailable, I'm really
reluctant to bypass and take a single patch that adds many things
that are independent from each other.
Especially not immediately before tagging 1.8.0 final.
^ permalink raw reply
* [PATCH v3 7/8] normalize_ceiling_entry(): resolve symlinks
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
longest_ancestor_length() relies on a textual comparison of directory
parts to find the part of path that overlaps with one of the paths in
prefix_list. But this doesn't work if any of the prefixes involves a
symbolic link, because the directories will look different even though
they might logically refer to the same directory. So canonicalize the
paths listed in GIT_CEILING_DIRECTORIES using real_path_if_valid()
before passing them to longest_ancestor_length().
path is already in canonical form, so doesn't need to be canonicalized
again.
This fixes some problems with using GIT_CEILING_DIRECTORIES that
contains paths involving symlinks, including t4035 if run with --root
set to a path involving symlinks.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
setup.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/setup.c b/setup.c
index df97ad3..2eb5b75 100644
--- a/setup.c
+++ b/setup.c
@@ -622,24 +622,22 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
}
/*
- * A "string_list_each_func_t" function that normalizes an entry from
- * GIT_CEILING_DIRECTORIES or discards it if unusable.
+ * A "string_list_each_func_t" function that canonicalizes an entry
+ * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
+ * discards it if unusable.
*/
static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
{
- const char *ceil = item->string;
- int len = strlen(ceil);
- char buf[PATH_MAX+1];
+ char *ceil = item->string;
+ const char *real_path;
- if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+ if (!*ceil || !is_absolute_path(ceil))
return 0;
- if (normalize_path_copy(buf, ceil) < 0)
+ real_path = real_path_if_valid(ceil);
+ if (!real_path)
return 0;
- len = strlen(buf);
- if (len > 1 && buf[len-1] == '/')
- buf[--len] = '\0';
free(item->string);
- item->string = xstrdup(buf);
+ item->string = xstrdup(real_path);
return 1;
}
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 8/8] string_list_longest_prefix(): remove function
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
This function was added in f103f95b11d087f07c0c48bf784cd9197e18f203 in
the erroneous expectation that it would be used in the
reimplementation of longest_ancestor_length(). But it turned out to
be easier to use a function specialized for comparing path prefixes
(i.e., one that knows about slashes and root paths) than to prepare
the paths in such a way that a generic string prefix comparison
function can be used. So delete string_list_longest_prefix() and its
documentation and test cases.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
Documentation/technical/api-string-list.txt | 8 --------
string-list.c | 20 -------------------
string-list.h | 8 --------
t/t0063-string-list.sh | 30 -----------------------------
test-string-list.c | 20 -------------------
5 files changed, 86 deletions(-)
diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt
index 94d7a2b..618400d 100644
--- a/Documentation/technical/api-string-list.txt
+++ b/Documentation/technical/api-string-list.txt
@@ -75,14 +75,6 @@ Functions
to be deleted. Preserve the order of the items that are
retained.
-`string_list_longest_prefix`::
-
- Return the longest string within a string_list that is a
- prefix (in the sense of prefixcmp()) of the specified string,
- or NULL if no such prefix exists. This function does not
- require the string_list to be sorted (it does a linear
- search).
-
`print_string_list`::
Dump a string_list to stdout, useful mainly for debugging purposes. It
diff --git a/string-list.c b/string-list.c
index c54b816..decfa74 100644
--- a/string-list.c
+++ b/string-list.c
@@ -136,26 +136,6 @@ void filter_string_list(struct string_list *list, int free_util,
list->nr = dst;
}
-char *string_list_longest_prefix(const struct string_list *prefixes,
- const char *string)
-{
- int i, max_len = -1;
- char *retval = NULL;
-
- for (i = 0; i < prefixes->nr; i++) {
- char *prefix = prefixes->items[i].string;
- if (!prefixcmp(string, prefix)) {
- int len = strlen(prefix);
- if (len > max_len) {
- retval = prefix;
- max_len = len;
- }
- }
- }
-
- return retval;
-}
-
void string_list_clear(struct string_list *list, int free_util)
{
if (list->items) {
diff --git a/string-list.h b/string-list.h
index 5efd07b..3a6a6dc 100644
--- a/string-list.h
+++ b/string-list.h
@@ -38,14 +38,6 @@ int for_each_string_list(struct string_list *list,
void filter_string_list(struct string_list *list, int free_util,
string_list_each_func_t want, void *cb_data);
-/*
- * Return the longest string in prefixes that is a prefix (in the
- * sense of prefixcmp()) of string, or NULL if no such prefix exists.
- * This function does not require the string_list to be sorted (it
- * does a linear search).
- */
-char *string_list_longest_prefix(const struct string_list *prefixes, const char *string);
-
/* Use these functions only on sorted lists: */
int string_list_has_string(const struct string_list *list, const char *string);
diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh
index 41c8826..dbfc05e 100755
--- a/t/t0063-string-list.sh
+++ b/t/t0063-string-list.sh
@@ -17,14 +17,6 @@ test_split () {
"
}
-test_longest_prefix () {
- test "$(test-string-list longest_prefix "$1" "$2")" = "$3"
-}
-
-test_no_longest_prefix () {
- test_must_fail test-string-list longest_prefix "$1" "$2"
-}
-
test_split "foo:bar:baz" ":" "-1" <<EOF
3
[0]: "foo"
@@ -96,26 +88,4 @@ test_expect_success "test remove_duplicates" '
test a:b:c = "$(test-string-list remove_duplicates a:a:a:b:b:b:c:c:c)"
'
-test_expect_success "test longest_prefix" '
- test_no_longest_prefix - '' &&
- test_no_longest_prefix - x &&
- test_longest_prefix "" x "" &&
- test_longest_prefix x x x &&
- test_longest_prefix "" foo "" &&
- test_longest_prefix : foo "" &&
- test_longest_prefix f foo f &&
- test_longest_prefix foo foobar foo &&
- test_longest_prefix foo foo foo &&
- test_no_longest_prefix bar foo &&
- test_no_longest_prefix bar:bar foo &&
- test_no_longest_prefix foobar foo &&
- test_longest_prefix foo:bar foo foo &&
- test_longest_prefix foo:bar bar bar &&
- test_longest_prefix foo::bar foo foo &&
- test_longest_prefix foo:foobar foo foo &&
- test_longest_prefix foobar:foo foo foo &&
- test_longest_prefix foo: bar "" &&
- test_longest_prefix :foo bar ""
-'
-
test_done
diff --git a/test-string-list.c b/test-string-list.c
index 4693295..00ce6c9 100644
--- a/test-string-list.c
+++ b/test-string-list.c
@@ -97,26 +97,6 @@ int main(int argc, char **argv)
return 0;
}
- if (argc == 4 && !strcmp(argv[1], "longest_prefix")) {
- /* arguments: <colon-separated-prefixes>|- <string> */
- struct string_list prefixes = STRING_LIST_INIT_DUP;
- int retval;
- const char *prefix_string = argv[2];
- const char *string = argv[3];
- const char *match;
-
- parse_string_list(&prefixes, prefix_string);
- match = string_list_longest_prefix(&prefixes, string);
- if (match) {
- printf("%s\n", match);
- retval = 0;
- }
- else
- retval = 1;
- string_list_clear(&prefixes, 0);
- return retval;
- }
-
fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
argv[1] ? argv[1] : "(there was none)");
return 1;
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 5/8] longest_ancestor_length(): take a string_list argument for prefixes
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
Change longest_ancestor_length() to take the prefixes argument as a
string_list rather than as a PATH_SEP-separated string. This will
make it easier to change the caller to alter the entries before
calling longest_ancestor_length().
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
cache.h | 2 +-
path.c | 22 +++++++++-------------
setup.c | 11 +++++++++--
test-path-utils.c | 8 +++++++-
4 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/cache.h b/cache.h
index b0d75bc..8103385 100644
--- a/cache.h
+++ b/cache.h
@@ -718,7 +718,7 @@ const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
const char *relative_path(const char *abs, const char *base);
int normalize_path_copy(char *dst, const char *src);
-int longest_ancestor_length(const char *path, const char *prefix_list);
+int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
int daemon_avoid_alias(const char *path);
int offset_1st_component(const char *path);
diff --git a/path.c b/path.c
index f455e8e..b80d2e6 100644
--- a/path.c
+++ b/path.c
@@ -570,30 +570,27 @@ int normalize_path_copy(char *dst, const char *src)
/*
* path = Canonical absolute path
- * prefix_list = Colon-separated list of absolute paths
+ * prefixes = string_list containing absolute paths
*
- * Determines, for each path in prefix_list, whether the "prefix" really
+ * Determines, for each path in prefixes, whether the "prefix" really
* is an ancestor directory of path. Returns the length of the longest
* ancestor directory, excluding any trailing slashes, or -1 if no prefix
- * is an ancestor. (Note that this means 0 is returned if prefix_list is
- * "/".) "/foo" is not considered an ancestor of "/foobar". Directories
+ * is an ancestor. (Note that this means 0 is returned if prefixes is
+ * ["/"].) "/foo" is not considered an ancestor of "/foobar". Directories
* are not considered to be their own ancestors. path must be in a
* canonical form: empty components, or "." or ".." components are not
- * allowed. prefix_list may be null, which is like "".
+ * allowed. Empty strings in prefixes are ignored.
*/
-int longest_ancestor_length(const char *path, const char *prefix_list)
+int longest_ancestor_length(const char *path, struct string_list *prefixes)
{
- struct string_list prefixes = STRING_LIST_INIT_DUP;
char buf[PATH_MAX+1];
int i, max_len = -1;
- if (prefix_list == NULL || !strcmp(path, "/"))
+ if (!strcmp(path, "/"))
return -1;
- string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
-
- for (i = 0; i < prefixes.nr; i++) {
- const char *ceil = prefixes.items[i].string;
+ for (i = 0; i < prefixes->nr; i++) {
+ const char *ceil = prefixes->items[i].string;
int len = strlen(ceil);
if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
@@ -611,7 +608,6 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
}
}
- string_list_clear(&prefixes, 0);
return max_len;
}
diff --git a/setup.c b/setup.c
index 3a1b2fd..b4cd356 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "dir.h"
+#include "string-list.h"
static int inside_git_dir = -1;
static int inside_work_tree = -1;
@@ -627,10 +628,11 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
static const char *setup_git_directory_gently_1(int *nongit_ok)
{
const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
+ struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
static char cwd[PATH_MAX+1];
const char *gitdirenv, *ret;
char *gitfile;
- int len, offset, offset_parent, ceil_offset;
+ int len, offset, offset_parent, ceil_offset = -1;
dev_t current_device = 0;
int one_filesystem = 1;
@@ -655,7 +657,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
if (gitdirenv)
return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
- ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
+ if (env_ceiling_dirs) {
+ string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
+ ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
+ string_list_clear(&ceiling_dirs, 0);
+ }
+
if (ceil_offset < 0 && has_dos_drive_prefix(cwd))
ceil_offset = 1;
diff --git a/test-path-utils.c b/test-path-utils.c
index 3bc20e9..acb0560 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "string-list.h"
int main(int argc, char **argv)
{
@@ -30,7 +31,12 @@ int main(int argc, char **argv)
}
if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
- int len = longest_ancestor_length(argv[2], argv[3]);
+ int len;
+ struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+
+ string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
+ len = longest_ancestor_length(argv[2], &ceiling_dirs);
+ string_list_clear(&ceiling_dirs, 0);
printf("%d\n", len);
return 0;
}
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 6/8] longest_ancestor_length(): require prefix list entries to be normalized
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
Move the responsibility for normalizing the prefixes passed to
longest_ancestor_length() to its caller. In t0060, only test
longest_ancestor_lengths using normalized paths: remove empty entries
and non-absolute paths, strip trailing slashes from the paths, and
remove tests that thereby become redundant.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
path.c | 26 +++++++++++---------------
setup.c | 23 +++++++++++++++++++++++
t/t0060-path-utils.sh | 41 +++++++++++++----------------------------
3 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/path.c b/path.c
index b80d2e6..d3d3f8b 100644
--- a/path.c
+++ b/path.c
@@ -570,20 +570,20 @@ int normalize_path_copy(char *dst, const char *src)
/*
* path = Canonical absolute path
- * prefixes = string_list containing absolute paths
+ * prefixes = string_list containing normalized, absolute paths without
+ * trailing slashes (except for the root directory, which is denoted by "/").
*
- * Determines, for each path in prefixes, whether the "prefix" really
+ * Determines, for each path in prefixes, whether the "prefix"
* is an ancestor directory of path. Returns the length of the longest
* ancestor directory, excluding any trailing slashes, or -1 if no prefix
* is an ancestor. (Note that this means 0 is returned if prefixes is
* ["/"].) "/foo" is not considered an ancestor of "/foobar". Directories
* are not considered to be their own ancestors. path must be in a
* canonical form: empty components, or "." or ".." components are not
- * allowed. Empty strings in prefixes are ignored.
+ * allowed.
*/
int longest_ancestor_length(const char *path, struct string_list *prefixes)
{
- char buf[PATH_MAX+1];
int i, max_len = -1;
if (!strcmp(path, "/"))
@@ -593,19 +593,15 @@ int longest_ancestor_length(const char *path, struct string_list *prefixes)
const char *ceil = prefixes->items[i].string;
int len = strlen(ceil);
- if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
- continue;
- if (normalize_path_copy(buf, ceil) < 0)
- continue;
- len = strlen(buf);
- if (len > 0 && buf[len-1] == '/')
- buf[--len] = '\0';
+ if (len == 1 && ceil[0] == '/')
+ len = 0; /* root matches anything, with length 0 */
+ else if (!strncmp(path, ceil, len) && path[len] == '/')
+ ; /* match of length len */
+ else
+ continue; /* no match */
- if (!strncmp(path, buf, len) &&
- path[len] == '/' &&
- len > max_len) {
+ if (len > max_len)
max_len = len;
- }
}
return max_len;
diff --git a/setup.c b/setup.c
index b4cd356..df97ad3 100644
--- a/setup.c
+++ b/setup.c
@@ -622,6 +622,28 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
}
/*
+ * A "string_list_each_func_t" function that normalizes an entry from
+ * GIT_CEILING_DIRECTORIES or discards it if unusable.
+ */
+static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
+{
+ const char *ceil = item->string;
+ int len = strlen(ceil);
+ char buf[PATH_MAX+1];
+
+ if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+ return 0;
+ if (normalize_path_copy(buf, ceil) < 0)
+ return 0;
+ len = strlen(buf);
+ if (len > 1 && buf[len-1] == '/')
+ buf[--len] = '\0';
+ free(item->string);
+ item->string = xstrdup(buf);
+ return 1;
+}
+
+/*
* We cannot decide in this function whether we are in the work tree or
* not, since the config can only be read _after_ this function was called.
*/
@@ -659,6 +681,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
if (env_ceiling_dirs) {
string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
+ filter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL);
ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
string_list_clear(&ceiling_dirs, 0);
}
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ef2345..09a42a4 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -93,47 +93,32 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX
norm_path /d1/.../d2 /d1/.../d2 POSIX
norm_path /d1/..././../d2 /d1/d2 POSIX
-ancestor / "" -1
ancestor / / -1
-ancestor /foo "" -1
-ancestor /foo : -1
-ancestor /foo ::. -1
-ancestor /foo ::..:: -1
ancestor /foo / 0
ancestor /foo /fo -1
ancestor /foo /foo -1
-ancestor /foo /foo/ -1
ancestor /foo /bar -1
-ancestor /foo /bar/ -1
ancestor /foo /foo/bar -1
-ancestor /foo /foo:/bar/ -1
-ancestor /foo /foo/:/bar/ -1
-ancestor /foo /foo::/bar/ -1
-ancestor /foo /:/foo:/bar/ 0
-ancestor /foo /foo:/:/bar/ 0
-ancestor /foo /:/bar/:/foo 0
-ancestor /foo/bar "" -1
+ancestor /foo /foo:/bar -1
+ancestor /foo /:/foo:/bar 0
+ancestor /foo /foo:/:/bar 0
+ancestor /foo /:/bar:/foo 0
ancestor /foo/bar / 0
ancestor /foo/bar /fo -1
-ancestor /foo/bar foo -1
ancestor /foo/bar /foo 4
-ancestor /foo/bar /foo/ 4
ancestor /foo/bar /foo/ba -1
ancestor /foo/bar /:/fo 0
ancestor /foo/bar /foo:/foo/ba 4
ancestor /foo/bar /bar -1
-ancestor /foo/bar /bar/ -1
-ancestor /foo/bar /fo: -1
-ancestor /foo/bar :/fo -1
-ancestor /foo/bar /foo:/bar/ 4
-ancestor /foo/bar /:/foo:/bar/ 4
-ancestor /foo/bar /foo:/:/bar/ 4
-ancestor /foo/bar /:/bar/:/fo 0
-ancestor /foo/bar /:/bar/ 0
-ancestor /foo/bar .:/foo/. 4
-ancestor /foo/bar .:/foo/.:.: 4
-ancestor /foo/bar /foo/./:.:/bar 4
-ancestor /foo/bar .:/bar -1
+ancestor /foo/bar /fo -1
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /:/foo:/bar 4
+ancestor /foo/bar /foo:/:/bar 4
+ancestor /foo/bar /:/bar:/fo 0
+ancestor /foo/bar /:/bar 0
+ancestor /foo/bar /foo 4
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /bar -1
test_expect_success 'strip_path_suffix' '
test c:/msysgit = $(test-path-utils strip_path_suffix \
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 4/8] longest_ancestor_length(): use string_list_split()
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
path.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/path.c b/path.c
index cbbdf7d..f455e8e 100644
--- a/path.c
+++ b/path.c
@@ -12,6 +12,7 @@
*/
#include "cache.h"
#include "strbuf.h"
+#include "string-list.h"
static char bad_path[] = "/bad-path/";
@@ -582,20 +583,22 @@ int normalize_path_copy(char *dst, const char *src)
*/
int longest_ancestor_length(const char *path, const char *prefix_list)
{
+ struct string_list prefixes = STRING_LIST_INIT_DUP;
char buf[PATH_MAX+1];
- const char *ceil, *colon;
- int len, max_len = -1;
+ int i, max_len = -1;
if (prefix_list == NULL || !strcmp(path, "/"))
return -1;
- for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
- for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
- len = colon - ceil;
+ string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
+
+ for (i = 0; i < prefixes.nr; i++) {
+ const char *ceil = prefixes.items[i].string;
+ int len = strlen(ceil);
+
if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
continue;
- strlcpy(buf, ceil, len+1);
- if (normalize_path_copy(buf, buf) < 0)
+ if (normalize_path_copy(buf, ceil) < 0)
continue;
len = strlen(buf);
if (len > 0 && buf[len-1] == '/')
@@ -608,6 +611,7 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
}
}
+ string_list_clear(&prefixes, 0);
return max_len;
}
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 3/8] Introduce new function real_path_if_valid()
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
The function is like real_path(), except that it returns NULL on error
instead of dying.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
abspath.c | 5 +++++
cache.h | 1 +
2 files changed, 6 insertions(+)
diff --git a/abspath.c b/abspath.c
index f8a526f..40cdc46 100644
--- a/abspath.c
+++ b/abspath.c
@@ -153,6 +153,11 @@ const char *real_path(const char *path)
return real_path_internal(path, 1);
}
+const char *real_path_if_valid(const char *path)
+{
+ return real_path_internal(path, 0);
+}
+
static const char *get_pwd_cwd(void)
{
static char cwd[PATH_MAX + 1];
diff --git a/cache.h b/cache.h
index a58df84..b0d75bc 100644
--- a/cache.h
+++ b/cache.h
@@ -714,6 +714,7 @@ static inline int is_absolute_path(const char *path)
}
int is_directory(const char *);
const char *real_path(const char *path);
+const char *real_path_if_valid(const char *path);
const char *absolute_path(const char *path);
const char *relative_path(const char *abs, const char *base);
int normalize_path_copy(char *dst, const char *src);
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 2/8] real_path_internal(): add comment explaining use of cwd
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
abspath.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/abspath.c b/abspath.c
index a7ab8e9..f8a526f 100644
--- a/abspath.c
+++ b/abspath.c
@@ -35,7 +35,14 @@ static const char *real_path_internal(const char *path, int die_on_error)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
char *retval = NULL;
+
+ /*
+ * If we have to temporarily chdir(), store the original CWD
+ * here so that we can chdir() back to it at the end of the
+ * function:
+ */
char cwd[1024] = "";
+
int buf_index = 1;
int depth = MAXDEPTH;
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 1/8] Introduce new static function real_path_internal()
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
In-Reply-To: <1350799057-13846-1-git-send-email-mhagger@alum.mit.edu>
It accepts a new parameter, die_on_error. If die_on_error is false,
it simply cleans up after itself and returns NULL rather than dying.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
abspath.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 72 insertions(+), 21 deletions(-)
diff --git a/abspath.c b/abspath.c
index 05f2d79..a7ab8e9 100644
--- a/abspath.c
+++ b/abspath.c
@@ -15,15 +15,26 @@ int is_directory(const char *path)
#define MAXDEPTH 5
/*
- * Use this to get the real path, i.e. resolve links. If you want an
- * absolute path but don't mind links, use absolute_path.
+ * Return the real path (i.e., absolute path, with symlinks resolved
+ * and extra slashes removed) equivalent to the specified path. (If
+ * you want an absolute path but don't mind links, use
+ * absolute_path().) The return value is a pointer to a static
+ * buffer.
+ *
+ * The input and all intermediate paths must be shorter than MAX_PATH.
+ * The directory part of path (i.e., everything up to the last
+ * dir_sep) must denote a valid, existing directory, but the last
+ * component need not exist. If die_on_error is set, then die with an
+ * informative error message if there is a problem. Otherwise, return
+ * NULL on errors (without generating any output).
*
* If path is our buffer, then return path, as it's already what the
* user wants.
*/
-const char *real_path(const char *path)
+static const char *real_path_internal(const char *path, int die_on_error)
{
static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];
+ char *retval = NULL;
char cwd[1024] = "";
int buf_index = 1;
@@ -35,11 +46,19 @@ const char *real_path(const char *path)
if (path == buf || path == next_buf)
return path;
- if (!*path)
- die("The empty string is not a valid path");
+ if (!*path) {
+ if (die_on_error)
+ die("The empty string is not a valid path");
+ else
+ goto error_out;
+ }
- if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
- die ("Too long path: %.*s", 60, path);
+ if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {
+ if (die_on_error)
+ die("Too long path: %.*s", 60, path);
+ else
+ goto error_out;
+ }
while (depth--) {
if (!is_directory(buf)) {
@@ -54,20 +73,36 @@ const char *real_path(const char *path)
}
if (*buf) {
- if (!*cwd && !getcwd(cwd, sizeof(cwd)))
- die_errno ("Could not get current working directory");
+ if (!*cwd && !getcwd(cwd, sizeof(cwd))) {
+ if (die_on_error)
+ die_errno("Could not get current working directory");
+ else
+ goto error_out;
+ }
- if (chdir(buf))
- die_errno ("Could not switch to '%s'", buf);
+ if (chdir(buf)) {
+ if (die_on_error)
+ die_errno("Could not switch to '%s'", buf);
+ else
+ goto error_out;
+ }
+ }
+ if (!getcwd(buf, PATH_MAX)) {
+ if (die_on_error)
+ die_errno("Could not get current working directory");
+ else
+ goto error_out;
}
- if (!getcwd(buf, PATH_MAX))
- die_errno ("Could not get current working directory");
if (last_elem) {
size_t len = strlen(buf);
- if (len + strlen(last_elem) + 2 > PATH_MAX)
- die ("Too long path name: '%s/%s'",
- buf, last_elem);
+ if (len + strlen(last_elem) + 2 > PATH_MAX) {
+ if (die_on_error)
+ die("Too long path name: '%s/%s'",
+ buf, last_elem);
+ else
+ goto error_out;
+ }
if (len && !is_dir_sep(buf[len-1]))
buf[len++] = '/';
strcpy(buf + len, last_elem);
@@ -77,10 +112,18 @@ const char *real_path(const char *path)
if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {
ssize_t len = readlink(buf, next_buf, PATH_MAX);
- if (len < 0)
- die_errno ("Invalid symlink '%s'", buf);
- if (PATH_MAX <= len)
- die("symbolic link too long: %s", buf);
+ if (len < 0) {
+ if (die_on_error)
+ die_errno("Invalid symlink '%s'", buf);
+ else
+ goto error_out;
+ }
+ if (PATH_MAX <= len) {
+ if (die_on_error)
+ die("symbolic link too long: %s", buf);
+ else
+ goto error_out;
+ }
next_buf[len] = '\0';
buf = next_buf;
buf_index = 1 - buf_index;
@@ -89,10 +132,18 @@ const char *real_path(const char *path)
break;
}
+ retval = buf;
+error_out:
+ free(last_elem);
if (*cwd && chdir(cwd))
die_errno ("Could not change back to '%s'", cwd);
- return buf;
+ return retval;
+}
+
+const char *real_path(const char *path)
+{
+ return real_path_internal(path, 1);
}
static const char *get_pwd_cwd(void)
--
1.7.11.3
^ permalink raw reply related
* [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks
From: Michael Haggerty @ 2012-10-21 5:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jiang Xin, Lea Wiemann, David Reiss, Johannes Sixt, git,
Michael Haggerty
v3 of the series, reworked WRT v2:
* Change longest_ancestor_length() to take a string_list for its
prefixes argument (instead of a PATH_SEP-separated string).
* Move the responsibility for canonicalizing prefixes from
longest_ancestor_length() to its caller in
setup_git_directory_gently_1(). This keeps
longest_ancestor_length() testable, though the test inputs now have
to be canonicalized.
* Remove function string_list_longest_prefix(), which was mainly
intended to be used in the implementation of
longest_ancestor_length() but is now unneeded.
Thanks to Junio for his comments.
I would like to explicitly point out a possible objection to this
whole enterprise. GIT_CEILING_DIRECTORIES is used to prevent git from
mucking around in certain directories, to avoid expensive accesses
(for example of remote directories). The original motivation for the
feature was a user whose home directory /home/$USER was automounted.
When git was invoked outside of a git tree, it would crawl up the
filesystem tree, eventually reaching /home, and then try to access the
paths
/home
/home/.git
/home/.git/objects
/home/objects
The last three accesses would be very expensive because the system
would attempt to automount the entries listed.
This patch series has the side effect that all of the directories
listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to
resolve any symlinks that are present in their paths. It is
admittedly odd that a feature intended to avoid accessing expensive
directories would now *intentionally* access directories near the
expensive ones. In the above scenario this shouldn't be a problem,
because /home would be the directory listed in
GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be
expensive. But there might be other scenarios for which this patch
series causes a performance regression.
Another point: After this change, it would be trivially possible to
support non-absolute paths in GIT_CEILING_DIRECTORIES; just remove the
call to is_absolute_path() from normalize_ceiling_entry(). This might
be convenient for the test suite.
Michael Haggerty (8):
Introduce new static function real_path_internal()
real_path_internal(): add comment explaining use of cwd
Introduce new function real_path_if_valid()
longest_ancestor_length(): use string_list_split()
longest_ancestor_length(): take a string_list argument for prefixes
longest_ancestor_length(): require prefix list entries to be
normalized
normalize_ceiling_entry(): resolve symlinks
string_list_longest_prefix(): remove function
Documentation/technical/api-string-list.txt | 8 ---
abspath.c | 105 ++++++++++++++++++++++------
cache.h | 3 +-
path.c | 46 ++++++------
setup.c | 32 ++++++++-
string-list.c | 20 ------
string-list.h | 8 ---
t/t0060-path-utils.sh | 41 ++++-------
t/t0063-string-list.sh | 30 --------
test-path-utils.c | 8 ++-
test-string-list.c | 20 ------
11 files changed, 157 insertions(+), 164 deletions(-)
--
1.7.11.3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox