* Re: Possible regression: overwriting untracked files in a fresh repo
From: Johannes Schindelin @ 2009-08-25 11:34 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090825030316.GA8098@coredump.intra.peff.net>
Hi,
On Mon, 24 Aug 2009, Jeff King wrote:
> Subject: [PATCH] checkout: do not imply "-f" on unborn branches
>
> [...]
Thanks!
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] git-tag(1): Refer to git-check-ref-format(1) for <name>
From: Nanako Shiraishi @ 2009-08-25 8:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jari Aalto
In-Reply-To: <20090822094518.6117@nanako3.lavabit.com>
Quoting myself...
> Subject: Documentation: consistently refer to check-ref-format
>
> Change the <name> placeholder to <tagname> in the SYNOPSIS section of
> git-tag documentation, and describe it in the OPTIONS section in a way
> similar to how documentation for git-branch does.
>
> Add SEE ALSO section to list the other documentation pages these two pages
> refer to.
>
> Signed-off-by: Nanako Shiraishi <nanako3@lavabit.com>
Should I further polish this patch?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: rebase hint unmerged file removed
From: Miklos Vajna @ 2009-08-25 8:11 UTC (permalink / raw)
To: Johannes Sixt; +Cc: bill lam, git
In-Reply-To: <4A939455.5030908@viscovery.net>
[-- Attachment #1: Type: text/plain, Size: 1282 bytes --]
On Tue, Aug 25, 2009 at 09:35:49AM +0200, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Ok, I bisected it to 6eb1b437 (cherry-pick/revert: make direct internal
> call to merge_tree(), 2008-09-02), which is part of a series around
> merge-recursive: 1ad6d46..ed520a8
>
> The commit message says: "One regression is that the status message is
> lost as there is no way to flush them from outside the refactored library
> code yet." But is it intentional that these messages were never recovered?
>
> BTW, I miss the messages a lot, too, but I'm unfamiliar with the code in
> question to try to do something about it.
I do not remember too much about this. The point of the commit was to
avoid fork()ing a new merge-recursive process, that's for sure. I tried
to search back the archive to see what was the interdiff between my
patch and Junio's fixup, but I haven't found my patch.
I think it's possible that I was not aware of this limitation at all and
just Junio added it as a note to the commit message, but I'm not 100%
sure.
I also think that it's possible to modify the library code to be able to
flush the status message outside the library, but I can't really send a
patch that implements this right now, due to lack of time.
Sorry,
Miklos
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: rebase hint unmerged file removed
From: Johannes Sixt @ 2009-08-25 7:35 UTC (permalink / raw)
To: Miklos Vajna; +Cc: bill lam, git
In-Reply-To: <4A92A6E6.5060702@viscovery.net>
Johannes Sixt schrieb:
> Rebase -i emitted used to write merge-recursive's conflict hints that said
> "modify/delete", "modify/rename", "content", etc. I think that's what you
> were looking for. But these hints have vanished since quite some time now.
> I haven't taken the time, yet, to track down when this happened and why.
Ok, I bisected it to 6eb1b437 (cherry-pick/revert: make direct internal
call to merge_tree(), 2008-09-02), which is part of a series around
merge-recursive: 1ad6d46..ed520a8
The commit message says: "One regression is that the status message is
lost as there is no way to flush them from outside the refactored library
code yet." But is it intentional that these messages were never recovered?
BTW, I miss the messages a lot, too, but I'm unfamiliar with the code in
question to try to do something about it.
-- Hannes
^ permalink raw reply
* Re: Git workflow: Managing topic branches.
From: Johannes Sixt @ 2009-08-25 7:07 UTC (permalink / raw)
To: Thomas Adam; +Cc: git list
In-Reply-To: <18071eea0908240744g359f8b1ey622259e89ac7592a@mail.gmail.com>
Thomas Adam schrieb:
> We have a work-flow such as this:
>
>
> o---o---o---o--o--o (stable)
> /
> o---o---o---o---o---o---o (master)
> \
> o---o--o---o---o---o (featureA)
>
>
> Master is where all our stable code lives after a release -- and also where
> bug-fixes for released code is put. When we're working on a new feature,
> almost all developers here will push (in this case) to "featureA" ---
> eventually this branch will get merged into master, tagged and the code
> released. Then a new branch, "featureB" is created off it, and process
> continues. (Yes, we're using Git in a very CVS-like way, alas.)
>
> Periodically though we need to release updates for our product. This the
> area which is where my question lies about whether the workflow is good or
> not. Here's how we do that:
>
> We have a branch called "stable" which contains all of our released code
> plus any updates release. When we wish to create a new update, we create a
> new branch off the tip of stable:
>
>
> o---o---o---o (updateN)
> /
> o---o---o---o--o--o (stable)
> /
> o---o---o---o---o---o---o (master)
> \
> o---o--o---o---o---o (featureA)
>
>
> Because bug-fixes happen on Master, we now want those fixes to appear on the
> updateN branch so we can create a tarball from them (to release to our
> customers). We're using "git cherry" to get a list of SHA1s that are
> relevant between updateN and master, as in:
>
> git cherry updateN master
>
> ... and then manually deciding (based on it's "+"/"-" output whether that
> SHA1 needs to be used and then:
>
> git cherry-pick SHA1
>
> ... onto updateN as appropriate.
Your workflow looks quite reasonable except for this last part. You should
make your history look like this:
o--o--o--o--o stable
/ \ \
--o--B--o--o--o---o--o--o--o master
\
o--o--o--o--o--o featureA
Instead of cherry-picking commits onto updateN (or stable), your
developers should think which branches need the change that they are about
to commit. If it is a serious bug-fix, then it should be enter the picture
on stable, not on master or feature branches. The important part is that
branch stable is merged into branch master (at least after each release,
but perhaps even more often).
Now assume that your developer discovers a bug while she was developing on
featureA that must go into stable. Previously you would have done it this
way (I presume):
--o--B--o--o--o---o--o--o--o--o master
| \ / / /
| o--o--o--o--o-----F' stable
\
o--o--o--o--o--o--F featureA
That is, the fix F was committed on the feature branch and later was
cherry-picked onto stable as F' and then merged into master. But she
should have done this instead:
--o--B--o--o--o---o--o--o--o--o master
| \ / / /
| o--o--o--o--o-----o stable
|\ /
| F------------------< fixF
\ \
o--o--o--o--o--o-----o featureA
That is, fix F should go on its own bugfix-branch and that branch is
merged into stable as well as into featureA (but only if the fix is really
needed there); stable is in turn merged into master again. The new branch
grows from a commit that is in common to all the branches that need the
fix. Since the fix is needed on featureA as well as on stable, the lastest
possible branch point is B (where feature A was branch off of master/stable).
Side note: An even better branch point would be the commit that introduced
the bug, which must have been B or a commit before it; otherwise the bug
would not have shown up during the development of featureA. This way you
could make the decision anytime later whether you want to merge the fix
into older releases as well.
-- Hannes
^ permalink raw reply
* Re: [PATCH/RFC] make the new block-sha1 the default
From: Junio C Hamano @ 2009-08-25 6:50 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jeff King, git
In-Reply-To: <20090825041859.GA10033@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:
>
>> ... and remove support for linking against the openssl SHA1 code.
>>
>> The block-sha1 implementation is not significantly worse and sometimes
>> even faster than the openssl SHA1 implementation. This allows for
>
> Is there a reason not to leave the option of linking against openssl?
I think it is a valid question. Why remove the _option_?
I would certainly understand it if you made BLK_SHA1 the _default_, though.
^ permalink raw reply
* Re: [PATCH] fix simple deepening of a repo
From: Junio C Hamano @ 2009-08-25 6:33 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Nicolas Pitre, Julian Phillips, Daniel Barkalow,
Johannes Schindelin, git
In-Reply-To: <20090825061248.GG1033@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> The client knows the *name* of the ref, but not the SHA-1 the ref is
> currently valued at. Thus when the client knows it wants a certain
> ref by name, it needs to send a "want " line to the server that would
> give it whatever that ref currently points at. Unfortunately since we
> have not obtained that value yet, we are stuck.
That could be something you can fix in the out-of-band procedure Gerrit
uses (you let the client learn both name and value offline, and then the
client uses that value on "want" line).
However, even if we limit the discussion to Gerrit, you would need an
updated client that can be called with the out-of-band information
(i.e. "we know that changes/88/4488/2 points at X, so use X when
requesting") when talking with such an updated server.
So I think that expand-refs is a much nicer general solution than just
"server side is configured to hide but still allow certain refs", and
client updates cannot be avoided.
And again,
> The problem with this is servers which are sending this expand-refs
> tag have hidden certain namespaces from older clients. Those names
> can't be seen by older git clients, unless the user does an upgrade.
I do not think "generally hidden, but if you need to know you are allowed
to peek" is much of a problem. You do not do that for regular refs, only
for "on-demand-as-needed" type things. If we are going to make extensive
use of notes on commits to give richer annotations, I suspect notes
hierarchy could be hidden by default in a similar way.
^ permalink raw reply
* Re: [PATCH] fix simple deepening of a repo
From: Shawn O. Pearce @ 2009-08-25 6:12 UTC (permalink / raw)
To: Junio C Hamano
Cc: Nicolas Pitre, Julian Phillips, Daniel Barkalow,
Johannes Schindelin, git
In-Reply-To: <7vab1osc2m.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > We aren't quite at the 50k ref stage yet, but we're starting to
> > consider that some of our repositories have a ton of refs, and
> > that the initial advertisement for either fetch or push is horrid.
> >
> > Since the refs are immutable I could actually teach the JGit
> > daemon to hide them from JGit's receive-pack, thus cutting down the
> > advertisement on push, but the refs exist so you can literally say:
>
> What do you mean "refs are immutable"?
>
> Do you mean "In the particular application, Gerrit, the server knows that
> certain refs will never move nor get deleted, once they are created"? If
> so, then I would understand, but otherwise what you are describing is not
> git anymore ;-)
The former. :-)
I mean that this particular server implementation will deny any
update made to refs/changes/, as if one had the following as the
update hook on that repository:
#!/bin/sh
case "$1" in
refs/changes/*) exit 1;;
*) exit 0;
esac
This of course is completely legal, and since the server knows the
ref cannot be moved, there is no need to advertise it to the client.
But this is a very specialized thing, its rare that the thing that
formats the advertisement knows what the update hook will permit
to be modified.
> > git fetch --uploadpack='git upload-pack --ref refs/changes/88/4488/2' URL refs/changes/88/4488/2
> >
> > Personally I'd prefer extending the protocol, because making the
> > end user supply information twice is stupid.
>
> In the upload-pack protocol, the server talks first, so it is rather hard
> to shoehorn a request from a client to ask "I know about refs/changes/*
> hiearchy, so don't talk about them".
Actually, that assumption is still a problem.
The client knows the *name* of the ref, but not the SHA-1 the ref is
currently valued at. Thus when the client knows it wants a certain
ref by name, it needs to send a "want " line to the server that would
give it whatever that ref currently points at. Unfortunately since we
have not obtained that value yet, we are stuck.
However, we do have one name we want to know about, but the server may
have 50k other names in the same namespace we do not know about.
I was thinking instead that we have a new protocol extension:
S: ... HEAD\0side-band ... expand-refs
S: ... refs/heads/master
S: 0000
C: ... expand refs/changes/88/4488/2
C: 0000
S: ... refs/changes/88/4488/2
S: 0000
C: ... want XXXXXX\0side-band-64k ...
> Of course, the client side cannot grab everything with refs/*:refs/remotes/*
> wildcard refspecs from such a server, but I think that can be considered a
> feature.
If expand accepted globs like fetch does, then fetch can ask for
expand of refs/changes/* if it does not see any refs/changes/*
on advertisement. Or just expand a particular ref, or handful of
refs, that the user has asked for on the fetch line.
The problem with this is servers which are sending this expand-refs
tag have hidden certain namespaces from older clients. Those names
can't be seen by older git clients, unless the user does an upgrade.
This might be OK for Gerrit Code Review's refs/changes/ namespace,
but it may not be good for others.
--
Shawn.
^ permalink raw reply
* Re: [PATCH-v2/RFC 3/6] xutils: fix ignore-all-space on incomplete line
From: Thell Fowler @ 2009-08-25 5:58 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Johannes Schindelin
In-Reply-To: <alpine.DEB.2.00.0908240910120.29625@GWPortableVCS>
Thell Fowler (git@tbfowler.name) wrote on Aug 24, 2009:
> Junio C Hamano (gitster@pobox.com) wrote on Aug 24, 2009:
>
> > Thell Fowler <git@tbfowler.name> writes:
> >
> > > It passed every test I threw at it, although it seemed to be a tad bit
> > > slower than the previous revision on my sample data so I ran the following
> > > command several times for both the previous and current version:
> > >
> >
> > Do you mean by "previous version" the one that was broken, or the one I
> > sent as a "how about" patch?
> >
>
> A quick test shows the version merged to pu is the one that had the
> fastest times. I'll be away from a connection most of today, but will
> test the different versions against the tests and some sample data and
> post back.
>
More extensive testing also shows the version currently in pu is the
fastest on my sample data when applied to master. I'm not sure why pu
shows slower times than those same commits applied to master, but they
are close enough together that I'm guessing no-one would really be
concerned.
I was sitting in a waiting room and decided to have a little fun figuring
out how to average the sys times...
for arg in "" -w -b --ignore-space-at-eol;do sum=0 && for i in {1..50}; \
do n="$(/usr/bin/time -f "%S" -o /dev/stdout sh -c 'git diff $arg dirty_first>/dev/null;')"; \
sum=$sum+$n; done; echo "scale=2; ($sum)/$i"|echo "$(bc) avg for diff $arg"; done;
pu
.28 avg for diff
.29 avg for diff -w
.33 avg for diff -b
.29 avg for diff --ignore-space-at-eol
pu commits applied to master <=== FASTEST
9c0d402 xutils: Fix xdl_recmatch() on incomplete lines
21245fd xutils: Fix hashing an incomplete line with whitespaces at the end
.26 avg for diff
.25 avg for diff -w
.29 avg for diff -b
.31 avg for diff --ignore-space-at-eol
'how about' patch applied to master
.26 avg for diff
.32 avg for diff -w
.29 avg for diff -b
.32 avg for diff --ignore-space-at-eol
current master (in order to see the difference in the basic git diff
-ignoring the fact that incomplete lines where broke since it only affects
2 files in the test data)
.30 avg for diff
.30 avg for diff -w
.29 avg for diff -b
.29 avg for diff --ignore-space-at-eol
-- Thell
^ permalink raw reply
* Re: gitosis-lite
From: Sitaram Chamarty @ 2009-08-25 5:53 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Git Mailing List
In-Reply-To: <m363cdm4pm.fsf@localhost.localdomain>
[took Tommi out of cc; he must be getting enough mail as is...]
On Tue, Aug 25, 2009 at 12:14 AM, Jakub Narebski<jnareb@gmail.com> wrote:
> A few comments about the code, taking gl-auth-command as example.
> Wouldn't it be better to use "use warnings" instead of 'perl -w'?
I'm not sure what is the minimum perl required for git
itself. Has it needed perl > 5.6.0 for more than a year at
least? The only real difference between these two is scope,
which is a non-issue here, so I played safe.
> It would be, I think, better if you have used POD for such
> documentation. One would be able to generate manpage using pod2man,
> and it is no less readable in source code. See e.g. perl/Git.pm or
> contrib/hooks/update-paranoid.
Hmm... I've been spoiled by Markdown's sane bullet list
handling. Visually, POD forces everything other than code
to be flush left -- any sort of list is definitely less
readable in source code as a result. IMHO of course.
>> our $GL_ADMINDIR;
>> our $GL_CONF;
>> our $GL_KEYDIR;
>> our $GL_CONF_COMPILED;
>> our $REPO_BASE;
>> our %repos;
>
> Why is the reason behind using 'our' instead of 'my' here?
They are assigned values in some file being "do"-ed, so they
can't be lexical scoped. However, I found a few others that
were holdovers from an earlier version. Fixed; thanks for
catching that.
>> # first, fix the biggest gripe I have with gitosis, a 1-line change
>> my $user=$ENV{GL_USER}=shift; # there; now that's available everywhere!
>
> Eh? This is standalone script, isn't it? Shouldn't it be
>
> my $user = $ENV{GL_USER} = $ARGV[0]; # there; now that's available everywhere!
Hmm... I didn't know there was a difference, other than
depleting @ARGV, if you're outside a subroutine. I'll take
a relook at it.
>> my $perm = 'W'; $perm = 'R' if $verb =~ $R_COMMANDS;
>
> Either split it into two lines, or use ?: confitional operator:
>
> my $perm = ($verb =~ $R_COMMANDS ? 'R' : 'W');
much nicer... Fixed, thanks.
>> open(LOG, ">>", "$GL_ADMINDIR/log");
>> print LOG "\n", scalar(localtime), " $ENV{SSH_ORIGINAL_COMMAND} $user\n";
>> close(LOG);
>
> It is better practice to use lexical variables instead of barewords
> for filehandles:
Good catch; thanks! I guess I'm showing my age :) Fixed
all of them!
> Don't forget to check for error.
Hmm.. well I'm still debating if a log file write error
should block git access / push, but there were two more
important closes (again in gl-compile-conf) that were
unguarded. Fixed, thanks.
>> $repo = "'$REPO_BASE/$repo.git'";
>> exec("git", "shell", "-c", "$verb $repo");
>
> That's not enough. You have to shell-quote $repo, like in gitweb or
> using String::ShellQuote module, or somehow use list form to pass
> arguments to git-shell. You protect here againts spaces in filename,
> but not againts "'" (single quote) and for show shells "!"
> (exclamation mark).
It'll never get here. It'll die much earlier if the
reponame does not match a much stricter pattern. Maybe too
strict, actually: ^[0-9a-zA-Z][0-9a-zA-Z._/-]*$
However, I then realised I should tighten up the R_COMMANDS
and W_COMMANDS patterns a wee bit; as it stands, if someone
could create a file called, say "git-upload-pack.pwned", and
put it in the PATH, he could get the "git" user to execute
it! Sure if he managed to put something in the PATH that's
already game over in some sense but we have to stop what we
can :-) Thanks for catching this.
And once again, I really appreciate the extra eyeballs on
the code!
--
Sitaram
^ permalink raw reply
* Re: checkout to a reflog entry
From: bill lam @ 2009-08-25 5:37 UTC (permalink / raw)
To: git
In-Reply-To: <20090825052200.GA15880@coredump.intra.peff.net>
On Tue, 25 Aug 2009, Jeff King wrote:
> A reflog entry is not a branch; it is just a pointer to the commit where
> a branch was at some point. Using "git checkout" on it will let you
> explore the contents, just as you might with a tag. If you want to build
Thank you for explanation on the difference between checkout and
reset.
--
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
^ permalink raw reply
* Re: [PATCH] fix simple deepening of a repo
From: Junio C Hamano @ 2009-08-25 5:21 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Nicolas Pitre, Julian Phillips, Daniel Barkalow,
Johannes Schindelin, git
In-Reply-To: <20090825021223.GE1033@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> We aren't quite at the 50k ref stage yet, but we're starting to
> consider that some of our repositories have a ton of refs, and
> that the initial advertisement for either fetch or push is horrid.
>
> Since the refs are immutable I could actually teach the JGit
> daemon to hide them from JGit's receive-pack, thus cutting down the
> advertisement on push, but the refs exist so you can literally say:
What do you mean "refs are immutable"?
Do you mean "In the particular application, Gerrit, the server knows that
certain refs will never move nor get deleted, once they are created"? If
so, then I would understand, but otherwise what you are describing is not
git anymore ;-)
And I think it is probably worth thinking things through to find a way to
take advantage of that knowledge.
Even though refs under refs/changes/ hierarchy may have that property, the
client won't know what's available unless you advertise it in some way.
You could assume some offline measure outside the git protocol exists for
clients to find out about them, and protocol extension could say "I do not
want to hear about refs that match these globs during this exchange,
because I have learnt about them offline", and the server could skip
advertisement.
> git fetch --uploadpack='git upload-pack --ref refs/changes/88/4488/2' URL refs/changes/88/4488/2
>
> Personally I'd prefer extending the protocol, because making the
> end user supply information twice is stupid.
In the upload-pack protocol, the server talks first, so it is rather hard
to shoehorn a request from a client to ask "I know about refs/changes/*
hiearchy, so don't talk about them".
I however think it is entirely reasonable to have a server side
configuration that tells upload-pack not to advertise refs/changes/*
hierarchy but still remembers they are OUR_REF. In send_ref() in
upload-pack.c, you'd do something like (I know, I know, you'd be doing
an equivalent of this in jgit):
static const char *capabilities = "multi_ack ...";
struct object *o = parse_object(sha1);
int skip_advertisement = exclude_ref_from_advertisement(refname);
if (!o)
die("git upload-pack: cannot find object %s:", sha1_to_hex(sha1));
if (!skip_advertisement) {
if (capabilities)
packet_write(1, "%s %s%c%s\n", sha1_to_hex(sha1), refname,
0, capabilities);
else
packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname);
capabilities = NULL;
}
if (!(o->flags & OUR_REF)) {
o->flags |= OUR_REF;
nr_our_refs++;
}
if (o->type == OBJ_TAG) {
o = deref_tag(o, refname, 0);
if (o && !skip_advertisement)
packet_write(1, "%s %s^{}\n", sha1_to_hex(o->sha1), refname);
}
return 0;
Doing it this way, receive_needs() will allow refs/changes/88/4488/2 to be
requested, because that is what send_ref() saw and marked as OUR_REF. It
was just not sent to the client. And get_common_commits() will behave the
same with or without this abbreviated advertisement,
Of course, the client side cannot grab everything with refs/*:refs/remotes/*
wildcard refspecs from such a server, but I think that can be considered a
feature.
^ permalink raw reply
* Re: checkout to a reflog entry
From: Jeff King @ 2009-08-25 5:22 UTC (permalink / raw)
To: git
In-Reply-To: <20090825045235.GB391@debian.b2j>
On Tue, Aug 25, 2009 at 12:52:35PM +0800, bill lam wrote:
> project(master)$ git checkout master@{1}
> Note: moving to 'master@{1}' which isn't a local branch
> [...]
> It becomes (no branch) although the SHA1 is correct. If I then
> checkout master, then it revert and jump back up the previous master
> HEAD. I tried HEAD@{xx} also failed. What is the correct way of to
> checkout that master@{..} and stay in that master branch? (Sometimes
> it worked I forgot how and why)
A reflog entry is not a branch; it is just a pointer to the commit where
a branch was at some point. Using "git checkout" on it will let you
explore the contents, just as you might with a tag. If you want to build
on it, you need to either:
1. Make a new branch to work on, starting at that point:
git checkout -b my-topic master@{1}
2. Reset your current branch (and worktree) to point at that commit:
git reset --hard master@{1}
-Peff
^ permalink raw reply
* [PATCH v2 3/3] gitweb: add t9501 tests for checking HTTP status codes
From: Mark Rada @ 2009-08-25 5:03 UTC (permalink / raw)
To: Junio C Hamano, Jakub Narebski; +Cc: git
Commit message changed as suggested by Jakub.
Also added a test_description, and removed a bit of whitespace
that I hadn't noticed before.
--
Mark A Rada (ferrous26)
marada@uwaterloo.ca
--->8---
This commit adds a new test file, t9501, that is used for gitweb
test cases that check HTTP status codes and messages.
Currently, the only tests are for the snapshot feature.
Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
t/t9501-gitweb-standalone-http-status.sh | 78 ++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
create mode 100644 t/t9501-gitweb-standalone-http-status.sh
diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
new file mode 100644
index 0000000..d0ff21d
--- /dev/null
+++ b/t/t9501-gitweb-standalone-http-status.sh
@@ -0,0 +1,78 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Mark Rada
+#
+
+test_description='gitweb as standalone script (http status tests).
+
+This test runs gitweb (git web interface) as a CGI script from the
+commandline, and checks that it returns the expected HTTP status
+code and message.'
+
+
+. ./gitweb-lib.sh
+
+# ----------------------------------------------------------------------
+# snapshot settings
+
+test_commit \
+ 'SnapshotTests' \
+ 'i can has snapshot?'
+
+cat >>gitweb_config.perl <<\EOF
+$feature{'snapshot'}{'override'} = 0;
+EOF
+
+test_expect_success \
+ 'snapshots: tgz only default format enabled' \
+ 'gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tgz" &&
+ grep "Status: 200 OK" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tbz2" &&
+ grep "403 - Unsupported snapshot format" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=txz" &&
+ grep "403 - Snapshot format not allowed" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=zip" &&
+ grep "403 - Unsupported snapshot format" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
+cat >>gitweb_config.perl <<\EOF
+$feature{'snapshot'}{'default'} = ['tgz','tbz2','txz','zip'];
+EOF
+
+test_expect_success \
+ 'snapshots: all enabled in default, use default disabled value' \
+ 'gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tgz" &&
+ grep "Status: 200 OK" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tbz2" &&
+ grep "Status: 200 OK" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=txz" &&
+ grep "403 - Snapshot format not allowed" gitweb.output &&
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=zip" &&
+ grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
+cat >>gitweb_config.perl <<\EOF
+$known_snapshot_formats{'zip'}{'disabled'} = 1;
+EOF
+
+test_expect_success \
+ 'snapshots: zip explicitly disabled' \
+ 'gitweb_run "p=.git;a=snapshot;h=HEAD;sf=zip" &&
+ grep "403 - Snapshot format not allowed" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
+cat >>gitweb_config.perl <<\EOF
+$known_snapshot_formats{'tgz'}{'disabled'} = 0;
+EOF
+
+test_expect_success \
+ 'snapshots: tgz explicitly enabled' \
+ 'gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tgz" &&
+ grep "Status: 200 OK" gitweb.output'
+test_debug 'cat gitweb.output'
+
+
+test_done
--
1.6.4
^ permalink raw reply related
* [PATCH v2 2/3] gitweb: split test suite into library and tests
From: Mark Rada @ 2009-08-25 5:00 UTC (permalink / raw)
To: Junio C Hamano, Jakub Narebski; +Cc: git
Change the subject from saying 'break' to 'split', because break
may have given people the wrong idea about what the commit is doing.
I also put the test_description back into t9500, having it in
gitweb-lib.sh was a mistake.
--
Mark A Rada (ferrous26)
marada@uwaterloo.ca
--->8---
To accommodate additions to the tests cases for gitweb, the preamble
from t9500 is now in its own library so that new sets of tests for
gitweb can use the same setup without copying the code.
Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
t/gitweb-lib.sh | 73 ++++++++++++++++++++++++++++++++
t/t9500-gitweb-standalone-no-errors.sh | 67 +----------------------------
2 files changed, 74 insertions(+), 66 deletions(-)
create mode 100644 t/gitweb-lib.sh
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
new file mode 100644
index 0000000..8452532
--- /dev/null
+++ b/t/gitweb-lib.sh
@@ -0,0 +1,73 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Jakub Narebski
+#
+
+gitweb_init () {
+ safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+ cat >gitweb_config.perl <<EOF
+#!/usr/bin/perl
+
+# gitweb configuration for tests
+
+our \$version = 'current';
+our \$GIT = 'git';
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = 'projects';
+our \$site_name = '[localhost]';
+our \$site_header = '';
+our \$site_footer = '';
+our \$home_text = 'indextext.html';
+our @stylesheets = ('file:///$TEST_DIRECTORY/../gitweb/gitweb.css');
+our \$logo = 'file:///$TEST_DIRECTORY/../gitweb/git-logo.png';
+our \$favicon = 'file:///$TEST_DIRECTORY/../gitweb/git-favicon.png';
+our \$projects_list = '';
+our \$export_ok = '';
+our \$strict_export = '';
+
+EOF
+
+ cat >.git/description <<EOF
+$0 test repository
+EOF
+}
+
+gitweb_run () {
+ GATEWAY_INTERFACE='CGI/1.1'
+ HTTP_ACCEPT='*/*'
+ REQUEST_METHOD='GET'
+ SCRIPT_NAME="$TEST_DIRECTORY/../gitweb/gitweb.perl"
+ QUERY_STRING=""$1""
+ PATH_INFO=""$2""
+ export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
+ SCRIPT_NAME QUERY_STRING PATH_INFO
+
+ GITWEB_CONFIG=$(pwd)/gitweb_config.perl
+ export GITWEB_CONFIG
+
+ # some of git commands write to STDERR on error, but this is not
+ # written to web server logs, so we are not interested in that:
+ # we are interested only in properly formatted errors/warnings
+ rm -f gitweb.log &&
+ perl -- "$SCRIPT_NAME" \
+ >gitweb.output 2>gitweb.log &&
+ if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
+
+ # gitweb.log is left for debugging
+ # gitweb.output is used to parse http output
+}
+
+. ./test-lib.sh
+
+if ! test_have_prereq PERL; then
+ say 'skipping gitweb tests, perl not available'
+ test_done
+fi
+
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+ say 'skipping gitweb tests, perl version is too old'
+ test_done
+}
+
+gitweb_init
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6275181..2fc7fdb 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -9,73 +9,8 @@ This test runs gitweb (git web interface) as CGI script from
commandline, and checks that it would not write any errors
or warnings to log.'
-gitweb_init () {
- safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
- cat >gitweb_config.perl <<EOF
-#!/usr/bin/perl
-
-# gitweb configuration for tests
-
-our \$version = "current";
-our \$GIT = "git";
-our \$projectroot = "$safe_pwd";
-our \$project_maxdepth = 8;
-our \$home_link_str = "projects";
-our \$site_name = "[localhost]";
-our \$site_header = "";
-our \$site_footer = "";
-our \$home_text = "indextext.html";
-our @stylesheets = ("file:///$TEST_DIRECTORY/../gitweb/gitweb.css");
-our \$logo = "file:///$TEST_DIRECTORY/../gitweb/git-logo.png";
-our \$favicon = "file:///$TEST_DIRECTORY/../gitweb/git-favicon.png";
-our \$projects_list = "";
-our \$export_ok = "";
-our \$strict_export = "";
-EOF
-
- cat >.git/description <<EOF
-$0 test repository
-EOF
-}
-
-gitweb_run () {
- GATEWAY_INTERFACE="CGI/1.1"
- HTTP_ACCEPT="*/*"
- REQUEST_METHOD="GET"
- SCRIPT_NAME="$TEST_DIRECTORY/../gitweb/gitweb.perl"
- QUERY_STRING=""$1""
- PATH_INFO=""$2""
- export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
- SCRIPT_NAME QUERY_STRING PATH_INFO
-
- GITWEB_CONFIG=$(pwd)/gitweb_config.perl
- export GITWEB_CONFIG
-
- # some of git commands write to STDERR on error, but this is not
- # written to web server logs, so we are not interested in that:
- # we are interested only in properly formatted errors/warnings
- rm -f gitweb.log &&
- perl -- "$SCRIPT_NAME" \
- >/dev/null 2>gitweb.log &&
- if grep "^[[]" gitweb.log >/dev/null 2>&1; then false; else true; fi
-
- # gitweb.log is left for debugging
-}
-
-. ./test-lib.sh
-
-if ! test_have_prereq PERL; then
- say 'skipping gitweb tests, perl not available'
- test_done
-fi
-
-perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
- say 'skipping gitweb tests, perl version is too old'
- test_done
-}
-
-gitweb_init
+. ./gitweb-lib.sh
# ----------------------------------------------------------------------
# no commits (empty, just initialized repository)
--
1.6.4
^ permalink raw reply related
* Re: [PATCH] fix simple deepening of a repo
From: Sverre Rabbelier @ 2009-08-25 5:00 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Nicolas Pitre, Julian Phillips, Daniel Barkalow, Junio C Hamano,
Johannes Schindelin, git
In-Reply-To: <20090825021223.GE1033@spearce.org>
Heya,
On Mon, Aug 24, 2009 at 19:12, Shawn O. Pearce<spearce@spearce.org> wrote:
> git fetch --uploadpack='git upload-pack --ref refs/changes/88/4488/2' URL refs/changes/88/4488/2
>
> Personally I'd prefer extending the protocol, because making the
> end user supply information twice is stupid.
$ git fetch --unadvertised URL refs/changes/88/4488/2
And we expand that to the above behind the screens?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH v2 1/3] gitweb: improve snapshot error handling
From: Mark Rada @ 2009-08-25 4:59 UTC (permalink / raw)
To: Junio C Hamano, Jakub Narebski; +Cc: git
I reworded the message to be more specific about what code was
being changed.
--
Mark A Rada (ferrous26)
marada@uwaterloo.ca
--->8---
The last check in the second block of checks in the &git_snapshot
routine is never executed because the second last check is a superset
of the last check.
This change will switch the order of the last two checks. It has the
advantage of giving clients a more specific reason why they cannot get
a snapshot format if the format they have chosen is disabled.
Signed-off-by: Mark Rada <marada@uwaterloo.ca>
---
gitweb/gitweb.perl | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4a42f61..7068db2 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -5174,10 +5174,10 @@ sub git_snapshot {
die_error(400, "Invalid snapshot format parameter");
} elsif (!exists($known_snapshot_formats{$format})) {
die_error(400, "Unknown snapshot format");
- } elsif (!grep($_ eq $format, @snapshot_fmts)) {
- die_error(403, "Unsupported snapshot format");
} elsif ($known_snapshot_formats{$format}{'disabled'}) {
die_error(403, "Snapshot format not allowed");
+ } elsif (!grep($_ eq $format, @snapshot_fmts)) {
+ die_error(403, "Unsupported snapshot format");
}
if (!defined $hash) {
--
1.6.4
^ permalink raw reply related
* checkout to a reflog entry
From: bill lam @ 2009-08-25 4:52 UTC (permalink / raw)
To: git
Sometimes when git reflog and try to checkout to a previous commit,
project((6a33dbc...))$ git reflog show master
77aa688 master@{0}: rebase -i (finish): refs/heads/master onto fc3eaa4
6a33dbc master@{1}: commit: wip
.....
project(master)$ git checkout master@{1}
Note: moving to 'master@{1}' which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
git checkout -b <new_branch_name>
HEAD is now at 6a33dbc... wip
project((6a33dbc...))$
project((6a33dbc...))$ git branch
* (no branch)
master
It becomes (no branch) although the SHA1 is correct. If I then checkout master,
then it revert and jump back up the previous master HEAD. I tried HEAD@{xx}
also failed. What is the correct way of to checkout that master@{..} and stay
in that master branch? (Sometimes it worked I forgot how and why)
--
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
^ permalink raw reply
* Re: hitting home directory's parent
From: Jeff King @ 2009-08-25 4:48 UTC (permalink / raw)
To: Daniel Convissor; +Cc: Git Mailing List
In-Reply-To: <20090823051244.GB10761@panix.com>
On Sun, Aug 23, 2009 at 01:12:44AM -0400, Daniel Convissor wrote:
> That explains what's happening. cmd_help() calls git_config(). Though
> it seems it would be more, uh, helpful, if help produced basic usage
> output no matter what, and then produce the more detailed config based
> output if possible.
I agree, but the way the code is structured right now means doing this
right is probably a pretty big change (see my other message to Duy). So
I think it will probably go unfixed for the time being.
> Is there a bug tracker y'all use where I can enter a ticket?
We don't have a formal bug tracker, but this is really just a sub-part
of another issue, so it will get addressed when the parent issue is
fixed.
-Peff
^ permalink raw reply
* Re: rebase hint unmerged file removed
From: bill lam @ 2009-08-25 4:42 UTC (permalink / raw)
To: git
In-Reply-To: <20090824143657.GA12809@coredump.intra.peff.net>
On Mon, 24 Aug 2009, Jeff King wrote:
> The version of "git status" in next (and presumably what will be in
> v1.6.5) shows unmerged paths with more information. In this case,
Thanks, Jeff, Johannes,
I'll wait to see.
--
regards,
====================================================
GPG key 1024D/4434BAB3 2008-08-24
gpg --keyserver subkeys.pgp.net --recv-keys 4434BAB3
^ permalink raw reply
* Re: hitting home directory's parent
From: Jeff King @ 2009-08-25 4:42 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Daniel Convissor, Git Mailing List
In-Reply-To: <fcaeb9bf0908222107i6d999335r998a304aaa3cd405@mail.gmail.com>
On Sun, Aug 23, 2009 at 11:07:39AM +0700, Nguyen Thai Ngoc Duy wrote:
> It is (and should be worked around with GIT_CEILING_DIRECTORIES).
> Unfortunately in my test, it could not chdir() back when it failed to
> find gitdir. chdir() was called with an absolute directory, and one
> directory in that path was inaccesible, leading another die("Cannot
> come back to cwd"). This one is fatal and should not be ignored. I
> don't know whether having an inaccesible parent directory is a real
> scenario, as lots of tools would break.
Hmm, good point. Some of those die()s really can't be removed, because
we have munged state in a way that can't be recovered (or is hard to
recover).
Also, thinking about it a bit more, I'm not sure it is correct to turn
such a die() into a "return -1" without passing more information along
to the caller. Because there are really three possible outcomes:
1. we are in a git repo
2. we are not in a git repo
3. some error occurred and we could not determine which
of (1) and (2) is the case
And the "gently" bit really just refers to (1) versus (2). It is
tempting to fold (3) into (2), but I'm not sure that is the right thing
to do; some callers (including script callers not under our control) may
care about the distinction (though I think we already blur it; for
example, a non-existent '.git' regular file is treated the same as one
which we couldn't stat for permission problems).
So perhaps the right thing is to either pass back a tri-state status, or
to have an extra_gently mode, either of which could be used by "git
help". The problem with either is that the current setup process is not
amenable to exiting in the middle -- it really may leave us in a weird
and buggy state due to the chdir.
So short of major surgery on the setup procedure, I think we may have to
leave this bug be for now (especially because I'm not convinced the
setup of a non-executable parent directory isn't a little bit crazy).
-Peff
^ permalink raw reply
* Re: [PATCH/RFC] make the new block-sha1 the default
From: Jeff King @ 2009-08-25 4:18 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.00.0908242249420.6044@xanadu.home>
On Mon, Aug 24, 2009 at 11:04:37PM -0400, Nicolas Pitre wrote:
> ... and remove support for linking against the openssl SHA1 code.
>
> The block-sha1 implementation is not significantly worse and sometimes
> even faster than the openssl SHA1 implementation. This allows for
Is there a reason not to leave the option of linking against openssl?
I'm still getting better numbers for OpenSSL over block-sha1 when doing
"git fsck --full" in some repos. Particularly those with large files and
few deltas, where the time is heavily influenced by sha-1 performance.
I'm seeing up to 20% speed improvement using OpenSSL on those repos, and
about 8% on linux-2.6 (the processor is a Conroe Core 2, git compiled
with -O2).
But what really kills me is that I usually compile git with '-O0'
because I am often investigating bugs and I like the debugger to act
sanely. The performance hit is usually not noticeable, but in this case
it is: my "git fsck --full" times jump from ~8.2s (OpenSSL) and ~10.3s
(block-sha1, -O2) to ~18.2s (block-sha1, -O0).
Certainly you can argue that it is idiotic to benchmark anything at -O0.
But right now, it is perfectly reasonable to compile git with -O0 and
assume OpenSSL is compiled with sane optimizations. I'd rather not take
that away without a good reason.
-Peff
^ permalink raw reply
* [PATCH/RFC] make the new block-sha1 the default
From: Nicolas Pitre @ 2009-08-25 3:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
... and remove support for linking against the openssl SHA1 code.
The block-sha1 implementation is not significantly worse and sometimes
even faster than the openssl SHA1 implementation. This allows for
getting rid of the dependency and runtime linking to openssl which is
a relatively important source of latency when executing git commands.
Signed-off-by: Nicolas Pitre <nico@cam.org>
---
OK... So here it is. After all, wanting to get rid of openssl is what
started it all in the first place.
diff --git a/INSTALL b/INSTALL
index ae7f750..55eb962 100644
--- a/INSTALL
+++ b/INSTALL
@@ -52,13 +52,6 @@ Issues of note:
- "zlib", the compression library. Git won't build without it.
- - "openssl". Unless you specify otherwise, you'll get the SHA1
- library from here.
-
- If you don't have openssl, you can use one of the SHA1 libraries
- that come with git (git includes the one from Mozilla, and has
- its own PowerPC and ARM optimized ones too - see the Makefile).
-
- libcurl library; git-http-fetch and git-fetch use them. You
might also want the "curl" executable for debugging purposes.
If you do not use http transfer, you are probably OK if you
diff --git a/Makefile b/Makefile
index 4190a5d..8f28b09 100644
--- a/Makefile
+++ b/Makefile
@@ -16,7 +16,6 @@ all::
# when attempting to read from an fopen'ed directory.
#
# Define NO_OPENSSL environment variable if you do not have OpenSSL.
-# This also implies BLK_SHA1.
#
# Define NO_CURL if you do not have libcurl installed. git-http-pull and
# git-http-push are not built, and you cannot use http:// and https://
@@ -84,10 +83,6 @@ all::
# specify your own (or DarwinPort's) include directories and
# library directories by defining CFLAGS and LDFLAGS appropriately.
#
-# Define BLK_SHA1 environment variable if you want the C version
-# of the SHA1 that assumes you can do unaligned 32-bit loads and
-# have a fast htonl() function.
-#
# Define PPC_SHA1 environment variable when running make to make use of
# a bundled SHA1 routine optimized for PowerPC.
#
@@ -1013,7 +1008,6 @@ ifndef NO_OPENSSL
endif
else
BASIC_CFLAGS += -DNO_OPENSSL
- BLK_SHA1 = 1
OPENSSL_LIBSSL =
endif
ifdef NEEDS_SSL_WITH_CRYPTO
@@ -1162,18 +1156,14 @@ ifdef NO_DEFLATE_BOUND
BASIC_CFLAGS += -DNO_DEFLATE_BOUND
endif
-ifdef BLK_SHA1
- SHA1_HEADER = "block-sha1/sha1.h"
- LIB_OBJS += block-sha1/sha1.o
-else
ifdef PPC_SHA1
SHA1_HEADER = "ppc/sha1.h"
LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
else
- SHA1_HEADER = <openssl/sha.h>
- EXTLIBS += $(LIB_4_CRYPTO)
-endif
+ SHA1_HEADER = "block-sha1/sha1.h"
+ LIB_OBJS += block-sha1/sha1.o
endif
+
ifdef NO_PERL_MAKEMAKER
export NO_PERL_MAKEMAKER
endif
diff --git a/cache.h b/cache.h
index dd7f71e..39a4edd 100644
--- a/cache.h
+++ b/cache.h
@@ -6,12 +6,6 @@
#include "hash.h"
#include SHA1_HEADER
-#ifndef git_SHA_CTX
-#define git_SHA_CTX SHA_CTX
-#define git_SHA1_Init SHA1_Init
-#define git_SHA1_Update SHA1_Update
-#define git_SHA1_Final SHA1_Final
-#endif
#include <zlib.h>
#if defined(NO_DEFLATE_BOUND) || ZLIB_VERNUM < 0x1200
diff --git a/configure.ac b/configure.ac
index b09b8e4..634186c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -160,7 +160,6 @@ AC_MSG_NOTICE([CHECKS for site configuration])
# a bundled SHA1 routine optimized for PowerPC.
#
# Define NO_OPENSSL environment variable if you do not have OpenSSL.
-# This also implies BLK_SHA1.
#
# Define OPENSSLDIR=/foo/bar if your openssl header and library files are in
# /foo/bar/include and /foo/bar/lib directories.
^ permalink raw reply related
* Re: Possible regression: overwriting untracked files in a fresh repo
From: Jeff King @ 2009-08-25 3:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vk50sve00.fsf@alter.siamese.dyndns.org>
On Mon, Aug 24, 2009 at 07:11:43PM -0700, Junio C Hamano wrote:
> This looks a lot saner; I like it. Care to wrap it up with the usual
> supporting material?
Patch is below. I did a little digging to see the origin of the
current behavior. It looks like invoking "force" was probably the
easiest way to make the shell version work, and then the behavior just
got ported to C (see below for details).
> I think the "You appear to be" can just go, but I do not feel very
> strongly either way.
I don't feel strongly, either. I couldn't think of a reason a user would
care, and it is potentially confusing to new users (though most will
simply clone and never see it), so I ripped it out.
And now you can try your new scissors patch. ;)
-- >8 --
Subject: [PATCH] checkout: do not imply "-f" on unborn branches
When checkout sees that HEAD points to a non-existent ref,
it currently acts as if "-f" was given; this behavior dates
back to 5a03e7f, which enabled checkout from unborn branches
in the shell version of "git-checkout". The reasoning given
is to avoid the code path which tries to merge the tree
contents. When checkout was converted to C, this code
remained intact.
The unfortunate side effect of this strategy is that the
"force" code path will overwrite working tree and index
state that may be precious to the user. Instead of enabling
"force", this patch uses the normal "merge" codepath for an
unborn branch, but substitutes the empty tree for the "old"
commit.
This means that in the absence of an index, any files in the
working tree will be treated as untracked files, and a
checkout which would overwrite them is aborted. Similarly,
any paths in the index will be merged with an empty entry
as the base, meaning that unless the new branch's content is
identical to what's in the index, there will be a conflict
and the checkout will be aborted.
The user is then free to correct the situation or proceed
with "-f" as appropriate.
This patch also removes the "warning: you are on a branch
yet to be born" message. Its function was to warn the user
that we were enabling the "-f" option. Since we are no
longer doing that, there is no reason for the user to care
whether we are switching away from an unborn branch.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin-checkout.c | 12 +++---------
t/t2015-checkout-unborn.sh | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 43 insertions(+), 9 deletions(-)
create mode 100755 t/t2015-checkout-unborn.sh
diff --git a/builtin-checkout.c b/builtin-checkout.c
index 8a9a474..c6d6ac9 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -402,7 +402,9 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.dir = xcalloc(1, sizeof(*topts.dir));
topts.dir->flags |= DIR_SHOW_IGNORED;
topts.dir->exclude_per_dir = ".gitignore";
- tree = parse_tree_indirect(old->commit->object.sha1);
+ tree = parse_tree_indirect(old->commit ?
+ old->commit->object.sha1 :
+ (unsigned char *)EMPTY_TREE_SHA1_BIN);
init_tree_desc(&trees[0], tree->buffer, tree->size);
tree = parse_tree_indirect(new->commit->object.sha1);
init_tree_desc(&trees[1], tree->buffer, tree->size);
@@ -541,14 +543,6 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
parse_commit(new->commit);
}
- if (!old.commit && !opts->force) {
- if (!opts->quiet) {
- warning("You appear to be on a branch yet to be born.");
- warning("Forcing checkout of %s.", new->name);
- }
- opts->force = 1;
- }
-
ret = merge_working_tree(opts, &old, new);
if (ret)
return ret;
diff --git a/t/t2015-checkout-unborn.sh b/t/t2015-checkout-unborn.sh
new file mode 100755
index 0000000..c551d39
--- /dev/null
+++ b/t/t2015-checkout-unborn.sh
@@ -0,0 +1,40 @@
+#!/bin/sh
+
+test_description='checkout from unborn branch protects contents'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ mkdir parent &&
+ (cd parent &&
+ git init &&
+ echo content >file &&
+ git add file &&
+ git commit -m base
+ ) &&
+ git fetch parent master:origin
+'
+
+test_expect_success 'checkout from unborn preserves untracked files' '
+ echo precious >expect &&
+ echo precious >file &&
+ test_must_fail git checkout -b new origin &&
+ test_cmp expect file
+'
+
+test_expect_success 'checkout from unborn preserves index contents' '
+ echo precious >expect &&
+ echo precious >file &&
+ git add file &&
+ test_must_fail git checkout -b new origin &&
+ test_cmp expect file &&
+ git show :file >file &&
+ test_cmp expect file
+'
+
+test_expect_success 'checkout from unborn merges identical index contents' '
+ echo content >file &&
+ git add file &&
+ git checkout -b new origin
+'
+
+test_done
--
1.6.4.1.330.g14cea.dirty
^ permalink raw reply related
* Re: gitosis-lite
From: Sitaram Chamarty @ 2009-08-25 3:00 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Jakub Narebski, Git Mailing List, Tommi Virtanen
In-Reply-To: <20090824151051.GB1033@spearce.org>
On Mon, Aug 24, 2009 at 8:40 PM, Shawn O. Pearce<spearce@spearce.org> wrote:
> Sitaram Chamarty <sitaramc@gmail.com> wrote:
>> I'll be honest: I
>> came away feeling very stupid after trying to read and
>> understand that program. It was... humbling :(
> *sigh* That's not good, the hook is meant as a practical example,
> if it was too complex to understand, I did a poor job of writing it.
Hmmm no. It was just doing too much to grasp in one
reading, especially by someone whose perl seems to have
rusted a wee bit, if you've seen Jakub's reply :)
I took a slightly longer look after your mail and I grokked
it a lot better. And wow... I guess you called it
"paranoid" for a reason :-)
In the environments I'm catering to, every commit will be
reviewed by someone else (usually someone more experienced,
and having a bigger picture), so "you're not allowed to
touch this file" type of thing will all come out.
All I need is to make sure the important branches can only
be pushed to by certain people; the "process" will take care
of the rest.
^ 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