* Re: [RFC/PATCH] Remove repo-config
From: Junio C Hamano @ 2008-01-16 20:32 UTC (permalink / raw)
To: Dan McGee; +Cc: git
In-Reply-To: <7vlk6psn5h.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Dan McGee <dpmcgee@gmail.com> writes:
>
>> On 01/15/2008 10:23 PM, Junio C Hamano wrote:
>>> I do not think it is unreasonable to add repo-config to feature
>>> removal schedule in 1.5.4 release notes. Perhaps something like...
>>>
>>> + * "git repo-config", which was an old name for "git config" command,
>>> + has been supported without being advertised for a long time. The
>>> + next feature release will remove it.
>>> +
>>
>> Seems fine to me. Does it need to be put in command-list.txt for the time being too, or what all is that file used for? Sorry I am not familiar.
>>
>> Something like:
>>
>> git-repo-config ancillarymanipulators deprecated
>
> Technically, you are right, but somehow it feels backwards.
>
> We stopped advertising the existence of the command in v1.5.0,
> and adding it to the list now would mean we need to add a new
> git-repo-config manual page that says "This is deprecated, use
> git-config instead".
This comment was wrong. We can keep the existing repo-config
documentation that has deprecation notice, and add the above
line to the list.
Sorry about the confusion.
^ permalink raw reply
* Re: Merging using only fast-forward
From: Junio C Hamano @ 2008-01-16 20:31 UTC (permalink / raw)
To: Randal L. Schwartz; +Cc: Sverre Hvammen Johansen, git
In-Reply-To: <863asxivlj.fsf@blue.stonehenge.com>
merlyn@stonehenge.com (Randal L. Schwartz) writes:
> Junio implemented a 7-line patch on the IRC channel (calling parts of it
> "for randal" or something, I believe :) to do precisely this.
>
> Perhaps you can test it, and submit it as if it were your idea. I, for one,
> would use it as well. I'm doing ugly things with parsing the output of
> git-status right now to achieve the same thing.
The mechanism itself is simple.
http://git.pastebin.com/m156a1856
A sane integration is a different story.
We have --ff and --no-ff options to merge. How should this new
option --ff-only mesh with them? Perhaps we would want to have
an option --ff that takes three values?
--ff=never
--ff=normal
--ff=only
and have the first one be synonym for existing --no-ff, the second
one to be a synonym for not giving anything (or giving --ff
explicitly), and the third one to be this new mode of operation?
^ permalink raw reply
* Re: Make 'git fsck' complain about non-commit branches
From: Linus Torvalds @ 2008-01-16 20:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vk5m9pmmq.fsf@gitster.siamese.dyndns.org>
On Wed, 16 Jan 2008, Junio C Hamano wrote:
>
> If we take that "plumbing knows much more about Porcelain
> convention" shift-of-paradigm all the way, refs/remotes/ would
> contain what are copied from refs/heads/ elsewhere, so checking
> would have been correct.
Yes. I'm taking a slightly weaker approach, which is to say that git
should check not "conventions", but "requirements".
So the reason I made it check HEAD and refs/heads/* is that yes, it's a
convention that they be branches (and thus must point to commit objects),
but it's also something deeper than that: git cit commands actually break
when it's not true.
So I think there is a difference between "convention" and "requirement".
One is how we do things, the other is how things must be done to work.
And yes, conventions have a tendency to become requirements, as people
start depending on them, so it's not a black-and-white or even a static
thing, and it changes over time.
I just suspect that at least right now, if refs/remotes/xyzzy isn't a
commit, nothing actually technically *breaks*, even if it is against the
convention.
For example, would it be wrong to have "remote tags" listed under
refs/remotes/<remotename>/tags? I don't think it necessarily is wrong.
It's not something we support right now, of course, and maybe we never
will, but I don't think it's something that is necessarily conceptually
broken - and maybe some external porcelain would want to do it that way?
So if core git doesn't really care, it shouldn't set the rules. But yes,
the rules clearly have expanded over time, and probably will continue to
do so..
Linus
^ permalink raw reply
* Re: [PATCH v2] Do not show "diff --git" metainfo with --no-prefix
From: Junio C Hamano @ 2008-01-16 20:19 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Daniel Barkalow, Chris Ortman, Johannes Schindelin, git
In-Reply-To: <alpine.LFD.1.00.0801152017490.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> That's why tying "--git" together with any prefix handling is wrong:
> because it's a totally different issue. It's true that "git-apply" right
> now doesn't understand these things, but assuming we want to teach
> git-apply to apply to subprojects eventually (we do, don't we?) we'll
> eventually have to teach it.
That's all correct but
* currently diff does not recurse, nor apply does not apply
recursively;
* "git diff" that comes with 1.5.4, if we do not do anything,
can produce a diff that will be rejected by the stricter
check "git apply" has when used with --no-prefix and friends;
* submodule aware versions of "git diff" can be told to add
"--mark-as-git-diff" when it passes "--src-prefix=a/git-gui"
and "--dst-prefix=b/git-gui" when it recurses internally, to
defeat what my proposed patch does.
So I think it makes more sense to mark output as a non-git diff
when custom prefix is used in the version we are going to ship
as part of 1.5.4.
^ permalink raw reply
* Re: [RFC/PATCH] Remove repo-config
From: Junio C Hamano @ 2008-01-16 20:13 UTC (permalink / raw)
To: Dan McGee; +Cc: git
In-Reply-To: <1200453554-14163-1-git-send-email-dpmcgee@gmail.com>
Dan McGee <dpmcgee@gmail.com> writes:
> 'git config' has been used in place of 'git repo-config' for some time in
> the documentation and most of the tools, so remove traces of repo-config
> from the source.
>
> Signed-off-by: Dan McGee <dpmcgee@gmail.com>
I'd agree with the deprecation. We stopped advertising it long
time ago (1.5.0 I think).
> diff --git a/Documentation/git-repo-config.txt b/Documentation/git-repo-config.txt
Let's defer the removal til post 1.5.4.
> diff --git a/Makefile b/Makefile
Likewise.
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
Likewise.
> diff --git a/contrib/examples/git-tag.sh b/contrib/examples/git-tag.sh
> index ae7c531..a3182df 100755
> --- a/contrib/examples/git-tag.sh
> +++ b/contrib/examples/git-tag.sh
> @@ -167,6 +167,7 @@ type=$(git cat-file -t $object) || exit 1
> tagger=$(git-var GIT_COMMITTER_IDENT) || exit 1
>
> test -n "$username" ||
> + #NOTE: 'git repo-config' has since been replaced by 'git config'
> username=$(git repo-config user.signingkey) ||
> username=$(expr "z$tagger" : 'z\(.*>\)')
Good.
> diff --git a/git.c b/git.c
Deferred.
> diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
> diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh
Doing s/repo-config/config/ in test scripts is good.
> diff --git a/templates/hooks--update b/templates/hooks--update
> index bd93dd1..09a99ff 100644
> --- a/templates/hooks--update
> +++ b/templates/hooks--update
> @@ -37,9 +37,9 @@ if [ -z "$refname" -o -z "$oldrev" -o -z "$newrev" ]; then
> fi
>
> # --- Config
> -allowunannotated=$(git-repo-config --bool hooks.allowunannotated)
> -allowdeletebranch=$(git-repo-config --bool hooks.allowdeletebranch)
> -allowdeletetag=$(git-repo-config --bool hooks.allowdeletetag)
> +allowunannotated=$(git config --bool hooks.allowunannotated)
> +allowdeletebranch=$(git config --bool hooks.allowdeletebranch)
> +allowdeletetag=$(git config --bool hooks.allowdeletetag)
>
> # check for no description
> projectdesc=$(sed -e '1q' "$GIT_DIR/description")
Good.
> @@ -53,7 +53,7 @@ fi
> if [ "$newrev" = "0000000000000000000000000000000000000000" ]; then
> newrev_type=delete
> else
> - newrev_type=$(git-cat-file -t $newrev)
> + newrev_type=$(git cat-file -t $newrev)
> fi
>
> case "$refname","$newrev_type" in
Good but does not belong to the topic.
^ permalink raw reply
* Re: [PATCH 2/2] close_lock_file(): new function in the lockfile API
From: Linus Torvalds @ 2008-01-16 20:08 UTC (permalink / raw)
To: Junio C Hamano
Cc: Brandon Casey, Brandon Casey, Git Mailing List, Alex Riesen,
Kristian Høgsberg
In-Reply-To: <7vejchr3pf.fsf_-_@gitster.siamese.dyndns.org>
On Wed, 16 Jan 2008, Junio C Hamano wrote:
> +
> +void close_lock_file(struct lock_file *lk)
> +{
> + close(lk->fd);
> + lk->fd = -1;
> +}
Since one of the main purposes of closing would be the error testing of
writes that haven't made it out yet on filesystems like NFS that do
open-close cache serialization, I'd suggest doing this as
int close_lock_file(struct lock_file *lk)
{
int fd = lk->fd;
lk->df = -1;
return close(fd);
}
to give the return code.
Linus
^ permalink raw reply
* Re: [PATCH 1/3] git-submodule: rename shell functions for consistency
From: Junio C Hamano @ 2008-01-16 20:08 UTC (permalink / raw)
To: Imran M Yousuf; +Cc: Junio C Hamano, git
In-Reply-To: <7bfdc29a0801151826u2218f825ga8100b1cc9fa8b2@mail.gmail.com>
"Imran M Yousuf" <imyousuf@gmail.com> writes:
> Thanks Junio for showing how it should be done. Due to some
> pre-scheduled appointment I was unavailable yesterday evening and thus
> was neither able to reply nor resubmit the changes.
Well, I did not show how it _should_ be done. That series was
merely an illustration of how I _think_ it should look like. I
did not test it, I do not know if it introduced new bugs, and
most importantly I do not know if it fulfills what you intended
to achieve with your patch.
In other words, I just tried to turn the table around. Instead
of me and others commenting on your patch saying "I do not like
this" piecemeal, now you have something you can comment on. You
can say the whole range of things from "I tested this and it is
what I want", "I like the general concept but I found this and
that bug and here is a fix", to "This is much worse than what I
proposed and here is why."
^ permalink raw reply
* Re: Make 'git fsck' complain about non-commit branches
From: Junio C Hamano @ 2008-01-16 19:59 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801151654050.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, 15 Jan 2008, Junio C Hamano wrote:
>>
>> So far, the plumbing level did not care much about the Porcelain
>> convention, such as refs/heads and refs/remotes (you seem to
>> have forgot) are about "branches" and must point at commit
>> objects.
>
> Yeah. I'm not sure this is all a great idea, but I think they are correct
> (and no, "refs/remotes/" would *not* have been correct).
If we take that "plumbing knows much more about Porcelain
convention" shift-of-paradigm all the way, refs/remotes/ would
contain what are copied from refs/heads/ elsewhere, so checking
would have been correct. If you are saying that we are not
prepared to take the change that far (which I tend to agree
with, as I like to keep the door open for people to do things
that at the first sight seem insane but later turns out to be
useful in workflows we haven't imagined so far), I'd agree that
not insisting on commitness under refs/remotes/ is correct.
Is that where your "refs/remotes would *not* have been correct"
comes from, or did I miss something more fundamental?
^ permalink raw reply
* Re: Be more careful about updating refs
From: Junio C Hamano @ 2008-01-16 19:52 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Git Mailing List
In-Reply-To: <alpine.LFD.1.00.0801151600140.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, 15 Jan 2008, Linus Torvalds wrote:
>>
>> This makes write_ref_sha1() more careful: it actually checks the SHA1 of
>> the ref it is updating, and refuses to update a ref with an object that it
>> cannot find.
>
> Side note: this breaks some tests, because those tests do things like
>
> git update-ref refs/heads/master 1111111111111111111111111111111111111111 &&
> test 1111111111111111111111111111111111111111 = $(cat .git/refs/heads/master)
>
> ...
> (Pet peeve on mine: people fixing assert()'s by changing the source-code,
> without ever asking themselves whether maybe the assert itself was the
> bug).
The rules for the plumbing used to be that refs can point at
anything that get_sha1() accepts. We did not even require it to
be parse_object() happy let alone it being parse_commit() kosher.
You changed the world order. I agree that the world order was
changed in a good way, but saying that the original test did not
check the right thing or it was a bug is not quite fair. At
worst, we can say that it was very sloppily written by assuming
that the commands involved in the particular test would not care
about corrupted repositories whose refs point at nonexistant
bogus objects.
I'll squash the following to your patch.
---
t/t1400-update-ref.sh | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index a90824b..71ab2dd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -7,12 +7,19 @@ test_description='Test git update-ref and basic ref logging'
. ./test-lib.sh
Z=0000000000000000000000000000000000000000
-A=1111111111111111111111111111111111111111
-B=2222222222222222222222222222222222222222
-C=3333333333333333333333333333333333333333
-D=4444444444444444444444444444444444444444
-E=5555555555555555555555555555555555555555
-F=6666666666666666666666666666666666666666
+
+test_expect_success setup '
+
+ for name in A B C D E F
+ do
+ test_tick &&
+ T=$(git write-tree) &&
+ sha1=$(echo $name | git commit-tree $T) &&
+ eval $name=$sha1
+ done
+
+'
+
m=refs/heads/master
n_dir=refs/heads/gu
n=$n_dir/fixes
^ permalink raw reply related
* Re: [PATCH] Improve use of lockfile API
From: Brandon Casey @ 2008-01-16 19:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7v4pddr2jy.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Ok, our mails crossed. I'll scrap part of my [2/2] and queue
> this series perhaps to 'next' after review.
Yeah, I received yours while I was readying mine.
Also, I forgot to add signed-off. So please add:
Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
when you commit, or I can resend if you like.
-brandon
^ permalink raw reply
* Re: [ANNOUNCE] Push Me Pull You 0.2 - Tech Preview Release
From: Junio C Hamano @ 2008-01-16 19:35 UTC (permalink / raw)
To: Mark Williamson; +Cc: git
In-Reply-To: <200801152131.33628.mark.williamson@cl.cam.ac.uk>
Mark Williamson <mark.williamson@cl.cam.ac.uk> writes:
> I'd like to announce a new release of the Push Me Pull You (pmpu) tool; a GUI
> for distributed revision control systems.
>
> PMPU supports plain hg, hg forest repositories, bzr, git and darcs as
> underlying repositories. It aims to provide a powerful graphical interface
> to the underlying functionality, based around the workflow of incoming and
> outgoing changesets.
I haven't tried to look at this since your 0.1 announcement
(which unfortunately was accepted with a thundering silence
here), but it would be interesting if it allowed to pull from Hg
into git (or other combinations). Is that one of the features
(or planned features)?
^ permalink raw reply
* Re: [PATCH] Improve use of lockfile API
From: Junio C Hamano @ 2008-01-16 19:30 UTC (permalink / raw)
To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <478E572E.3020505@nrlssc.navy.mil>
Ok, our mails crossed. I'll scrap part of my [2/2] and queue
this series perhaps to 'next' after review.
^ permalink raw reply
* Re: I don't want the .git directory next to my code.
From: Junio C Hamano @ 2008-01-16 19:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Mike, Neil Macneale, git
In-Reply-To: <alpine.LFD.1.00.0801161000310.2806@woody.linux-foundation.org>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Let me do a few examples of why this is a good idea:
>
> - the whole point of development trees and SCM's (and that's *especially*
> true with git) is how you can try things out, go backwards in time, and
> generally just do *development*.
>
> If you do that in what is your public deployment area, you're already
> very limited. Not only may you not want to make that .git directory
> accessible to others (while you *do* obviously want to make the
> deployment itself), you also end up exposing things like your
> management scripts and source code along with "generated files" etc
> that are the things you actually want to deploy.
>
> Yes, it's certainly quite possible that you simply don't have any
> management scripts etc, and that you don't generate any files, and you
> simply want to just deploy the exact files that you also want to track.
Even without any management script, you cannot do any _development_
in such a tree. By definition, mucking with the files in the
deployment area means all of your changes are immediately visible by
the clients.
One reason (admittedly misguided) people mentioned why they want
to do this is because they want to be able to "git-add" files
generated on the deployed server by client actions (think of a
Wiki that drops new contents in an area writable by the
webserver). If your deployment area is _not_ managed by git,
they instead need to write a Makefile target in their
development area that takes new/modified files back to the
development tree and "git-add" those copies.
The right way to manage these client-generated contents would of
course be to commit them to a branch separate from the sources
you develop (otherwise your history will be a mixed mess between
the true development and client content changes), so the
argument is very weak and is not a good justification for
wanting to have deployment and development tree as one.
But I can well imagine that it is a tempting way to work for
people who have not thought about the reason why history
matters, especially for the ones who still suffer from CVS
braindamage that makes separate branches inpractical.
^ permalink raw reply
* Re: Git Cygwin - unable to create any repository - help!
From: Alex Riesen @ 2008-01-16 19:17 UTC (permalink / raw)
To: Paul Umbers; +Cc: Robin Rosenberg, git
In-Reply-To: <a5eb9c330801161048x4b5a88dcsebd7cf9754f72ba6@mail.gmail.com>
Paul Umbers, Wed, Jan 16, 2008 19:48:28 +0100:
> Latest output, log file attached:
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ uname -a > somefile
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ git init
> Initialized empty Git repository in .git/
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) add somefile
>
Could you please retry with "-m all"? "-m syscall" is not what one
might expect, its some tracing of cygwin and not enough at that.
^ permalink raw reply
* [PATCH] lockfile.c: modify handling of rename failures and alternate_index_output
From: Brandon Casey @ 2008-01-16 19:13 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
In-Reply-To: <1200510486-11438-1-git-send-email-casey@nrlssc.navy.mil>
commit_lock_file(): if rename() fails, we should not
truncate the name of the lockfile. This file should be deleted
by rollback_lock_file() or remove_lock_file().
commit_locked_index(): call close_lock_file() when alternate_index_output
is set and handle rename() failures like commit_lock_file().
---
lockfile.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/lockfile.c b/lockfile.c
index bcc4786..e8e5dbb 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -171,15 +171,16 @@ int close_lock_file(struct lock_file *lk)
int commit_lock_file(struct lock_file *lk)
{
char result_file[PATH_MAX];
- int i;
+ size_t i;
if (lk->fd >= 0 && close_lock_file(lk))
return -1;
strcpy(result_file, lk->filename);
i = strlen(result_file) - 5; /* .lock */
result_file[i] = 0;
- i = rename(lk->filename, result_file);
+ if (rename(lk->filename, result_file))
+ return -1;
lk->filename[0] = 0;
- return i;
+ return 0;
}
int hold_locked_index(struct lock_file *lk, int die_on_error)
@@ -195,9 +196,12 @@ void set_alternate_index_output(const char *name)
int commit_locked_index(struct lock_file *lk)
{
if (alternate_index_output) {
- int result = rename(lk->filename, alternate_index_output);
+ if (lk->fd >= 0 && close_lock_file(lk))
+ return -1;
+ if (rename(lk->filename, alternate_index_output))
+ return -1;
lk->filename[0] = 0;
- return result;
+ return 0;
}
else
return commit_lock_file(lk);
--
1.5.4.rc3.14.g44397-dirty
^ permalink raw reply related
* [PATCH] refs.c: rework ref_locks by abstracting from underlying struct lock_file
From: Brandon Casey @ 2008-01-16 19:14 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
In-Reply-To: <1200510486-11438-2-git-send-email-casey@nrlssc.navy.mil>
Instead of calling close_lock_file() and commit_lock_file() directly,
which take a struct lock_file argument, add two new functions:
close_ref() and commit_ref(), which handle calling the previous
lock_file functions and modifying the ref_lock structure.
---
refs.c | 21 ++++++++++++++++++---
1 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/refs.c b/refs.c
index 2c78956..e705ed3 100644
--- a/refs.c
+++ b/refs.c
@@ -1018,6 +1018,22 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
return 1;
}
+static int close_ref(struct ref_lock *lock)
+{
+ if (close_lock_file(lock->lk))
+ return -1;
+ lock->lock_fd = -1;
+ return 0;
+}
+
+static int commit_ref(struct ref_lock *lock)
+{
+ if (commit_lock_file(lock->lk))
+ return -1;
+ lock->lock_fd = -1;
+ return 0;
+}
+
void unlock_ref(struct ref_lock *lock)
{
/* Do not free lock->lk -- atexit() still looks at them */
@@ -1128,7 +1144,7 @@ int write_ref_sha1(struct ref_lock *lock,
}
if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock->lock_fd, &term, 1) != 1
- || close_lock_file(lock->lk) < 0) {
+ || close_ref(lock) < 0) {
error("Couldn't write %s", lock->lk->filename);
unlock_ref(lock);
return -1;
@@ -1161,12 +1177,11 @@ int write_ref_sha1(struct ref_lock *lock,
!strcmp(head_ref, lock->ref_name))
log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
}
- if (commit_lock_file(lock->lk)) {
+ if (commit_ref(lock)) {
error("Couldn't set %s", lock->ref_name);
unlock_ref(lock);
return -1;
}
- lock->lock_fd = -1;
unlock_ref(lock);
return 0;
}
--
1.5.4.rc3.14.g44397-dirty
^ permalink raw reply related
* [PATCH] Improve use of lockfile API
From: Brandon Casey @ 2008-01-16 19:12 UTC (permalink / raw)
To: Git Mailing List, Junio C Hamano
-Remove remaining double close(2)'s.
i.e. close() before commit_locked_index() or commit_lock_file()
-New close_lock_file() function which will close() the lock descriptor and
assign -1 to it, but will not remove or rename it.
-commit_lock_file() was modified to detect and return -1 on failure
to close the lock file.
---
There are two places where we fdopen() the lock file descriptor.
In builtin-pack-refs.c and fast-import.c.
Presently, I assigned -1 to the lock->fd and added a comment.
Additionally in bundle.c we must assign -1 to the lock->fd
since it is closed by start_command().
We could do something along the lines of:
FILE* fdopen_lock_file(struct lock_file *lk)
{
FILE* f;
if ((f = fdopen(lk->fd)) == NULL)
return NULL;
lk->fd = -1;
return f;
}
But that may be overkill.
I'll think we'll probably catch this case since we are now
checking that the close() succeeds in commit_lock_file().
-brandon
builtin-add.c | 2 +-
builtin-apply.c | 2 +-
builtin-checkout-index.c | 4 ++--
builtin-commit.c | 9 ++++++---
builtin-diff.c | 2 +-
builtin-fetch-pack.c | 1 -
builtin-mv.c | 1 -
builtin-pack-refs.c | 4 ++++
builtin-read-tree.c | 2 +-
builtin-rerere.c | 4 ++--
builtin-reset.c | 1 -
builtin-revert.c | 4 ++--
builtin-rm.c | 2 +-
builtin-update-index.c | 2 +-
builtin-write-tree.c | 9 +++------
bundle.c | 12 ++++++++----
cache.h | 1 +
config.c | 8 ++------
fast-import.c | 10 ++++++++--
lockfile.c | 17 ++++++++++++++---
merge-recursive.c | 2 +-
refs.c | 12 ++++--------
22 files changed, 63 insertions(+), 48 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 5c29cc2..4a91e3e 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -259,7 +259,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
finish:
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(&lock_file))
+ commit_locked_index(&lock_file))
die("Unable to write new index file");
}
diff --git a/builtin-apply.c b/builtin-apply.c
index d57bb6e..15432b6 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2921,7 +2921,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
if (update_index) {
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(&lock_file))
+ commit_locked_index(&lock_file))
die("Unable to write new index file");
}
diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 70d619d..7e42024 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -246,8 +246,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
* want to update cache.
*/
if (state.refresh_cache) {
- close(newfd); newfd = -1;
rollback_lock_file(&lock_file);
+ newfd = -1;
}
state.refresh_cache = 0;
}
@@ -297,7 +297,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
if (0 <= newfd &&
(write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(&lock_file)))
+ commit_locked_index(&lock_file)))
die("Unable to write new index file");
return 0;
}
diff --git a/builtin-commit.c b/builtin-commit.c
index 16345e9..a764053 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -237,7 +237,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
int fd = hold_locked_index(&index_lock, 1);
add_files_to_cache(0, also ? prefix : NULL, pathspec);
refresh_cache(REFRESH_QUIET);
- if (write_cache(fd, active_cache, active_nr))
+ if (write_cache(fd, active_cache, active_nr) ||
+ close_lock_file(&index_lock))
die("unable to write new_index file");
commit_style = COMMIT_NORMAL;
return index_lock.filename;
@@ -298,7 +299,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
fd = hold_locked_index(&index_lock, 1);
add_remove_files(&partial);
refresh_cache(REFRESH_QUIET);
- if (write_cache(fd, active_cache, active_nr))
+ if (write_cache(fd, active_cache, active_nr) ||
+ close_lock_file(&index_lock))
die("unable to write new_index file");
fd = hold_lock_file_for_update(&false_lock,
@@ -308,7 +310,8 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)
add_remove_files(&partial);
refresh_cache(REFRESH_QUIET);
- if (write_cache(fd, active_cache, active_nr))
+ if (write_cache(fd, active_cache, active_nr) ||
+ close_lock_file(&false_lock))
die("unable to write temporary index file");
return false_lock.filename;
}
diff --git a/builtin-diff.c b/builtin-diff.c
index 29365a0..8d7a569 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -190,7 +190,7 @@ static void refresh_index_quietly(void)
refresh_cache(REFRESH_QUIET|REFRESH_UNMERGED);
if (active_cache_changed &&
- !write_cache(fd, active_cache, active_nr) && !close(fd))
+ !write_cache(fd, active_cache, active_nr))
commit_locked_index(lock_file);
rollback_lock_file(lock_file);
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 807fa93..e68e015 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -783,7 +783,6 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
unlink(shallow);
rollback_lock_file(&lock);
} else {
- close(fd);
commit_lock_file(&lock);
}
}
diff --git a/builtin-mv.c b/builtin-mv.c
index a3f9ad1..990e213 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -264,7 +264,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) ||
commit_locked_index(&lock_file))
die("Unable to write new index file");
}
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 1923fb1..fbe451f 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -108,6 +108,10 @@ static int pack_refs(unsigned int flags)
die("failed to write ref-pack file");
if (fflush(cbdata.refs_file) || fsync(fd) || fclose(cbdata.refs_file))
die("failed to write ref-pack file (%s)", strerror(errno));
+ /* Since the lock file was fdopen()'ed and then fclose()'ed above,
+ * assign -1 to the lock file descriptor so that commit_lock_file()
+ * won't try to close() it. */
+ packed.fd = -1;
if (commit_lock_file(&packed) < 0)
die("unable to overwrite old ref-pack file (%s)", strerror(errno));
if (cbdata.flags & PACK_REFS_PRUNE)
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 43cd56a..c0ea034 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -283,7 +283,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
}
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(&lock_file))
+ commit_locked_index(&lock_file))
die("unable to write new index file");
return 0;
}
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 37e6248..a9e3ebc 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -61,9 +61,9 @@ static int write_rr(struct path_list *rr, int out_fd)
write_in_full(out_fd, path, length) != length)
die("unable to write rerere record");
}
- if (close(out_fd) != 0)
+ if (commit_lock_file(&write_lock) != 0)
die("unable to write rerere record");
- return commit_lock_file(&write_lock);
+ return 0;
}
static int handle_file(const char *path,
diff --git a/builtin-reset.c b/builtin-reset.c
index 10dba60..7ee811f 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -108,7 +108,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock)
return error("Could not read index");
result = refresh_cache(0) ? 1 : 0;
if (write_cache(fd, active_cache, active_nr) ||
- close(fd) ||
commit_locked_index(index_lock))
return error ("Could not refresh index");
return result;
diff --git a/builtin-revert.c b/builtin-revert.c
index 4bf8eb2..358af53 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -371,13 +371,13 @@ static int revert_or_cherry_pick(int argc, const char **argv)
i++;
}
}
- if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
+ if (commit_lock_file(&msg_file) < 0)
die ("Error wrapping up %s", defmsg);
fprintf(stderr, "Automatic %s failed.%s\n",
me, help_msg(commit->object.sha1));
exit(1);
}
- if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
+ if (commit_lock_file(&msg_file) < 0)
die ("Error wrapping up %s", defmsg);
fprintf(stderr, "Finished one %s.\n", me);
diff --git a/builtin-rm.c b/builtin-rm.c
index a3d25e6..c0a8bb6 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -250,7 +250,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
if (active_cache_changed) {
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(&lock_file))
+ commit_locked_index(&lock_file))
die("Unable to write new index file");
}
diff --git a/builtin-update-index.c b/builtin-update-index.c
index e1a938d..c3a14c7 100644
--- a/builtin-update-index.c
+++ b/builtin-update-index.c
@@ -738,7 +738,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
get_index_file(), strerror(lock_error));
}
if (write_cache(newfd, active_cache, active_nr) ||
- close(newfd) || commit_locked_index(lock_file))
+ commit_locked_index(lock_file))
die("Unable to write new index file");
}
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index b89d02e..d16b9ed 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -35,11 +35,9 @@ int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
missing_ok, 0) < 0)
die("git-write-tree: error building trees");
if (0 <= newfd) {
- if (!write_cache(newfd, active_cache, active_nr)
- && !close(newfd)) {
- commit_lock_file(lock_file);
+ if (!write_cache(newfd, active_cache, active_nr) &&
+ !commit_lock_file(lock_file))
newfd = -1;
- }
}
/* Not being able to write is fine -- we are only interested
* in updating the cache-tree part, and if the next caller
@@ -60,8 +58,7 @@ int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
hashcpy(sha1, active_cache_tree->sha1);
if (0 <= newfd)
- close(newfd);
- rollback_lock_file(lock_file);
+ rollback_lock_file(lock_file);
return 0;
}
diff --git a/bundle.c b/bundle.c
index 316aa74..61f388a 100644
--- a/bundle.c
+++ b/bundle.c
@@ -317,6 +317,12 @@ int create_bundle(struct bundle_header *header, const char *path,
rls.git_cmd = 1;
if (start_command(&rls))
return error("Could not spawn pack-objects");
+
+ /* start_command closed bundle_fd if it was > 1
+ * so set the lock fd to -1 so commit_lock_file()
+ * won't fail trying to close it */
+ lock.fd = -1;
+
for (i = 0; i < revs.pending.nr; i++) {
struct object *object = revs.pending.objects[i].item;
if (object->flags & UNINTERESTING)
@@ -326,10 +332,8 @@ int create_bundle(struct bundle_header *header, const char *path,
}
if (finish_command(&rls))
return error ("pack-objects died");
- close(bundle_fd);
- if (!bundle_to_stdout)
- commit_lock_file(&lock);
- return 0;
+
+ return bundle_to_stdout ? close(bundle_fd) : commit_lock_file(&lock);
}
int unbundle(struct bundle_header *header, int bundle_fd)
diff --git a/cache.h b/cache.h
index 39331c2..4f4495f 100644
--- a/cache.h
+++ b/cache.h
@@ -303,6 +303,7 @@ struct lock_file {
char filename[PATH_MAX];
};
extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
+extern int close_lock_file(struct lock_file *);
extern int commit_lock_file(struct lock_file *);
extern int hold_locked_index(struct lock_file *, int);
diff --git a/config.c b/config.c
index 857deb6..526a3f4 100644
--- a/config.c
+++ b/config.c
@@ -955,14 +955,12 @@ int git_config_set_multivar(const char* key, const char* value,
munmap(contents, contents_sz);
}
- if (close(fd) || commit_lock_file(lock) < 0) {
+ if (commit_lock_file(lock) < 0) {
fprintf(stderr, "Cannot commit config file!\n");
ret = 4;
goto out_free;
}
- /* fd is closed, so don't try to close it below. */
- fd = -1;
/*
* lock is committed, so don't try to roll it back below.
* NOTE: Since lockfile.c keeps a linked list of all created
@@ -973,8 +971,6 @@ int git_config_set_multivar(const char* key, const char* value,
ret = 0;
out_free:
- if (0 <= fd)
- close(fd);
if (lock)
rollback_lock_file(lock);
free(config_filename);
@@ -1072,7 +1068,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
}
fclose(config_file);
unlock_and_out:
- if (close(out_fd) || commit_lock_file(lock) < 0)
+ if (commit_lock_file(lock) < 0)
ret = error("Cannot commit config file!");
out:
free(config_filename);
diff --git a/fast-import.c b/fast-import.c
index 82e9161..1ce631c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1546,10 +1546,16 @@ static void dump_marks(void)
}
dump_marks_helper(f, 0, marks);
- fclose(f);
- if (commit_lock_file(&mark_lock))
+ if (ferror(f) || fclose(f))
failure |= error("Unable to write marks file %s: %s",
mark_file, strerror(errno));
+ /* Since the lock file was fdopen()'ed and then fclose()'ed above,
+ * assign -1 to the lock file descriptor so that commit_lock_file()
+ * won't try to close() it. */
+ mark_lock.fd = -1;
+ if (commit_lock_file(&mark_lock))
+ failure |= error("Unable to write commit file %s: %s",
+ mark_file, strerror(errno));
}
static int read_next_command(void)
diff --git a/lockfile.c b/lockfile.c
index f45d3ed..bcc4786 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -13,7 +13,8 @@ static void remove_lock_file(void)
while (lock_file_list) {
if (lock_file_list->owner == me &&
lock_file_list->filename[0]) {
- close(lock_file_list->fd);
+ if (lock_file_list->fd >= 0)
+ close(lock_file_list->fd);
unlink(lock_file_list->filename);
}
lock_file_list = lock_file_list->next;
@@ -159,11 +160,20 @@ int hold_lock_file_for_update(struct lock_file *lk, const char *path, int die_on
return fd;
}
+int close_lock_file(struct lock_file *lk)
+{
+ if (close(lk->fd))
+ return -1;
+ lk->fd = -1;
+ return 0;
+}
+
int commit_lock_file(struct lock_file *lk)
{
char result_file[PATH_MAX];
int i;
- close(lk->fd);
+ if (lk->fd >= 0 && close_lock_file(lk))
+ return -1;
strcpy(result_file, lk->filename);
i = strlen(result_file) - 5; /* .lock */
result_file[i] = 0;
@@ -196,7 +206,8 @@ int commit_locked_index(struct lock_file *lk)
void rollback_lock_file(struct lock_file *lk)
{
if (lk->filename[0]) {
- close(lk->fd);
+ if (lk->fd >= 0)
+ close(lk->fd);
unlink(lk->filename);
}
lk->filename[0] = 0;
diff --git a/merge-recursive.c b/merge-recursive.c
index b34177d..c292a77 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1753,7 +1753,7 @@ int main(int argc, char *argv[])
if (active_cache_changed &&
(write_cache(index_fd, active_cache, active_nr) ||
- close(index_fd) || commit_locked_index(lock)))
+ commit_locked_index(lock)))
die ("unable to write %s", get_index_file());
return clean ? 0: 1;
diff --git a/refs.c b/refs.c
index 58f6d17..2c78956 100644
--- a/refs.c
+++ b/refs.c
@@ -864,7 +864,6 @@ static int repack_without_ref(const char *refname)
die("too long a refname '%s'", list->name);
write_or_die(fd, line, len);
}
- close(fd);
return commit_lock_file(&packlock);
}
@@ -1021,12 +1020,9 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
void unlock_ref(struct ref_lock *lock)
{
- if (lock->lock_fd >= 0) {
- close(lock->lock_fd);
- /* Do not free lock->lk -- atexit() still looks at them */
- if (lock->lk)
- rollback_lock_file(lock->lk);
- }
+ /* Do not free lock->lk -- atexit() still looks at them */
+ if (lock->lk)
+ rollback_lock_file(lock->lk);
free(lock->ref_name);
free(lock->orig_ref_name);
free(lock);
@@ -1132,7 +1128,7 @@ int write_ref_sha1(struct ref_lock *lock,
}
if (write_in_full(lock->lock_fd, sha1_to_hex(sha1), 40) != 40 ||
write_in_full(lock->lock_fd, &term, 1) != 1
- || close(lock->lock_fd) < 0) {
+ || close_lock_file(lock->lk) < 0) {
error("Couldn't write %s", lock->lk->filename);
unlock_ref(lock);
return -1;
--
1.5.4.rc3.14.g44397-dirty
^ permalink raw reply related
* Re: Git Cygwin - unable to create any repository - help!
From: Alex Riesen @ 2008-01-16 19:12 UTC (permalink / raw)
To: Paul Umbers; +Cc: Robin Rosenberg, git
In-Reply-To: <a5eb9c330801161048x4b5a88dcsebd7cf9754f72ba6@mail.gmail.com>
Paul Umbers, Wed, Jan 16, 2008 19:48:28 +0100:
> Latest output, log file attached:
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ uname -a > somefile
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ git init
> Initialized empty Git repository in .git/
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) add somefile
>
> paulumbers@Devteam29 ~/workspace/git/git-1.5.3
> $ git ls-files -s somefile
> 100644 2ed63d326ffdb2fd4b703780f4d61f1893cac63b 0 somefile
>
> Checking .git/objects/2e ... there is no sha1 file
>
Hmm... Could you try to create a repo so that the whole path contains
no whitespace? Something like "c:/tmp/mydir". Just a suggestion, I am
still inspecting your log...
^ permalink raw reply
* Re: [PATCH] Documentation: fix and clarify grammar in git-merge docs.
From: Junio C Hamano @ 2008-01-16 19:07 UTC (permalink / raw)
To: dave; +Cc: git
In-Reply-To: <1200460565-16797-1-git-send-email-dave@krondo.com>
dave@krondo.com writes:
> From: Dave Peticolas <dave@krondo.com>
>
>
> Signed-off-by: Dave Peticolas <dave@krondo.com>
> ---
> Documentation/git-merge.txt | 25 ++++++++++++-------------
> 1 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index ed3a924..abf63fe 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -74,19 +74,18 @@ it happens. In other words, `git-diff --cached HEAD` must
> report no changes.
>
> [NOTE]
> -This is a bit of lie. In certain special cases, your index are
> -allowed to be different from the tree of `HEAD` commit. The most
> -notable case is when your `HEAD` commit is already ahead of what
> -is being merged, in which case your index can have arbitrary
> -difference from your `HEAD` commit. Otherwise, your index entries
> -are allowed have differences from your `HEAD` commit that match
> -the result of trivial merge (e.g. you received the same patch
> -from external source to produce the same result as what you are
> -merging). For example, if a path did not exist in the common
> -ancestor and your head commit but exists in the tree you are
> -merging into your repository, and if you already happen to have
> -that path exactly in your index, the merge does not have to
> -fail.
> +This is a bit of a lie. In certain special cases, your index is
> +allowed to be different from the tree of the `HEAD` commit. The
> +most notable case is when your `HEAD` commit is already ahead of
> +what is being merged, in which case your index can have arbitrary
> +differences from your `HEAD` commit. Also, your index entries may
> +have differences from your `HEAD` commit that match the result of
> +a trivial merge (e.g., you received the same patch from an external
> +source to produce the same result as what you are merging). For example,
> +if a path did not exist in the common ancestor and your head commit
> +but exists in the tree you are merging into your repository, and if
> +you already happen to have that path exactly in your index, the merge
> +does not have to fail.
While checking and fixing the grammatical errors is very much
appreciated, line re-wrapping is not. It makes it unnecessarily
cumbersome to see what got really changed.
Thanks anyway. The changes look good.
^ permalink raw reply
* [PATCH 2/2] close_lock_file(): new function in the lockfile API
From: Junio C Hamano @ 2008-01-16 19:05 UTC (permalink / raw)
To: Brandon Casey
Cc: Linus Torvalds, Brandon Casey, Git Mailing List, Alex Riesen,
Kristian Høgsberg
In-Reply-To: <Pine.LNX.4.44.0801152006260.944-100000@demand>
The lockfile API is a handy way to obtain a file that is cleaned
up if you die(). But sometimes you would need this sequence to
work:
1. hold_lock_file_for_update() to get a file descriptor for
writing;
2. write the contents out, without being able to decide if the
results should be committed or rolled back;
3. do something else that makes the decision --- and this
"something else" needs the lockfile not to have an open file
descriptor for writing (e.g. Windows do not want a open file
to be renamed);
4. call commit_lock_file() or rollback_lock_file() as
appropriately.
This adds close_lock_file() you can call between step 2 and 3 in
the above sequence.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-lockfile.txt | 11 +++++++++--
cache.h | 2 +-
lockfile.c | 6 ++++++
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index 5b1553e..def5f2a 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -45,6 +45,11 @@ rollback_lock_file::
with an earlier call to `hold_lock_file_for_update()`,
close the file descriptor and remove the lockfile.
+close_lock_file::
+ Take a pointer to the `struct lock_file` initialized
+ with an earlier call to `hold_lock_file_for_update()`,
+ and close the file descriptor.
+
Because the structure is used in an `atexit(3)` handler, its
storage has to stay throughout the life of the program. It
cannot be an auto variable allocated on the stack.
@@ -54,8 +59,10 @@ done writing to the file descriptor. If you do not call either
and simply `exit(3)` from the program, an `atexit(3)` handler
will close and remove the lockfile.
-You should not close the file descriptor you obtained from
-`hold_lock_file_for_update` function yourself. The `struct
+If you need to close the file descriptor you obtained from
+`hold_lock_file_for_update` function yourself, do so by calling
+`close_lock_file()`. You should never call `close(2)` yourself!
+Otherwise the `struct
lock_file` structure still remembers that the file descriptor
needs to be closed, and a later call to `commit_lock_file()` or
`rollback_lock_file()` will result in duplicate calls to
diff --git a/cache.h b/cache.h
index 39331c2..5033b34 100644
--- a/cache.h
+++ b/cache.h
@@ -308,7 +308,7 @@ extern int commit_lock_file(struct lock_file *);
extern int hold_locked_index(struct lock_file *, int);
extern int commit_locked_index(struct lock_file *);
extern void set_alternate_index_output(const char *);
-
+extern void close_lock_file(struct lock_file *);
extern void rollback_lock_file(struct lock_file *);
extern int delete_ref(const char *, const unsigned char *sha1);
diff --git a/lockfile.c b/lockfile.c
index f45d3ed..e57d850 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -201,3 +201,9 @@ void rollback_lock_file(struct lock_file *lk)
}
lk->filename[0] = 0;
}
+
+void close_lock_file(struct lock_file *lk)
+{
+ close(lk->fd);
+ lk->fd = -1;
+}
--
1.5.4.rc3.14.g44397
^ permalink raw reply related
* [PATCH 1/2] Document lockfile API
From: Junio C Hamano @ 2008-01-16 19:00 UTC (permalink / raw)
To: Brandon Casey
Cc: Linus Torvalds, Brandon Casey, Git Mailing List, Alex Riesen,
Kristian Høgsberg
In-Reply-To: <Pine.LNX.4.44.0801152006260.944-100000@demand>
We have nice set of placeholders, but nobody stepped in to fill
the gap in the API documentation, so I am doing it myself.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/technical/api-lockfile.txt | 67 ++++++++++++++++++++++++++---
1 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt
index 73ac102..5b1553e 100644
--- a/Documentation/technical/api-lockfile.txt
+++ b/Documentation/technical/api-lockfile.txt
@@ -1,12 +1,65 @@
lockfile API
============
-Talk about <lockfile.c>, things like:
+The lockfile API serves two purposes:
-* lockfile lifetime -- atexit(3) looks at them, do not put them on the
- stack;
-* hold_lock_file_for_update()
-* commit_lock_file()
-* rollback_rock_file()
+* Mutual exclusion. When we write out a new index file, first
+ we create a new file `$GIT_DIR/index.lock`, write the new
+ contents into it, and rename it to the final destination
+ `$GIT_DIR/index`. We try to create the `$GIT_DIR/index.lock`
+ file with O_EXCL so that we can notice and fail when somebody
+ else is already trying to update the index file.
-(JC, Dscho, Shawn)
+* Automatic cruft removal. After we create the "lock" file, we
+ may decide to `die()`, and we would want to make sure that we
+ remove the file that has not been committed to its final
+ destination. This is done by remembering the lockfiles we
+ created in a linked list and cleaning them up from an
+ `atexit(3)` handler. Outstanding lockfiles are also removed
+ when the program dies on a signal.
+
+
+The functions
+-------------
+
+hold_lock_file_for_update::
+
+ Take a pointer to `struct lock_file`, the filename of
+ the final destination (e.g. `$GIT_DIR/index`) and a flag
+ `die_on_error`. Attempt to create a lockfile for the
+ destination and return the file descriptor for writing
+ to the file. If `die_on_error` flag is true, it dies if
+ a lock is already taken for the file; otherwise it
+ returns a negative integer to the caller on failure.
+
+commit_lock_file::
+
+ Take a pointer to the `struct lock_file` initialized
+ with an earlier call to `hold_lock_file_for_update()`,
+ close the file descriptor and rename the lockfile to its
+ final destination.
+
+rollback_lock_file::
+
+ Take a pointer to the `struct lock_file` initialized
+ with an earlier call to `hold_lock_file_for_update()`,
+ close the file descriptor and remove the lockfile.
+
+Because the structure is used in an `atexit(3)` handler, its
+storage has to stay throughout the life of the program. It
+cannot be an auto variable allocated on the stack.
+
+Call `commit_lock_file()` or `rollback_lock_file()` when you are
+done writing to the file descriptor. If you do not call either
+and simply `exit(3)` from the program, an `atexit(3)` handler
+will close and remove the lockfile.
+
+You should not close the file descriptor you obtained from
+`hold_lock_file_for_update` function yourself. The `struct
+lock_file` structure still remembers that the file descriptor
+needs to be closed, and a later call to `commit_lock_file()` or
+`rollback_lock_file()` will result in duplicate calls to
+`close(2)`. Worse yet, if you `close(2)`, open another file
+descriptor for completely different purpose, and then call
+`commit_lock_file()` or `rollback_lock_file()`, they may close
+that unrelated file descriptor.
--
1.5.4.rc3.14.g44397
^ permalink raw reply related
* Re: Git Cygwin - unable to create any repository - help!
From: Paul Umbers @ 2008-01-16 18:57 UTC (permalink / raw)
To: Alex Riesen; +Cc: Junio C Hamano, Robin Rosenberg, git
In-Reply-To: <20080116183840.GB3181@steel.home>
[-- Attachment #1: Type: text/plain, Size: 1317 bytes --]
Done. The cygwin install is the standard, all-defaults install (and
I've done it 5 times now). I've attached a tar.gz with the test
results run under sh and bash. Hope that helps.
On Jan 16, 2008 11:38 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> Paul Umbers, Wed, Jan 16, 2008 19:10:43 +0100:
> > > After seeing the above error, running the test with -i (stop
> > > immediately on failure):
> > >
> > > $ cd t
> > > $ sh -x ./t0000-basic.sh -i -v
> > >
> > Tried Junio's latest suggestion. The resulting output and contents of
> > the trash are attached as a tar.gz. Thanks for all your help guys, I'm
>
> Well, either it didn't work or you omited something critical (like
> stderr):
>
> * expecting success: tree=$(git write-tree)
> * FAIL 5: writing tree out with git write-tree
> tree=$(git write-tree)
>
> that is too short. All the traces missing. Could you please retry
> with
>
> sh -x ./t0000-basic.sh -d -v -i &> test_results.txt
>
> ? If that is what you actually did, I suspect you have a very broken
> shell installed. Could you check if you have bash (bash --version)
> and try it instead of "sh"?
>
>
--
Computer Science is no more about computers than astronomy is about telescopes.
--- Edsger W. Dijkstra
Paul Umbers MSc MBCS MIAP
paul.umbers@gmail.com
[-- Attachment #2: test_results_sh_and_bash.tar.gz --]
[-- Type: application/x-gzip, Size: 2593 bytes --]
^ permalink raw reply
* Re: Git Cygwin - unable to create any repository - help!
From: Paul Umbers @ 2008-01-16 18:50 UTC (permalink / raw)
To: Alex Riesen; +Cc: Robin Rosenberg, git
In-Reply-To: <20080116183124.GA3181@steel.home>
[-- Attachment #1: Type: text/plain, Size: 1343 bytes --]
Sorry, screwed up the log.tar.gz ... try this one.
On Jan 16, 2008 11:31 AM, Alex Riesen <raa.lkml@gmail.com> wrote:
> Paul Umbers, Wed, Jan 16, 2008 16:42:46 +0100:
> > OK, I think this worked (I'm a Java man, not C/C++). I downloaded the
> > latest 1.5.3 source from the git repository and ran "make" with
> > GIT_TEST_OPTS="--verbose --debug". Here's the output:
> >
> ...
> > * expecting success: tree=$(git write-tree)
> > error: invalid object e69de29bb2d1d6434b8b29ae775ad8c2e48c5391
> > fatal: git-write-tree: error building trees
> > * FAIL 5: writing tree out with git write-tree
> > tree=$(git write-tree)
>
> Ok, since you managed to compile it, could you please try to strace
> git-add? Cygwins strace is a bit unusual, but strace --help can
> provide enough information to configure it to trace filesystem
> operations.
>
> In the top-level of Git source directory:
>
> $ uname -a > somefile
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) add somefile
> $ git ls-files -s somefile
>
> or
>
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) hash-object somefile
>
> Than check if the sha1file is missing and send in the log.
>
>
--
Computer Science is no more about computers than astronomy is about telescopes.
--- Edsger W. Dijkstra
Paul Umbers MSc MBCS MIAP
paul.umbers@gmail.com
[-- Attachment #2: log.tar.gz --]
[-- Type: application/x-gzip, Size: 13596 bytes --]
^ permalink raw reply
* Re: Git Cygwin - unable to create any repository - help!
From: Paul Umbers @ 2008-01-16 18:48 UTC (permalink / raw)
To: Alex Riesen; +Cc: Robin Rosenberg, git
In-Reply-To: <20080116183124.GA3181@steel.home>
[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]
Latest output, log file attached:
paulumbers@Devteam29 ~/workspace/git/git-1.5.3
$ uname -a > somefile
paulumbers@Devteam29 ~/workspace/git/git-1.5.3
$ git init
Initialized empty Git repository in .git/
paulumbers@Devteam29 ~/workspace/git/git-1.5.3
$ strace -o log -f -m syscall ./git --exec-path=$(pwd) add somefile
paulumbers@Devteam29 ~/workspace/git/git-1.5.3
$ git ls-files -s somefile
100644 2ed63d326ffdb2fd4b703780f4d61f1893cac63b 0 somefile
Checking .git/objects/2e ... there is no sha1 file
>
> Ok, since you managed to compile it, could you please try to strace
> git-add? Cygwins strace is a bit unusual, but strace --help can
> provide enough information to configure it to trace filesystem
> operations.
>
> In the top-level of Git source directory:
>
> $ uname -a > somefile
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) add somefile
> $ git ls-files -s somefile
>
> or
>
> $ strace -o log -f -m syscall ./git --exec-path=$(pwd) hash-object somefile
>
> Than check if the sha1file is missing and send in the log.
>
>
--
Computer Science is no more about computers than astronomy is about telescopes.
--- Edsger W. Dijkstra
Paul Umbers MSc MBCS MIAP
paul.umbers@gmail.com
[-- Attachment #2: log.tar.gz --]
[-- Type: application/x-gzip, Size: 184 bytes --]
^ permalink raw reply
* Re: Git Cygwin - unable to create any repository - help!
From: Alex Riesen @ 2008-01-16 18:38 UTC (permalink / raw)
To: Paul Umbers; +Cc: Junio C Hamano, Robin Rosenberg, git
In-Reply-To: <a5eb9c330801161010h41e55486y5e8a4335dd939b73@mail.gmail.com>
Paul Umbers, Wed, Jan 16, 2008 19:10:43 +0100:
> > After seeing the above error, running the test with -i (stop
> > immediately on failure):
> >
> > $ cd t
> > $ sh -x ./t0000-basic.sh -i -v
> >
> Tried Junio's latest suggestion. The resulting output and contents of
> the trash are attached as a tar.gz. Thanks for all your help guys, I'm
Well, either it didn't work or you omited something critical (like
stderr):
* expecting success: tree=$(git write-tree)
* FAIL 5: writing tree out with git write-tree
tree=$(git write-tree)
that is too short. All the traces missing. Could you please retry
with
sh -x ./t0000-basic.sh -d -v -i &> test_results.txt
? If that is what you actually did, I suspect you have a very broken
shell installed. Could you check if you have bash (bash --version)
and try it instead of "sh"?
^ 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