* Re: [PATCH] fix guilt-pop and push to fail if no relevant patches
From: Josef 'Jeff' Sipek @ 2008-10-17 14:28 UTC (permalink / raw)
To: Scott Moser; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0810170736240.27798@brickies>
On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:
> Jeff,
> Did you not like the patch below for some reason ?
I don't remember my train of thought, but I ended up making a simpler patch
to address the push-pushing-more-than-it-should bug. I completely missed the
part about the exit codes.
> It seemed fairly straightforward to me that guilt-pop and guilt-push
> should exit failure if they did not do anything due to having nothing to
> do.
I'd actually say that it's not obvious, but...see below :)
> On Mon, 29 Sep 2008, Scott Moser wrote:
> > currently guilt-pop and guilt-push will exit with '0' if there are no more
> > relevant patches in the series (ie, if you've pushed or popped all of them)
> >
> > This means that you cannot do something like:
> > while guilt-push; do
> > guilt refresh || break
> > done
> >
> > for reference, quilt does exit with non-zero in those cases:
> > $ quilt push -a && quilt push
> > File series fully applied, ends at patch my.patch
> > $ echo $?
> > 1
> >
> > $ quilt pop -a; quilt pop
> > No patch removed
> > $ echo $?
> > 2
Who am I to argue against compatibility.
...
> > diff --git a/guilt-push b/guilt-push
> > index 018f9ac..48f886b 100755
> > --- a/guilt-push
> > +++ b/guilt-push
[snipped long diff]
With my fix, this should be a 2-liner :)
Sorry for missing the return code part...
Josef 'Jeff' Sipek.
--
We have joy, we have fun, we have Linux on a Sun...
^ permalink raw reply
* Re: [PATCH] fix guilt-pop and push to fail if no relevant patches
From: Scott Moser @ 2008-10-17 14:40 UTC (permalink / raw)
To: Josef 'Jeff' Sipek; +Cc: git
In-Reply-To: <20081017142832.GF27647@josefsipek.net>
On Fri, 17 Oct 2008, Josef 'Jeff' Sipek wrote:
> On Fri, Oct 17, 2008 at 07:37:33AM -0400, Scott Moser wrote:
> > Jeff,
> > Did you not like the patch below for some reason ?
>
> I don't remember my train of thought, but I ended up making a simpler patch
> to address the push-pushing-more-than-it-should bug. I completely missed the
> part about the exit codes.
>
good enough. I hadn't pulled in a while and didn't realize you'd made a
change. I never actually saw a problem with "pushing too much" , but
only in the exit codes.
> > > diff --git a/guilt-push b/guilt-push
> > > index 018f9ac..48f886b 100755
> > > --- a/guilt-push
> > > +++ b/guilt-push
> [snipped long diff]
>
> With my fix, this should be a 2-liner :)
for what its worth, the patch i sent only added a couple lines. It
really just changed indentation. So, the patch was long, only 2 new
lines that did anything.
anyway... I'm happy if you make it exit failure.
Thanks,
^ permalink raw reply
* Re: Working with remotes; cloning remote references
From: Marc Branchaud @ 2008-10-17 14:44 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Peter Harris, git
In-Reply-To: <48F83FD0.90606@drmicha.warpmail.net>
Thanks for joining the thread, Michael!
Michael J Gruber wrote:
>
> I don't think there is a direct gittish way for transferring the remote
> config from one repo to a clone (other than copying what git submodule
> does with .gitmodules etc.).
That's my impression, too, so I'm suggesting that the clone/push/pull
commands get an option to work with remotes in config.
> Would it be sufficient for you if a clone could trigger the main repo to
> update its remotes (i.e. git remote update)?
Hmmm, I'm not sure... How would the overall
merge-in-changes-from-upstream process work in that case?
Say the main repo has the code for "ThingOne" merged under a top-level
thing-one/ directory:
main/$ git remote add -f ThingOne git://thing/ThingOne.git
main/$ git merge -s ours --no-commit ThingOne/master
main/$ git read-tree --prefix=thing-one/ -i ThingOne/master
main/$ git commit -m "Merged ThingOne into /thing-one/"
Then the ThingOne folks update their code, so we want to incorporate
their changes into our version of their code. I think you're suggesting
that we might make a clone of the main repo then trigger a "git remote
update" in the clone's origin:
clone/$ make-origin-do-git-remote-update
What's not clear to me is how to proceed from here. At this point I'm
hoping there's some way I could do some work in the clone to merge the
changes the ThingOne folks did into our code. I'm too new to git to
know if there is a way forward -- is there?
(With the subtree merge pattern I'd run "git pull -s subtree ThingOne
master" in the main repo -- is there an equivalent achievable in the
clone if the main has done a "git remote update"?)
Thanks,
Marc
^ permalink raw reply
* Excluding files from git-diff
From: Erik Hahn @ 2008-10-17 14:53 UTC (permalink / raw)
To: git
I'm currently working on a script whose developer does not use
git. Hence, when I mail him the patch, I don't want to include the
.gitignore file. Is it possible to exclude a file from git-diff (except
not adding it to git, of course?)
-Erik
--
hackerkey://v4sw5hw2ln3pr5ck0ma2u7LwXm4l7Gi2e2t4b7Ken4/7a16s0r1p-5.62/-6.56g5OR
^ permalink raw reply
* Re: Working with remotes; cloning remote references
From: Michael J Gruber @ 2008-10-17 15:08 UTC (permalink / raw)
To: Marc Branchaud; +Cc: Michael J Gruber, Peter Harris, git
In-Reply-To: <48F8A4E8.8070008@xiplink.com>
Marc Branchaud venit, vidit, dixit 17.10.2008 16:44:
> Thanks for joining the thread, Michael!
Sure, there's no need to mention it.
Well, maybe there is, given the recent discussion: Everyone can
contribute (with or without C/perl/whatever fu) and thus "pay back" for
free software, even by "only" trying to answer questions. But it
requires participation, combining efforts, rather than splitting off and
not returning... Uh, sorry, your question:
> Michael J Gruber wrote:
>> Would it be sufficient for you if a clone could trigger the main repo to
>> update its remotes (i.e. git remote update)?
>
> Hmmm, I'm not sure... How would the overall
> merge-in-changes-from-upstream process work in that case?
I thought your problem was "transporting" the remote branches from main
(which tracks external) to clone, without having the remote config for
external on the clones. In that case being able to trigger "remote
update" on main from the clones would help.
> Say the main repo has the code for "ThingOne" merged under a top-level
> thing-one/ directory:
>
> main/$ git remote add -f ThingOne git://thing/ThingOne.git
> main/$ git merge -s ours --no-commit ThingOne/master
> main/$ git read-tree --prefix=thing-one/ -i ThingOne/master
> main/$ git commit -m "Merged ThingOne into /thing-one/"
>
> Then the ThingOne folks update their code, so we want to incorporate
> their changes into our version of their code. I think you're suggesting
> that we might make a clone of the main repo then trigger a "git remote
> update" in the clone's origin:
>
> clone/$ make-origin-do-git-remote-update
>
> What's not clear to me is how to proceed from here. At this point I'm
> hoping there's some way I could do some work in the clone to merge the
> changes the ThingOne folks did into our code. I'm too new to git to
> know if there is a way forward -- is there?
>
> (With the subtree merge pattern I'd run "git pull -s subtree ThingOne
> master" in the main repo -- is there an equivalent achievable in the
> clone if the main has done a "git remote update"?)
"pull -s strategy repo master" does a fetch followed by "merge -s
strategy repomaster", where repomaster is the ref for master on repo.
So, if you got that branch (repomaster=ThingOne/master) by cloning from
main you can do the merge (subtree or other) on your clone, even without
the remote repo config for ThingOne on clone.
I guess you want to do those merges on several clones, and main, and
push everything back to main. I don't know how well that mixes, but I
don't expect problems, only unexpected ones ;)
Cheers,
Michael
^ permalink raw reply
* Re: Excluding files from git-diff
From: Raphael Zimmerer @ 2008-10-17 15:08 UTC (permalink / raw)
To: Erik Hahn; +Cc: git
In-Reply-To: <20081017145313.GA23471@eriks>
On Fri, Oct 17, 2008 at 04:53:13PM +0200, Erik Hahn wrote:
> I'm currently working on a script whose developer does not use
> git. Hence, when I mail him the patch, I don't want to include the
> .gitignore file. Is it possible to exclude a file from git-diff (except
> not adding it to git, of course?)
Just add the content of .gitignore to .git/info/exclude, and remove
.gitignore from the repository.
Raphael
^ permalink raw reply
* Re: Excluding files from git-diff
From: Johannes Schindelin @ 2008-10-17 15:08 UTC (permalink / raw)
To: Erik Hahn; +Cc: git
In-Reply-To: <20081017145313.GA23471@eriks>
Hi,
On Fri, 17 Oct 2008, Erik Hahn wrote:
> I'm currently working on a script whose developer does not use git.
> Hence, when I mail him the patch, I don't want to include the .gitignore
> file. Is it possible to exclude a file from git-diff (except not adding
> it to git, of course?)
You can specify everything else:
git diff -- *
Hth,
Dscho
^ permalink raw reply
* Re: Excluding files from git-diff
From: Michael J Gruber @ 2008-10-17 15:18 UTC (permalink / raw)
To: Erik Hahn; +Cc: git
In-Reply-To: <20081017145313.GA23471@eriks>
Erik Hahn venit, vidit, dixit 17.10.2008 16:53:
> I'm currently working on a script whose developer does not use
> git. Hence, when I mail him the patch, I don't want to include the
> .gitignore file. Is it possible to exclude a file from git-diff (except
> not adding it to git, of course?)
Am I right in assuming that by "script" you mean a manuscript consisting
of several files, rather than a single script (programme in scripting
language)? In any case:
git diff commit1 commit2 file
gives you the diff for "file" between those commits, so if you're really
interested in one file that's the way to go; you can also specify more
than one file here.
On the other hand, your .gitignore probably doesn't change that often,
so that it shouldn't show up in the diff after that anyways. Or put it
in .git/info/excludes.
Cheers,
Michael
^ permalink raw reply
* Re: "checkout --track -b" broken? (with suggested fix)
From: Daniel Barkalow @ 2008-10-17 15:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jay Soffian
In-Reply-To: <7vej2fohfr.fsf@gitster.siamese.dyndns.org>
On Thu, 16 Oct 2008, Junio C Hamano wrote:
> Sorry for bringing up a potentially old issue, but I do not think the test
> added by 9ed36cf (branch: optionally setup branch.*.merge from upstream
> local branches, 2008-02-19) is correct. It does this:
>
> test_expect_success \
> 'checkout w/--track from non-branch HEAD fails' '
> git checkout -b delete-me master &&
> rm .git/refs/heads/delete-me &&
> test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
> test_must_fail git checkout --track -b track'
>
> It creates branch 'delete-me' at the tip of 'master' to check it out, and
> then it manually deletes the branch. It expects "checkout --track -b track"
> to fail because the current branch is broken and there is no way to set up
> a tracking information for the new branch.
>
> But I think this is bogus. The checkout fails not because of lack of
> trackability (due to broken current _branch_), but because there is no
> current _commit_ to branch from.
>
> Jay CC'ed because 9ed36cf is his; Daniel CC'ed as branch.c was
> originally his.
>
> Here is an attempt to fix the test, which then revealed that the feature
> the commit introduced is broken in the tip of 'maint'. Back when 9ed36cf
> was written, test_must_fail was unavailable, and test_expect_failure meant
> something different, so transplanting this on top of that old commit won't
> reveal the breakage, but I strongly suspect that the feature was broken
> from the very beginning.
>
> The patch to branch.c is a quick fix for this issue. The resulting code
> passes all the tests, but I am not very proud of hardcoding the "HEAD" in
> the code. There must be a better way to do this.
I agree with the change to the test. I think it would be better to
hard-code "refs/heads/" instead of "HEAD", and I feel like we must have a
"is this ref name a branch?" function, if only because someone could stick
"refs/tags/foo" in HEAD, and we should still say it's not something you
could track, despite it being something different from "HEAD".
> branch.c | 4 +++-
> t/t7201-co.sh | 11 +++++------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git c/branch.c w/branch.c
> index b1e59f2..6a75057 100644
> --- c/branch.c
> +++ w/branch.c
> @@ -129,7 +129,9 @@ void create_branch(const char *head,
> die("Cannot setup tracking information; starting point is not a branch.");
> break;
> case 1:
> - /* Unique completion -- good */
> + /* Unique completion -- good, only if it is a real ref */
> + if (track == BRANCH_TRACK_EXPLICIT && !strcmp(real_ref, "HEAD"))
> + die("Cannot setup tracking information; starting point is not a branch.");
> break;
> default:
> die("Ambiguous object name: '%s'.", start_name);
> diff --git c/t/t7201-co.sh w/t/t7201-co.sh
> index fbec70d..da1fbf8 100755
> --- c/t/t7201-co.sh
> +++ w/t/t7201-co.sh
> @@ -330,12 +330,11 @@ test_expect_success \
> test "$(git config branch.track2.merge)"
> git config branch.autosetupmerge false'
>
> -test_expect_success \
> - 'checkout w/--track from non-branch HEAD fails' '
> - git checkout -b delete-me master &&
> - rm .git/refs/heads/delete-me &&
> - test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
> - test_must_fail git checkout --track -b track'
> +test_expect_success 'checkout w/--track from non-branch HEAD fails' '
> + git checkout master^0 &&
> + test_must_fail git symbolic-ref HEAD &&
> + test_must_fail git checkout --track -b track
> +'
>
> test_expect_success 'checkout an unmerged path should fail' '
> rm -f .git/index &&
>
^ permalink raw reply
* Howto setup-git-server-over-http.txt with SSL and basic authentication?
From: Josef Wolf @ 2008-10-17 16:06 UTC (permalink / raw)
To: git
Hello folks,
I am new to git and I am trying to set up a git repository as described in
http://www.kernel.org/pub/software/scm/git/docs/howto/setup-git-server-over-http.txt
but with SSL and basic authentication.
This is what I have done:
1. Create bare git repository on the server:
root: mkdir -p /data/git/test
root: ( cd /data/git/test ; git --bare init )
root: chown -R wwwrun:www /data/git
2. Add a new DAV location to the (existing and already working) apache
config, pointing to the newly created git repository. I list only
the relevant parts here:
LoadModule dav_svn_module /usr/lib/apache2/mod_dav_svn.so
LoadModule dav_fs_module /usr/lib/apache2/mod_dav_fs.so
DAVLockDB "/data/dav/lock/DAV.lock"
Alias /git/test /data/git/test
<Location /git/test>
DAV on
Order Allow,Deny
Allow from all
AllowOverride None
AuthName "test"
AuthType Basic
AuthUserFile /m/b/httpd/passwd
AuthGroupFile /m/b/httpd/group
Require group test test-ro
SSLRequireSSL
<LimitExcept GET PROPFIND OPTIONS REPORT>
Require group test
</LimitExcept>
</Location>
3. Now it's time to test DAV access, so I go to the client:
konqueror webdavs://repo.host.org/git/test
After asking for credentials, Konqueror shows me content, and I can
read/copy/delete files to/from the DAV directory. So the server
seems to work fine.
Now I go test curl:
$ curl --cacert /etc/cacerts/myca.pem \
https://user@repo.host.org/git/test/HEAD
fails with "authentication required", but
$ curl --cacert /etc/cacerts/myca.pem --user user \
https://user@repo.host.org/git/test/HEAD
works fine. So I put this information into ~/.curlrc:
$ cat ~/.curlrc
--cacert /etc/cacerts/myca.pem
--user user
$ curl https://repo.host.org/git/test/HEAD
Enter host password for user 'user':
ref: refs/heads/master
$
this looks good now.
4. OK, now I go to the next step on the client:
$ git-config remote.upload.url https://repo.host.org/git/test/
error: could not lock config file .git/config
$
hmm, maybe I should have an empty repos here? So:
$ git init
Initialized empty Git repository in /tmp/test/.git/
$ git-config remote.upload.url https://repo.host.org/git/test/
$ git push upload master
error: Cannot access URL https://repo.host.org/git/test/, return code 60
error: failed to push some refs to 'https://repo.host.org/git/test/'
OK, from the above mentioned howto, this looks like cacert is missing.
Looks like (unlike the howto states) ~/.curlrc is ignored by git.
So I go searching for appropriate configuration options in git:
$ git-config http.sslCAPath /etc/cacerts
don't help (why?), but
$ git-config http.sslCAInfo /etc/cacerts/myca.pem
$ git push upload master
error: Cannot access URL https://repo.host.org/git/test/, return code 22
error: failed to push some refs to 'https://repo.host.org/git/test/'
Finally, that's a new error code. This is probably because the server
requires authentication. But I can't find any hints how to specify
credentials in git-config or git-push.
So now come my questions:
0. The howto says curl is used for transport. Why is my ~/.curlrc ignored?
1. Since .curlrc is ignored: How do I specify credentials for git?
2. Why don't sslCAPath work?
3. Is there a way to override credentials and sslCAPath on a per-remote
basis (as can be done with http.proxy, for example)
Any hints?
^ permalink raw reply
* Re: Excluding files from git-diff
From: Christian Jaeger @ 2008-10-17 16:30 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Erik Hahn, git
In-Reply-To: <48F8ACC2.1010903@drmicha.warpmail.net>
Michael J Gruber wrote:
> your .gitignore probably doesn't change that often,
> so that it shouldn't show up in the diff after that anyways. Or put it
> in .git/info/excludes.
For me, adding entries to that file does not make "git diff" or "gitk"
or even "git ls-files" ignore files matching the entries. Only "git
ls-files --others --exclude-from=.git/info/exclude" will exclude them.
And "git diff " and gitk don't seem to know the --exclude-from option.
Is there a way to really invert the patterns given to "git diff" or
alike? I.e. instead of saying "git diff -- * .somedotfile
.someothernongitignoredotfile" one could just say something like "git
diff --invert-matches -- .gitignore"? And even better, could one
configure some such so that it has effect on all tools by default?
(That would help working around the clutter problem from the "Separating
generated files?" thread; i.e. in some cases this could be good enough
to not require splitting a project into multiple repositories. (Although
I'm all the same currently working on a
"intergit-find-matching-commit-in" script, since there may be more good
reasons than only the clutter to split projects.))
Christian.
^ permalink raw reply
* Re: Excluding files from git-diff
From: Eric Raible @ 2008-10-17 17:33 UTC (permalink / raw)
To: git
In-Reply-To: <48F8BDA7.50901@pflanze.mine.nu>
Christian Jaeger <christian <at> pflanze.mine.nu> writes:
>
> Michael J Gruber wrote:
> > Or put it
> > in .git/info/excludes.
>
> Only "git ls-files --others --exclude-from=.git/info/exclude" will exclude
Could it be a simple as "excludes" vs "exclude"?
- Eric
^ permalink raw reply
* Re: GitTogether topics status (4th of October)
From: René Scharfe @ 2008-10-17 19:18 UTC (permalink / raw)
To: Christian Couder
Cc: git, Shawn O. Pearce, Junio C Hamano, Scott Chacon, Sam Vilain,
Petr Baudis
In-Reply-To: <200810041816.41026.chriscool@tuxfamily.org>
Christian Couder schrieb:
> Hi,
>
> As can be seen on the GitTogether page on the wiki:
>
> http://git.or.cz/gitwiki/GitTogether
While I won't be there, another good topic might be the usage of a patch
tracker like patchwork (http://ozlabs.org/~jk/projects/patchwork/).
René
^ permalink raw reply
* Re: --diff-filter=T does not list x changes
From: Anders Melchiorsen @ 2008-10-17 19:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <7v1vyfoca2.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Anders Melchiorsen <mail@cup.kalibalik.dk> writes:
>
>> I hope you agree that this reading is not obvious from the
>> documentation,...
>
> Yup, didn't I already say that the documentation is buggy?
Possibly, though not in this thread.
>> How about adding a diff-filter=X for the executable bit?
>
> I do not think it is a good idea for two reasons. Backward
> compatibility and sane design.
>
> For one thing, "diff --name-status" never shows X, so you would
> introduce an unnecessary inconsistency. If you change
> "--name-status" to avoid that, you would be breaking people's
> existing scripts that expect to see "M" for such a change.
(I noticed that X is already used in diff-filter, but will keep it for
this discussion)
I was thinking that X could be a subset of M. So only if you
specifically ask for diff-filter=X (and not M) would you get this new
functionality. That should keep it compatible. It would then pick
files that have had their x flipped, regardless of their change in
content. With diff-filter=M, it would work as it does today.
If name-status output must be consistent, it could even output M for
these changes. That would still be unambiguous (but probably confusing).
...
As you say that this is an unnecessary inconsistency, I wonder whether
you have a different way to pick out the commits that toggle the x
bit? That is a problem that I am facing, with no solution shown so far ...
Anders.
^ permalink raw reply
* Re: Working with remotes; cloning remote references
From: Marc Branchaud @ 2008-10-17 19:50 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Peter Harris, git
In-Reply-To: <48F8AA5E.6090908@drmicha.warpmail.net>
Michael J Gruber wrote:
>
> "pull -s strategy repo master" does a fetch followed by "merge -s
> strategy repomaster", where repomaster is the ref for master on repo.
> So, if you got that branch (repomaster=ThingOne/master) by cloning from
> main you can do the merge (subtree or other) on your clone, even without
> the remote repo config for ThingOne on clone.
I'm afraid I'm having trouble translating what you're saying into actual
git commands (or are you proposing some new git functionality?). How
would I get the ThingOne/master branch into the clone?
After some more thought I realized that the clone can just pull directly
from the ThingOne repository:
clone/$ git pull -s subtree git://thing/ThingOne.git master
(I'm still getting used to git's ability to match commit IDs from
anywhere -- it's magic! :) )
This goes a long way to where we want to be, in that we don't have to do
our merging work in the original main repository.
It would be nice, though, if the clone were able to use the main
repository's definition of the ThingOne remote. I can think of some
plausible scenarios where a person could get confused about which
repo/branch they're supposed to pull. It's easy to recover from that
kind of mistake, but there'd be less chance of a mistake if one could
tell git to "pull from X as defined in the origin repository".
And actually, git's remote functionality feels a bit crippled if clones
can't make some use of the origin's remotes. Is there a reason for
keeping remote definitions out of a clone?
Marc
^ permalink raw reply
* [PATCH 1/2] index-pack: rationalize delta resolution code
From: Nicolas Pitre @ 2008-10-17 19:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Instead of having strange loops for walking unresolved deltas with the
same base duplicated in many places, let's rework the code so this is
done in a single place instead. This simplifies callers quite a bit too.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
index-pack.c | 123 +++++++++++++++++++++++++++------------------------------
1 files changed, 58 insertions(+), 65 deletions(-)
diff --git a/index-pack.c b/index-pack.c
index d3a4d31..82843e7 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -244,7 +244,8 @@ static void link_base_data(struct base_data *base, struct base_data *c)
c->base = base;
c->child = NULL;
- base_cache_used += c->size;
+ if (c->data)
+ base_cache_used += c->size;
prune_base_data(c);
}
@@ -494,8 +495,10 @@ static void *get_base_data(struct base_data *c)
free(raw);
if (!c->data)
bad_object(obj->idx.offset, "failed to apply delta");
- } else
+ } else {
c->data = get_data_from_pack(obj);
+ c->size = obj->size;
+ }
base_cache_used += c->size;
prune_base_data(c);
@@ -504,49 +507,73 @@ static void *get_base_data(struct base_data *c)
}
static void resolve_delta(struct object_entry *delta_obj,
- struct base_data *base_obj, enum object_type type)
+ struct base_data *base, struct base_data *result)
{
void *delta_data;
unsigned long delta_size;
- union delta_base delta_base;
- int j, first, last;
- struct base_data result;
- delta_obj->real_type = type;
+ delta_obj->real_type = base->obj->type;
delta_data = get_data_from_pack(delta_obj);
delta_size = delta_obj->size;
- result.data = patch_delta(get_base_data(base_obj), base_obj->size,
- delta_data, delta_size,
- &result.size);
+ result->obj = delta_obj;
+ result->data = patch_delta(get_base_data(base), base->obj->size,
+ delta_data, delta_size, &result->size);
free(delta_data);
- if (!result.data)
+ if (!result->data)
bad_object(delta_obj->idx.offset, "failed to apply delta");
- sha1_object(result.data, result.size, type, delta_obj->idx.sha1);
+ sha1_object(result->data, result->size, delta_obj->real_type,
+ delta_obj->idx.sha1);
nr_resolved_deltas++;
+}
+
+static void find_unresolved_deltas(struct base_data *base,
+ struct base_data *prev_base)
+{
+ int i, ref, ref_first, ref_last, ofs, ofs_first, ofs_last;
+
+ /*
+ * This is a recursive function. Those brackets should help reducing
+ * stack usage by limiting the scope of the delta_base union.
+ */
+ {
+ union delta_base base_spec;
+
+ hashcpy(base_spec.sha1, base->obj->idx.sha1);
+ ref = !find_delta_children(&base_spec, &ref_first, &ref_last);
+
+ memset(&base_spec, 0, sizeof(base_spec));
+ base_spec.offset = base->obj->idx.offset;
+ ofs = !find_delta_children(&base_spec, &ofs_first, &ofs_last);
+ }
+
+ if (!ref && !ofs)
+ return;
- result.obj = delta_obj;
- link_base_data(base_obj, &result);
+ link_base_data(prev_base, base);
- hashcpy(delta_base.sha1, delta_obj->idx.sha1);
- if (!find_delta_children(&delta_base, &first, &last)) {
- for (j = first; j <= last; j++) {
- struct object_entry *child = objects + deltas[j].obj_no;
- if (child->real_type == OBJ_REF_DELTA)
- resolve_delta(child, &result, type);
+ if (ref) {
+ for (i = ref_first; i <= ref_last; i++) {
+ struct object_entry *child = objects + deltas[i].obj_no;
+ if (child->real_type == OBJ_REF_DELTA) {
+ struct base_data result;
+ resolve_delta(child, base, &result);
+ find_unresolved_deltas(&result, base);
+ }
}
}
- memset(&delta_base, 0, sizeof(delta_base));
- delta_base.offset = delta_obj->idx.offset;
- if (!find_delta_children(&delta_base, &first, &last)) {
- for (j = first; j <= last; j++) {
- struct object_entry *child = objects + deltas[j].obj_no;
- if (child->real_type == OBJ_OFS_DELTA)
- resolve_delta(child, &result, type);
+ if (ofs) {
+ for (i = ofs_first; i <= ofs_last; i++) {
+ struct object_entry *child = objects + deltas[i].obj_no;
+ if (child->real_type == OBJ_OFS_DELTA) {
+ struct base_data result;
+ resolve_delta(child, base, &result);
+ find_unresolved_deltas(&result, base);
+ }
}
}
- unlink_base_data(&result);
+ unlink_base_data(base);
}
static int compare_delta_entry(const void *a, const void *b)
@@ -622,37 +649,13 @@ static void parse_pack_objects(unsigned char *sha1)
progress = start_progress("Resolving deltas", nr_deltas);
for (i = 0; i < nr_objects; i++) {
struct object_entry *obj = &objects[i];
- union delta_base base;
- int j, ref, ref_first, ref_last, ofs, ofs_first, ofs_last;
struct base_data base_obj;
if (obj->type == OBJ_REF_DELTA || obj->type == OBJ_OFS_DELTA)
continue;
- hashcpy(base.sha1, obj->idx.sha1);
- ref = !find_delta_children(&base, &ref_first, &ref_last);
- memset(&base, 0, sizeof(base));
- base.offset = obj->idx.offset;
- ofs = !find_delta_children(&base, &ofs_first, &ofs_last);
- if (!ref && !ofs)
- continue;
- base_obj.data = get_data_from_pack(obj);
- base_obj.size = obj->size;
base_obj.obj = obj;
- link_base_data(NULL, &base_obj);
-
- if (ref)
- for (j = ref_first; j <= ref_last; j++) {
- struct object_entry *child = objects + deltas[j].obj_no;
- if (child->real_type == OBJ_REF_DELTA)
- resolve_delta(child, &base_obj, obj->type);
- }
- if (ofs)
- for (j = ofs_first; j <= ofs_last; j++) {
- struct object_entry *child = objects + deltas[j].obj_no;
- if (child->real_type == OBJ_OFS_DELTA)
- resolve_delta(child, &base_obj, obj->type);
- }
- unlink_base_data(&base_obj);
+ base_obj.data = NULL;
+ find_unresolved_deltas(&base_obj, NULL);
display_progress(progress, nr_resolved_deltas);
}
}
@@ -745,7 +748,6 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
for (i = 0; i < n; i++) {
struct delta_entry *d = sorted_by_pos[i];
enum object_type type;
- int j, first, last;
struct base_data base_obj;
if (objects[d->obj_no].real_type != OBJ_REF_DELTA)
@@ -759,16 +761,7 @@ static void fix_unresolved_deltas(struct sha1file *f, int nr_unresolved)
die("local object %s is corrupt", sha1_to_hex(d->base.sha1));
base_obj.obj = append_obj_to_pack(f, d->base.sha1,
base_obj.data, base_obj.size, type);
- link_base_data(NULL, &base_obj);
-
- find_delta_children(&d->base, &first, &last);
- for (j = first; j <= last; j++) {
- struct object_entry *child = objects + deltas[j].obj_no;
- if (child->real_type == OBJ_REF_DELTA)
- resolve_delta(child, &base_obj, type);
- }
-
- unlink_base_data(&base_obj);
+ find_unresolved_deltas(&base_obj, NULL);
display_progress(progress, nr_resolved_deltas);
}
free(sorted_by_pos);
--
1.6.0.2.711.gf1ba4
^ permalink raw reply related
* [PATCH 2/2] index-pack: smarter memory usage during delta resolution
From: Nicolas Pitre @ 2008-10-17 19:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1224273478-18384-1-git-send-email-nico@cam.org>
There is no need to keep the base object data around after its last delta
has been resolved. This also means that long delta chains with only one
delta per base won't grow the cache size unnecessarily as the base will
be freed before recursing down.
To make it easy, find_delta_children() is modified so the first and last
indices are initialized in all cases.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
index-pack.c | 73 +++++++++++++++++++++++++++++++---------------------------
1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/index-pack.c b/index-pack.c
index 82843e7..9def641 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -221,17 +221,23 @@ static void bad_object(unsigned long offset, const char *format, ...)
die("pack has bad object at offset %lu: %s", offset, buf);
}
+static void free_base_data(struct base_data *c)
+{
+ if (c->data) {
+ free(c->data);
+ c->data = NULL;
+ base_cache_used -= c->size;
+ }
+}
+
static void prune_base_data(struct base_data *retain)
{
struct base_data *b = base_cache;
for (b = base_cache;
base_cache_used > delta_base_cache_limit && b;
b = b->child) {
- if (b->data && b != retain) {
- free(b->data);
- b->data = NULL;
- base_cache_used -= b->size;
- }
+ if (b->data && b != retain)
+ free_base_data(b);
}
}
@@ -256,10 +262,7 @@ static void unlink_base_data(struct base_data *c)
base->child = NULL;
else
base_cache = NULL;
- if (c->data) {
- free(c->data);
- base_cache_used -= c->size;
- }
+ free_base_data(c);
}
static void *unpack_entry_data(unsigned long offset, unsigned long size)
@@ -409,22 +412,24 @@ static int find_delta(const union delta_base *base)
return -first-1;
}
-static int find_delta_children(const union delta_base *base,
- int *first_index, int *last_index)
+static void find_delta_children(const union delta_base *base,
+ int *first_index, int *last_index)
{
int first = find_delta(base);
int last = first;
int end = nr_deltas - 1;
- if (first < 0)
- return -1;
+ if (first < 0) {
+ *first_index = 0;
+ *last_index = -1;
+ return;
+ }
while (first > 0 && !memcmp(&deltas[first - 1].base, base, UNION_BASE_SZ))
--first;
while (last < end && !memcmp(&deltas[last + 1].base, base, UNION_BASE_SZ))
++last;
*first_index = first;
*last_index = last;
- return 0;
}
static void sha1_object(const void *data, unsigned long size,
@@ -529,7 +534,7 @@ static void resolve_delta(struct object_entry *delta_obj,
static void find_unresolved_deltas(struct base_data *base,
struct base_data *prev_base)
{
- int i, ref, ref_first, ref_last, ofs, ofs_first, ofs_last;
+ int i, ref_first, ref_last, ofs_first, ofs_last;
/*
* This is a recursive function. Those brackets should help reducing
@@ -539,37 +544,37 @@ static void find_unresolved_deltas(struct base_data *base,
union delta_base base_spec;
hashcpy(base_spec.sha1, base->obj->idx.sha1);
- ref = !find_delta_children(&base_spec, &ref_first, &ref_last);
+ find_delta_children(&base_spec, &ref_first, &ref_last);
memset(&base_spec, 0, sizeof(base_spec));
base_spec.offset = base->obj->idx.offset;
- ofs = !find_delta_children(&base_spec, &ofs_first, &ofs_last);
+ find_delta_children(&base_spec, &ofs_first, &ofs_last);
}
- if (!ref && !ofs)
+ if (ref_last == -1 && ofs_last == -1)
return;
link_base_data(prev_base, base);
- if (ref) {
- for (i = ref_first; i <= ref_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- if (child->real_type == OBJ_REF_DELTA) {
- struct base_data result;
- resolve_delta(child, base, &result);
- find_unresolved_deltas(&result, base);
- }
+ for (i = ref_first; i <= ref_last; i++) {
+ struct object_entry *child = objects + deltas[i].obj_no;
+ if (child->real_type == OBJ_REF_DELTA) {
+ struct base_data result;
+ resolve_delta(child, base, &result);
+ if (i == ref_last && ofs_last == -1)
+ free_base_data(base);
+ find_unresolved_deltas(&result, base);
}
}
- if (ofs) {
- for (i = ofs_first; i <= ofs_last; i++) {
- struct object_entry *child = objects + deltas[i].obj_no;
- if (child->real_type == OBJ_OFS_DELTA) {
- struct base_data result;
- resolve_delta(child, base, &result);
- find_unresolved_deltas(&result, base);
- }
+ for (i = ofs_first; i <= ofs_last; i++) {
+ struct object_entry *child = objects + deltas[i].obj_no;
+ if (child->real_type == OBJ_OFS_DELTA) {
+ struct base_data result;
+ resolve_delta(child, base, &result);
+ if (i == ofs_last)
+ free_base_data(base);
+ find_unresolved_deltas(&result, base);
}
}
--
1.6.0.2.711.gf1ba4
^ permalink raw reply related
* Re: [ANNOUNCE] cgit 0.8
From: Ondrej Certik @ 2008-10-17 21:31 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Asheesh Laroia, Git Mailing List
In-Reply-To: <8c5c35580810051522l3c8fcfe4md8b78b569f287246@mail.gmail.com>
On Mon, Oct 6, 2008 at 12:22 AM, Lars Hjemli <hjemli@gmail.com> wrote:
> On Sun, Oct 5, 2008 at 22:49, Asheesh Laroia <git@asheesh.org> wrote:
>> On Sun, 5 Oct 2008, Lars Hjemli wrote:
>>
>>> cgit-0.8, another webinterface for git, is now available.
>>>
>> I'm curious - is there any interest in the cgit world in providing gitweb
>> URL compatibility?
>
> Well, it's a request that's popped up a few times, but no patches so
> far. It would probably be a nice feature if it could be done cleanly.
While we are at the urls, I prefer the urls that mercurial has, e.g.:
http://hg.sympy.org/sympy/rev/62b1589fefa7
is there any reason why cgit uses
http://hjemli.net/git/cgit/commit/?id=140012d
instead of:
http://hjemli.net/git/cgit/commit/140012d
?
If its just a matter of preparing a patch, I'll do that.
Ondrej
^ permalink raw reply
* Re: Detached checkout will clobber branch head when using symlink HEAD
From: Junio C Hamano @ 2008-10-17 23:08 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Draisey, git
In-Reply-To: <20081016203916.GB9487@coredump.intra.peff.net>
I do not think that is a correct approach.
The offending callchain is:
update_ref(..., "HEAD", REF_NODEREF, ...);
-> lock_any_ref_for_update("HEAD", ..., REF_NODEREF);
-> lock_ref_sha1_basic("HEAD", ..., REF_NODEREF, ...);
. calls resolve_ref() to read HEAD to arrive at
refs/heads/master
. however, it notices REF_NODEREF and adjusts the ref to be updated
back to "HEAD";
-> hold_lock_file_for_update(..., "HEAD", 1);
-> lock_file(..., "HEAD");
. resolves symlink "HEAD" to "refs/heads/master", and
locks it! This creates "refs/heads/master.lock", that is
then renamed to "refs/heads/master" when unlocked.
In other words, the breakage is in lock_file() and not in resolve_ref().
The latter gives the same output to the caller whether the HEAD is
symbolic link or textual symref -- at least it should.
The behaviour of lock_file() to resolve symlink at this point in the code
comes from d58e8d3 (When locking in a symlinked repository, try to lock
the original, 2007-07-25), and as explained in the log message of that
commit, we cannot unconditionally remove it.
Three patches to fix this issue, that come on top of the fix to t7201
(--track from detached HEAD should fail) I sent out last night, will
follow.
^ permalink raw reply
* [PATCH 1/3] demonstrate breakage of detached checkout with symbolic link HEAD
From: Junio C Hamano @ 2008-10-17 23:10 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Draisey, git
In-Reply-To: <7vfxmuhlad.fsf@gitster.siamese.dyndns.org>
When core.prefersymlinkrefs is in use, detaching the HEAD by
checkout incorrectly clobbers the tip of the current branch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t7201-co.sh | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index ee2cab6..01304d7 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,6 +339,18 @@ test_expect_success 'checkout w/--track from non-branch HEAD fails' '
test "z$(git rev-parse master^0)" = "z$(git rev-parse HEAD)"
'
+test_expect_failure 'detch a symbolic link HEAD' '
+ git checkout master &&
+ git config --bool core.prefersymlinkrefs yes &&
+ git checkout side &&
+ git checkout master &&
+ it=$(git symbolic-ref HEAD) &&
+ test "z$it" = zrefs/heads/master &&
+ here=$(git rev-parse --verify refs/heads/master) &&
+ git checkout side^ &&
+ test "z$(git rev-parse --verify refs/heads/master)" = "z$here"
+'
+
test_expect_success 'checkout an unmerged path should fail' '
rm -f .git/index &&
O=$(echo original | git hash-object -w --stdin) &&
--
1.6.0.2.734.gae0be
^ permalink raw reply related
* [PATCH 2/3] Enhance hold_lock_file_for_{update,append}() API
From: Junio C Hamano @ 2008-10-17 23:10 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Draisey, git
In-Reply-To: <7vfxmuhlad.fsf@gitster.siamese.dyndns.org>
This changes the "die_on_error" boolean parameter to a mere "flags", and
changes the existing callers of hold_lock_file_for_update/append()
functions to pass LOCK_DIE_ON_ERROR.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-commit.c | 3 ++-
builtin-fetch-pack.c | 3 ++-
builtin-revert.c | 3 ++-
bundle.c | 3 ++-
cache.h | 1 +
lockfile.c | 26 +++++++++++++++-----------
pack-refs.c | 3 ++-
refs.c | 3 ++-
rerere.c | 3 ++-
sha1_file.c | 2 +-
10 files changed, 31 insertions(+), 19 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index e2a7e48..b563a0d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -320,7 +320,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
die("unable to write new_index file");
fd = hold_lock_file_for_update(&false_lock,
- git_path("next-index-%d", getpid()), 1);
+ git_path("next-index-%d", getpid()),
+ LOCK_DIE_ON_ERROR);
create_base_index();
add_remove_files(&partial);
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 85509f5..21ce3e0 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -813,7 +813,8 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
)
die("shallow file was changed during fetch");
- fd = hold_lock_file_for_update(&lock, shallow, 1);
+ fd = hold_lock_file_for_update(&lock, shallow,
+ LOCK_DIE_ON_ERROR);
if (!write_shallow_commits(fd, 0)) {
unlink(shallow);
rollback_lock_file(&lock);
diff --git a/builtin-revert.c b/builtin-revert.c
index 27881e9..e839387 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -338,7 +338,8 @@ static int revert_or_cherry_pick(int argc, const char **argv)
* reverse of it if we are revert.
*/
- msg_fd = hold_lock_file_for_update(&msg_file, defmsg, 1);
+ msg_fd = hold_lock_file_for_update(&msg_file, defmsg,
+ LOCK_DIE_ON_ERROR);
encoding = get_encoding(message);
if (!encoding)
diff --git a/bundle.c b/bundle.c
index 00b2aab..7d17a1f 100644
--- a/bundle.c
+++ b/bundle.c
@@ -186,7 +186,8 @@ int create_bundle(struct bundle_header *header, const char *path,
if (bundle_to_stdout)
bundle_fd = 1;
else
- bundle_fd = hold_lock_file_for_update(&lock, path, 1);
+ bundle_fd = hold_lock_file_for_update(&lock, path,
+ LOCK_DIE_ON_ERROR);
/* write signature */
write_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));
diff --git a/cache.h b/cache.h
index 884fae8..941a9dc 100644
--- a/cache.h
+++ b/cache.h
@@ -411,6 +411,7 @@ struct lock_file {
char on_list;
char filename[PATH_MAX];
};
+#define LOCK_DIE_ON_ERROR 1
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index 4023797..bc1b585 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -121,9 +121,10 @@ static char *resolve_symlink(char *p, size_t s)
}
-static int lock_file(struct lock_file *lk, const char *path)
+static int lock_file(struct lock_file *lk, const char *path, int flags)
{
- if (strlen(path) >= sizeof(lk->filename)) return -1;
+ if (strlen(path) >= sizeof(lk->filename))
+ return -1;
strcpy(lk->filename, path);
/*
* subtract 5 from size to make sure there's room for adding
@@ -155,21 +156,21 @@ static int lock_file(struct lock_file *lk, const char *path)
return lk->fd;
}
-int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on_error)
+int hold_lock_file_for_update(struct lock_file *lk, const char *path, int flags)
{
- int fd = lock_file(lk, path);
- if (fd < 0 && die_on_error)
+ int fd = lock_file(lk, path, flags);
+ if (fd < 0 && (flags & LOCK_DIE_ON_ERROR))
die("unable to create '%s.lock': %s", path, strerror(errno));
return fd;
}
-int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on_error)
+int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags)
{
int fd, orig_fd;
- fd = lock_file(lk, path);
+ fd = lock_file(lk, path, flags);
if (fd < 0) {
- if (die_on_error)
+ if (flags & LOCK_DIE_ON_ERROR)
die("unable to create '%s.lock': %s", path, strerror(errno));
return fd;
}
@@ -177,13 +178,13 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int die_on
orig_fd = open(path, O_RDONLY);
if (orig_fd < 0) {
if (errno != ENOENT) {
- if (die_on_error)
+ if (flags & LOCK_DIE_ON_ERROR)
die("cannot open '%s' for copying", path);
close(fd);
return error("cannot open '%s' for copying", path);
}
} else if (copy_fd(orig_fd, fd)) {
- if (die_on_error)
+ if (flags & LOCK_DIE_ON_ERROR)
exit(128);
close(fd);
return -1;
@@ -215,7 +216,10 @@ int commit_lock_file(struct lock_file *lk)
int hold_locked_index(struct lock_file *lk, int die_on_error)
{
- return hold_lock_file_for_update(lk, get_index_file(), die_on_error);
+ return hold_lock_file_for_update(lk, get_index_file(),
+ die_on_error
+ ? LOCK_DIE_ON_ERROR
+ : 0);
}
void set_alternate_index_output(const char *name)
diff --git a/pack-refs.c b/pack-refs.c
index 848d311..2c76fb1 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -89,7 +89,8 @@ int pack_refs(unsigned int flags)
memset(&cbdata, 0, sizeof(cbdata));
cbdata.flags = flags;
- fd = hold_lock_file_for_update(&packed, git_path("packed-refs"), 1);
+ fd = hold_lock_file_for_update(&packed, git_path("packed-refs"),
+ LOCK_DIE_ON_ERROR);
cbdata.refs_file = fdopen(fd, "w");
if (!cbdata.refs_file)
die("unable to create ref-pack file structure (%s)",
diff --git a/refs.c b/refs.c
index 39a3b23..5467e98 100644
--- a/refs.c
+++ b/refs.c
@@ -845,7 +845,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
error("unable to create directory for %s", ref_file);
goto error_return;
}
- lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, 1);
+ lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file,
+ LOCK_DIE_ON_ERROR);
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
diff --git a/rerere.c b/rerere.c
index 323e493..2b7a99d 100644
--- a/rerere.c
+++ b/rerere.c
@@ -346,7 +346,8 @@ int setup_rerere(struct string_list *merge_rr)
return -1;
merge_rr_path = xstrdup(git_path("MERGE_RR"));
- fd = hold_lock_file_for_update(&write_lock, merge_rr_path, 1);
+ fd = hold_lock_file_for_update(&write_lock, merge_rr_path,
+ LOCK_DIE_ON_ERROR);
read_rr(merge_rr);
return fd;
}
diff --git a/sha1_file.c b/sha1_file.c
index e2cb342..5cfae5d 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -385,7 +385,7 @@ static void read_info_alternates(const char * relative_base, int depth)
void add_to_alternates_file(const char *reference)
{
struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), 1);
+ int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
char *alt = mkpath("%s/objects\n", reference);
write_or_die(fd, alt, strlen(alt));
if (commit_lock_file(lock))
--
1.6.0.2.734.gae0be
^ permalink raw reply related
* [PATCH 3/3] Fix checkout not to clobber the branch when using symlinked HEAD upon detaching
From: Junio C Hamano @ 2008-10-17 23:11 UTC (permalink / raw)
To: Jeff King; +Cc: Matt Draisey, git
In-Reply-To: <7vfxmuhlad.fsf@gitster.siamese.dyndns.org>
When you are using core.prefersymlinkrefs (i.e. your ".git/HEAD" is a
symlink to "refs/heads/$current_branch"), attempt to detach HEAD resulted
in clobbering the tip of the current branch.
The offending callchain is:
update_ref(..., "HEAD", REF_NODEREF, ...);
-> lock_any_ref_for_update("HEAD", ..., REF_NODEREF);
-> lock_ref_sha1_basic("HEAD", ..., REF_NODEREF, ...);
. calls resolve_ref() to read HEAD to arrive at
refs/heads/master
. however, it notices REF_NODEREF and adjusts the ref to be updated
back to "HEAD";
-> hold_lock_file_for_update(..., "HEAD", 1);
-> lock_file(..., "HEAD");
. resolves symlink "HEAD" to "refs/heads/master", and
locks it! This creates "refs/heads/master.lock", that is
then renamed to "refs/heads/master" when unlocked.
The behaviour of lock_file() to resolve symlink at this point in the code
comes from d58e8d3 (When locking in a symlinked repository, try to lock
the original, 2007-07-25), and as explained in the log message of that
commit, we cannot unconditionally remove it.
This patch fixes this. It teaches lock_file() not to dereference the
symbolic link when LOCK_NODEREF is given, and uses this new flag in
lock_ref_sha1_basic() when it is operating directly on HEAD (iow when
REF_NODEREF was given to it).
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
I haven't tested it beyond running the full testsuite, which it does pass,
but I can't give any more guarantee than that. Testing is for wimps ;-)
cache.h | 1 +
lockfile.c | 3 ++-
refs.c | 10 ++++++----
t/t7201-co.sh | 2 +-
4 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/cache.h b/cache.h
index 941a9dc..8ab2fd8 100644
--- a/cache.h
+++ b/cache.h
@@ -412,6 +412,7 @@ struct lock_file {
char filename[PATH_MAX];
};
#define LOCK_DIE_ON_ERROR 1
+#define LOCK_NODEREF 2
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
extern int hold_lock_file_for_append(struct lock_file *, const char *path, int);
extern int commit_lock_file(struct lock_file *);
diff --git a/lockfile.c b/lockfile.c
index bc1b585..6d75608 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -130,7 +130,8 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
* subtract 5 from size to make sure there's room for adding
* ".lock" for the lock file name
*/
- resolve_symlink(lk->filename, sizeof(lk->filename)-5);
+ if (!(flags & LOCK_NODEREF))
+ resolve_symlink(lk->filename, sizeof(lk->filename)-5);
strcat(lk->filename, ".lock");
lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
if (0 <= lk->fd) {
diff --git a/refs.c b/refs.c
index 5467e98..9e422dc 100644
--- a/refs.c
+++ b/refs.c
@@ -790,7 +790,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
struct ref_lock *lock;
struct stat st;
int last_errno = 0;
- int type;
+ int type, lflags;
int mustexist = (old_sha1 && !is_null_sha1(old_sha1));
lock = xcalloc(1, sizeof(struct ref_lock));
@@ -830,8 +830,11 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
lock->lk = xcalloc(1, sizeof(struct lock_file));
- if (flags & REF_NODEREF)
+ lflags = LOCK_DIE_ON_ERROR;
+ if (flags & REF_NODEREF) {
ref = orig_ref;
+ lflags |= LOCK_NODEREF;
+ }
lock->ref_name = xstrdup(ref);
lock->orig_ref_name = xstrdup(orig_ref);
ref_file = git_path("%s", ref);
@@ -845,9 +848,8 @@ static struct ref_lock *lock_ref_sha1_basic(const char *ref, const unsigned char
error("unable to create directory for %s", ref_file);
goto error_return;
}
- lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file,
- LOCK_DIE_ON_ERROR);
+ lock->lock_fd = hold_lock_file_for_update(lock->lk, ref_file, lflags);
return old_sha1 ? verify_lock(lock, old_sha1, mustexist) : lock;
error_return:
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 01304d7..d9a80aa 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -339,7 +339,7 @@ test_expect_success 'checkout w/--track from non-branch HEAD fails' '
test "z$(git rev-parse master^0)" = "z$(git rev-parse HEAD)"
'
-test_expect_failure 'detch a symbolic link HEAD' '
+test_expect_success 'detch a symbolic link HEAD' '
git checkout master &&
git config --bool core.prefersymlinkrefs yes &&
git checkout side &&
--
1.6.0.2.734.gae0be
^ permalink raw reply related
* [PATCH] feature request: git-mergetool --force
From: William Pursell @ 2008-10-17 23:23 UTC (permalink / raw)
To: git
I occasionally use commands like 'cp $REMOTE $MERGED' with
mergetool, and would prefer to not be prompted to start
the tool on each file. A --force option would be handy.
--
William Pursell
diff --git a/git-mergetool.sh b/git-mergetool.sh
index 94187c3..5c9ce09 100755
--- a/git-mergetool.sh
+++ b/git-mergetool.sh
@@ -8,7 +8,7 @@
# at the discretion of Junio C Hamano.
#
-USAGE='[--tool=tool] [file to merge] ...'
+USAGE='[--tool=tool] [--force] [file to merge] ...'
SUBDIRECTORY_OK=Yes
OPTIONS_SPEC=
. git-sh-setup
@@ -176,8 +176,10 @@ merge_file () {
echo "Normal merge conflict for '$MERGED':"
describe_file "$local_mode" "local" "$LOCAL"
describe_file "$remote_mode" "remote" "$REMOTE"
- printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
- read ans
+ if test x"$force_option" != xyes; then
+ printf "Hit return to start merge resolution tool (%s): " "$merge_tool"
+ read ans
+ fi
case "$merge_tool" in
kdiff3)
@@ -283,6 +285,9 @@ merge_file () {
while test $# != 0
do
case "$1" in
+ -f|--fo|--for|--forc|--force)
+ force_option=yes
+ ;;
-t|--tool*)
case "$#,$1" in
*,*=*)
^ permalink raw reply related
* Re: Excluding files from git-diff
From: Anders Melchiorsen @ 2008-10-17 23:38 UTC (permalink / raw)
To: Erik Hahn; +Cc: git
In-Reply-To: <20081017145313.GA23471@eriks>
Erik Hahn wrote:
> I'm currently working on a script whose developer does not use
> git. Hence, when I mail him the patch, I don't want to include the
> .gitignore file. Is it possible to exclude a file from git-diff (except
> not adding it to git, of course?)
>
I don't think that this is what you are asking about, but it's a neat
trick anyway :-).
To permanently exclude files from git diff, set up a custom diff driver
in ~/.gitconfig:
[diff "nodiff"]
command = /bin/true
and then mention the files that should use this driver in
.git/info/attributes:
.gitignore diff=nodiff
^ permalink raw reply
* Re: "checkout --track -b" broken? (with suggested fix)
From: Junio C Hamano @ 2008-10-17 23:57 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git, Jay Soffian
In-Reply-To: <alpine.LNX.1.00.0810171113340.19665@iabervon.org>
Daniel Barkalow <barkalow@iabervon.org> writes:
> On Thu, 16 Oct 2008, Junio C Hamano wrote:
>
>> The patch to branch.c is a quick fix for this issue. The resulting code
>> passes all the tests, but I am not very proud of hardcoding the "HEAD" in
>> the code. There must be a better way to do this.
>
> I agree with the change to the test. I think it would be better to
> hard-code "refs/heads/" instead of "HEAD", and I feel like we must have a
> "is this ref name a branch?" function, if only because someone could stick
> "refs/tags/foo" in HEAD, and we should still say it's not something you
> could track, despite it being something different from "HEAD".
But you can track things under refs/remotes/, so...
^ 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