* Re: [PATCH] Remove redundant assignments
From: Junio C Hamano @ 2011-11-27 8:26 UTC (permalink / raw)
To: Jan Stępień; +Cc: git
In-Reply-To: <1322381142-20645-1-git-send-email-jstepien@users.sourceforge.net>
Jan Stępień <jstepien@users.sourceforge.net> writes:
> There were two redundant variable assignments in transport.c and
> wt-status.c. Removing them hasn't introduced any compiler warnings or
> regressions.
It may not have added any warnings for _you_.
These "type a = a;" initializations (they are not even assignments and
would not result in any code in the output for sane compilers) are idioms
to squelch "unused" warnings from versions of compiler whose data flow
analysis is less than perfect by telling them that the author of the
section of the code knows what s/he is doing.
At least, that is why these pseudo initializations were added in these
codepaths. I personally feel that they should be made into useless but
real assignments (which may result in a useless assignment emited in the
resulting object code) rather than removing like your patch did which
unfortunately forces people with imperfect compilers to live with the
warnings, if one finds them ugly.
> Signed-off-by: Jan Stępień <jstepien@users.sourceforge.net>
> ---
> transport.c | 2 +-
> wt-status.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 51814b5..5143718 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -105,7 +105,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
> return;
>
> for (;;) {
> - int cmp = cmp, len;
> + int cmp, len;
>
> if (!fgets(buffer, sizeof(buffer), f)) {
> fclose(f);
> diff --git a/wt-status.c b/wt-status.c
> index 70fdb76..35f61f4 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -229,7 +229,7 @@ static void wt_status_print_change_data(struct wt_status *s,
> {
> struct wt_status_change_data *d = it->util;
> const char *c = color(change_type, s);
> - int status = status;
> + int status;
> char *one_name;
> char *two_name;
> const char *one, *two;
^ permalink raw reply
* [PATCH] Remove redundant assignments
From: Jan Stępień @ 2011-11-27 8:05 UTC (permalink / raw)
To: git; +Cc: Jan Stępień
There were two redundant variable assignments in transport.c and
wt-status.c. Removing them hasn't introduced any compiler warnings or
regressions.
Signed-off-by: Jan Stępień <jstepien@users.sourceforge.net>
---
transport.c | 2 +-
wt-status.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/transport.c b/transport.c
index 51814b5..5143718 100644
--- a/transport.c
+++ b/transport.c
@@ -105,7 +105,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
return;
for (;;) {
- int cmp = cmp, len;
+ int cmp, len;
if (!fgets(buffer, sizeof(buffer), f)) {
fclose(f);
diff --git a/wt-status.c b/wt-status.c
index 70fdb76..35f61f4 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -229,7 +229,7 @@ static void wt_status_print_change_data(struct wt_status *s,
{
struct wt_status_change_data *d = it->util;
const char *c = color(change_type, s);
- int status = status;
+ int status;
char *one_name;
char *two_name;
const char *one, *two;
--
1.7.7.4
^ permalink raw reply related
* Re: what are the chances of a 'pre-upload' hook?
From: Junio C Hamano @ 2011-11-27 7:51 UTC (permalink / raw)
To: Jeff King; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <20111126233133.GA31129@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It depends on what you want your hook to do. I thought Sitaram's
> use-case was putting something like "git fetch upstream" into a hook
> that runs before upload-pack, to create a transparent proxy. That has
> nothing to do with the accessing user.
>
> At GitHub, we run a pre-upload-pack hook to keep statistics. That has
> nothing to do with the accessing user. We have to patch git to provide
> the hook.
I personally have a huge problem with shipping an inherently unsafe
mechanism that is disabled by default even if it is marked with big red
letters saying that it is unsafe and should not be enabled casually. It
makes it up to the system administrator to decide what is casual and what
is not, but when end users are get harmed by a clueless administrator's
decision, the repercussion will come back to the Git software, not to the
clueless administrator who enabled an unsafe mechanism in an environment
where it was not designed to support.
It may merely be a matter of perception. After all, the pre-upload-pack
hook I did in a8563ec (upload-pack: add a trigger for post-upload-pack
hook, 2009-08-26) in response to GitHub's stat request was so trivial that
even clueless admins could trojan it back into the current upload-pack
source to use in their own site, without letting their users know that
their fetch request may be opening their account to a random hook
script. So in that sense, it might not be worth my effort to fight for Git
users' safety to find a better solution and instead go with the big red
letter approach which is easy. I know perfect is an enemy of good, but
when I do not think an easy Quick-and-Dirty solution is even less than
good, I need to point out that not having less than bad is better than
having it.
If the mechanism to notify the external machinery (i.e. counting accesses,
learning the true destination of a new fetch request and have the fetcher
wait while the real fetch goes to the origin site) were not via a hook
that runs as the fetcher but were via something else that runs as the
owner of that external service (i.e. counting accesses, maintaining the
proxy object pool), I wouldn't have trouble with the proposal.
For example, upload-pack could write the details of the original request
to a named pipe to ask the "service" process listening on the other end,
and wait for its response.
^ permalink raw reply
* gitweb: in-page errors don't work with mod_perl
From: Jürgen Kreileder @ 2011-11-27 4:05 UTC (permalink / raw)
To: git
Hi
when gitweb.perl (208a1cc3d3) is run with mod_perl2 (2.0.5-2ubuntu1 on
Ubuntu 11.10) custom error pages don't work: Any page with status !=
200 shows the plain Apache error document instead of a gitweb's error
page.
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* gitweb: 404 links on some blob pages
From: Jürgen Kreileder @ 2011-11-27 4:03 UTC (permalink / raw)
To: git
Hi,
some blob pages have broken links:
For example, on
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.m;h=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
the blob_plain link for WindowController.m leads to '404 - Cannot find file':
https://git.blackdown.de/?p=contactalbum.git;a=blob_plain;f=Classes/WindowController.m;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
And the tree link for "[contactalbum.git]" leads to '404 - Reading tree failed':
https://git.blackdown.de/?p=contactalbum.git;a=tree;hb=b84d1882cb6c3a2d2058cbdd56b2280b48f1690a
Here's a page from git.kernel.org which shows exactly the same problem:
http://git.kernel.org/?p=linux/kernel/git/next/linux-next-history.git;a=blob;f=drivers/Makefile;h=91077ac6b1564a21449a155cde1b84d6678d6e13;hb=91077ac6b1564a21449a155cde1b84d6678d6e13
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* gitweb: broken links with pathinfo
From: Jürgen Kreileder @ 2011-11-27 4:02 UTC (permalink / raw)
To: git
Hi,
if pathinfo (gitweb.perl 208a1cc3d3) is enabled, some pages lead to
404 errors. For example, on
https://git.blackdown.de/contactalbum.git/blobdiff/c7fdc45614bd127581852ce429c4483b1d21c0d4..82f7c8babf6095224bf5d8ab04f6eea4b1a555ee:/Classes/WindowController.h
a/Classes/WindowController.h ->
https://git.blackdown.de/contactalbum.git/blob/3c27189e8adc690f421334a9cad7ad719c066eb4:/Classes/WindowController.h
b/Classes/WindowController.h ->
https://git.blackdown.de/contactalbum.git/blob/f285b2728d2c7782806f0b7f6d696b226a88f03c:/Classes/WindowController.h
Both links give '404 - Cannot find file'
With pathinfo disabled, I get the following working URLs intead:
a/Classes/WindowController.h ->
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.h;h=3c27189e8adc690f421334a9cad7ad719c066eb4;hb=3c27189e8adc690f421334a9cad7ad719c066eb4
b/Classes/WindowController.h ->
https://git.blackdown.de/?p=contactalbum.git;a=blob;f=Classes/WindowController.h;h=f285b2728d2c7782806f0b7f6d696b226a88f03c;hb=f285b2728d2c7782806f0b7f6d696b226a88f03c
Jürgen
--
http://blog.blackdown.de/
http://www.flickr.com/photos/jkreileder/
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-27 0:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git, Sitaram Chamarty
In-Reply-To: <CAPc5daUodry_=6pZxA=QOpuRUj9C2ed9Gzp6E1_G93iGfOOvOA@mail.gmail.com>
On Sat, Nov 26, 2011 at 03:57:40PM -0800, Junio C Hamano wrote:
> Did I say anything about saNe?. I was talking about saFe.
Fine. But that doesn't change my point: the purpose of such a feature is
to tell git "do _not_ be safe; I have decided already for you whether it
is OK to do this".
> > By turning it on, you
> > are saying "it's OK to run arbitrary code from the repo as the current
> > user".
>
> The problem I have with it is that you are saying much more than that.
> ... as the current user ANYWHERE on the machine.
Just because it is passed through the environment does not mean you need
to have it set all the time. There is nothing wrong with:
GIT_ALLOW_UNTRUSTED_HOOKS=true git fetch ~bob/repo.git
We can even spell it:
git --allow-untrusted-hooks fetch ~bob/repo.git
but it should probably still end up as an environment variable to make
it through to the remote side (you could also tack it on to the
upload-pack command line; that wouldn't make it across git:// or http://
connections, but those are irrelevant here anyway).
-Peff
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-26 23:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sitaram Chamarty, Git
In-Reply-To: <CAPc5daXY_4aimugj8Z4BFE8YvBSM1K+evPU69rLGH5ETo6PO=Q@mail.gmail.com>
On Sat, Nov 26, 2011 at 03:47:09PM -0800, Junio C Hamano wrote:
> > My point is to make it available, give it safe
> > semantics by default, and let people who are running daemon-like service
> > (i.e., where the admin controls the daemon and arbitrary users can't
> > write into the hooks directory) use it by setting an environment
> > variable, rather than patching git.
>
> I think we re on the same page on that point, and this thread is to find
> such a safe default and safe semantics when enabled.
>
> Unfortunately neither your "trusted" switch nor check the gid of repository
> is that safe thing (sane default part is easy; do not allow it by default).
Sorry, why is the trusted switch not a sane thing? By turning it on, you
are saying "it's OK to run arbitrary code from the repo as the current
user". It's _exactly_ what some people are going to want to do[1],
regardless of any other heuristics.
Sure, maybe it's giving people rope to hang themselves with, but I don't
see a problem with that as long as the issues are clearly laid out in
the documentation.
-Peff
[1] An alternate and even more flexible form is to not just say "it's OK
to run hooks", but to say "run this particular hook as a
pre-upload-hook" without any regard for what's in $GIT_DIR/hooks. It is
a superset of the other form, because of course the hook you tell it
to run can be "sh $GIT_DIR/hooks/pre-upload-pack".
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-26 23:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <7vr50uwk7x.fsf@alter.siamese.dyndns.org>
On Sat, Nov 26, 2011 at 03:13:38PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Bob could run a specialized server for (b) that listens on a unix socket
> > and triggers his hook. But why? Why not just do the whole thing over
> > git-daemon or smart http, which already exist?
>
> If that "whole thing" is "to allow an arbitrary code to run anywhere as
> incoming user", I would apply the "why?" to a different part of the
> statemennt. Why allow running an arbitrary code at all?
Because there are useful hooks that Bob wants to run when somebody
fetches or pushes to/from his repository?
> A pre-anything hook wants to see if the accessing user, not the owner of
> the repository, can or cannot do something to the repository and decide
> what to do.
It depends on what you want your hook to do. I thought Sitaram's
use-case was putting something like "git fetch upstream" into a hook
that runs before upload-pack, to create a transparent proxy. That has
nothing to do with the accessing user.
At GitHub, we run a pre-upload-pack hook to keep statistics. That has
nothing to do with the accessing user. We have to patch git to provide
the hook.
And even if you _did_ want to know something about the accessing user,
that doesn't mean you necessarily can or want to be running code as
them. If I'm running a smart-http server, I might have verified the user
already via cookie, client cert, or basic auth. That information could
be passed down to a pre-upload-pack hook where it could make a decision.
But we don't _have_ a pre-upload-pack hook, because it can be dangerous
in some situations. My point is to make it available, give it safe
semantics by default, and let people who are running daemon-like service
(i.e., where the admin controls the daemon and arbitrary users can't
write into the hooks directory) use it by setting an environment
variable, rather than patching git.
-Peff
^ permalink raw reply
* Re: git branch -M" regression in 1.7.7?
From: Andreas Schwab @ 2011-11-26 23:09 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Conrad Irwin,
☂Josh Chia (谢任中), git,
Soeren Sonnenburg
In-Reply-To: <7v39day0fb.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> I havn't look at the patch (not a regression between 1.7.7 and 1.7.8 so
> not a candidate for the remainder of this cycle), but I like the above
> description quite a lot. I think Linus's "git reset --sane" which was
> initially called "git reset --merge" but ended up as "git reset --keep"
> should have been spelled as "checkout -B <current-branch>" from the
> beginning.
It is more convenient if you don't have to spell out the name of the
current branch (which fails if you aren't on a branch).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Junio C Hamano @ 2011-11-26 23:13 UTC (permalink / raw)
To: Jeff King; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <20111126225519.GA29482@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Bob could run a specialized server for (b) that listens on a unix socket
> and triggers his hook. But why? Why not just do the whole thing over
> git-daemon or smart http, which already exist?
If that "whole thing" is "to allow an arbitrary code to run anywhere as
incoming user", I would apply the "why?" to a different part of the
statemennt. Why allow running an arbitrary code at all?
Running things as Bob with setuid is not a solution, either.
A pre-anything hook wants to see if the accessing user, not the owner of
the repository, can or cannot do something to the repository and decide
what to do.
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Jeff King @ 2011-11-26 22:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <7v7h2my0ky.fsf@alter.siamese.dyndns.org>
On Sat, Nov 26, 2011 at 02:34:53PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > The easiest way would be something like a "trust remote hooks"
> > environment variable, off by default. Admins in situation (2) could set
> > it for their git-daemon (or webserver, or whatever, or
> > gitolite-over-ssh), once they decided it was safe.
>
> Alice and Bob may work on the same project, and they may want to trust
> each other as participants of that project, but that does not mean Alice
> wants to give Bob a blanket access to places she owns that are not shared
> with the project members (e.g. her $HOME directory), so I am afraid "trust
> remote hooks" is not a workable solution for the casual sharing on sites
> that do not fall into either of your two classes.
Of course. My point isn't that such a feature would cover all cases, but
that it would give people a tool to make that decision for themselves,
instead of blanket-forbidding it.
> The real reason why the upload-hook violates the expectation of the users
> is because it would run as the user who fetches, I think. If it ran with
> the intersection of capabilities of the owner of the repository and the
> user who is fetching, I suspect that we would not have to worry about it.
Sure. And I think we would probably trust automatically a hook that is
owned by the executing uid. Or possibly root, though that could be
surprising in NFS root-squashed environments.
> What would happen if we allowed some hooks to run only when the process is
> running under a group-id that can write into the repository? When Alice
> fetches from the repository, it would still run as her and would have an
> access to her $HOME, so this won't really work yet, but I am wondering if
> there is a workable alternative along this line.
I don't think so. It comes down to this: Alice is executing arbitrary
code from Bob's repository, which Bob (and maybe others) have access to
write. This is giving Bob permission to run arbitrary code as Alice's
user.
Checking things like group access doesn't matter. If Alice is in the
group, too, it doesn't make a difference; Bob can still write the files,
and Alice is still running the code under her UID.
I think the only solutions are:
1. Alice accepts that she can trust Bob enough to run his arbitrary
code.
2. We use some technique to run the code as the repo owner's UID.
The usual methods for doing (2) are:
a. setuid, though doing it right can be quite tricky (e.g., cleansing
environment, file descriptors, etc). And it requires root
cooperation.
b. running a server with a socket as Bob, and triggering the hook code
from the server
Bob could run a specialized server for (b) that listens on a unix socket
and triggers his hook. But why? Why not just do the whole thing over
git-daemon or smart http, which already exist?
-Peff
^ permalink raw reply
* Re: Infinite loop in cascade_filter_fn()
From: Junio C Hamano @ 2011-11-26 22:48 UTC (permalink / raw)
To: Carlos Martín Nieto
Cc: Henrik Grubbström, Git Mailing list, Junio C Hamano
In-Reply-To: <20111125170219.GD10417@beez.lab.cmartin.tk>
Carlos Martín Nieto <cmn@elego.de> writes:
> diff --git a/convert.c b/convert.c
> index 86e9c29..c050b86 100644
> --- a/convert.c
> +++ b/convert.c
> @@ -880,20 +880,29 @@ static int lf_to_crlf_filter_fn(struct stream_filter *filter,
> const char *input, size_t *isize_p,
> char *output, size_t *osize_p)
> {
> - size_t count;
> + size_t count, o = 0;
> + static int want_lf = 0;
I do not think we want function scope static state anywhere in the cascade
filter chain, as it will forbid us from running more than one output chain
at the same time in the future. I think the correct way to structure it
would be to create lf_to_crlf_filter as a proper subclass of stream_filter
(see how cascade_filter_fn() casts its filter argument down to an instance
of the cascade_filter class and uses it to keep track of its state) and
keep this variable as its own filter state [*1*].
[Footnote]
*1* We currently use a singleton instance of lf_to_crlf_filter object
because the implementation assumed there is no need for per-instance
state.
^ permalink raw reply
* Re: git branch -M" regression in 1.7.7?
From: Junio C Hamano @ 2011-11-26 22:38 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Conrad Irwin, ☂Josh Chia (谢任中), git,
Soeren Sonnenburg
In-Reply-To: <20111126085455.GB22656@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> In other words, when on master, "git checkout -B master <commit>"
> would be another way to say "git reset --keep <commit>", except that
> it also sets up tracking.
I havn't look at the patch (not a regression between 1.7.7 and 1.7.8 so
not a candidate for the remainder of this cycle), but I like the above
description quite a lot. I think Linus's "git reset --sane" which was
initially called "git reset --merge" but ended up as "git reset --keep"
should have been spelled as "checkout -B <current-branch>" from the
beginning.
^ permalink raw reply
* Re: what are the chances of a 'pre-upload' hook?
From: Junio C Hamano @ 2011-11-26 22:34 UTC (permalink / raw)
To: Jeff King; +Cc: Sitaram Chamarty, Git Mailing List
In-Reply-To: <20111125144007.GA4047@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The easiest way would be something like a "trust remote hooks"
> environment variable, off by default. Admins in situation (2) could set
> it for their git-daemon (or webserver, or whatever, or
> gitolite-over-ssh), once they decided it was safe.
Alice and Bob may work on the same project, and they may want to trust
each other as participants of that project, but that does not mean Alice
wants to give Bob a blanket access to places she owns that are not shared
with the project members (e.g. her $HOME directory), so I am afraid "trust
remote hooks" is not a workable solution for the casual sharing on sites
that do not fall into either of your two classes.
The real reason why the upload-hook violates the expectation of the users
is because it would run as the user who fetches, I think. If it ran with
the intersection of capabilities of the owner of the repository and the
user who is fetching, I suspect that we would not have to worry about it.
What would happen if we allowed some hooks to run only when the process is
running under a group-id that can write into the repository? When Alice
fetches from the repository, it would still run as her and would have an
access to her $HOME, so this won't really work yet, but I am wondering if
there is a workable alternative along this line.
^ permalink raw reply
* What to do if the path of my git submodules change upstream
From: Harald Heigl @ 2011-11-26 16:45 UTC (permalink / raw)
To: git
> Hi everyone!
>
> First setup of my git was a server (with ssh) and some clients.
>
> Today I changed to gitolite because I wanted a more sophisticated way of
> managing my repos. So far so good…
> So the old path “ssh://[ip]/[fullpath].git” would change to a new path
> “git@[servername]:[gitreponame]”.
> This is no problem for “normal” repos, I change the remote origin and
> continue using push and pull.
>
> I have some submodules:
> I changed the .gitmodules to reflect my changes, did a git submodule sync.
> This works flawlessly too!
>
> But what if someone wants to checkout an older version of the project?
(for
> comparison, or because he/she wanted to try something out)
> He would get an old .gitmodules with old paths.
> After a git submodule sync he would get errors, because old paths won’t
> work anymore, because I changed some paths on the server
>
> It is only one project I have this problem and therein I changed the
> .gitmodules only 3 times. Is it possible to rewrite .gitmodules on these
> specific commits on the server (perhaps with git-filter-branch)?
> Or is there another easy solution? Has someone ever had this problem?
>
> Hope you can help,
> Kind regards,
> Harald Heigl
Hi everyone!
I try to answer myself, I found a possibility to change the file
".gitmodules" in my git history:
---------------
Method 1 with some git plumbing:
cd [gitrepo]
cd ..
[prepare my "new" .gitmodules-File here]
cd [gitrepo]
git hash-object -w ../.gitmodules
git filter-branch --index-filter 'git rm --cached --ignore-unmatch
.gitmodules;git update-index --add --cacheinfo 100644 \
3e0f4ab4126e01d55fda040234e3593aea1bff79 .gitmodules'
022d9c5fecb4d6c268365dfe186aa65389bcd492^..
Push this new branch to a remote repo and clone from there again (so I'll
have a sober dir, without the old branches)
More or less this adds a new object to git (with SHA1 3e0f4ab in my case)
and then rewrites every index to remove my old .gitmodules and add the newly
entered .gitmodules.
The only "problem" is that this will rewrite all Commits and therefore also
the SHA1 which won't match the one on the server.
As my understanding in git is now: After a change of the (already pushed)
commits I'll have to delete the remote repo, push my changed repo upstream
and then everyone in the team will have to clone this new repo. Good that
this project affects only a small group in my company.
------------
Method 2 with git rebase -i:
git rebase -i 022d9c5fecb4d6c268365dfe186aa65389bcd492^
This will rebase anything and I can rewrite my file on the specific commits.
But this will change my whole history (and SHAs too). If there are some
merges in these commit the rebase would flatten anything. This wouldn't
disturb that way, but leads to conflicts I have to resolve on the rebase
side for every commit that conflicts.
------------
I'll think over it, if I use Method 1 or don't change anything. Perhaps the
latter would be the best :-)
Hope this helps others,
Kind regards
^ permalink raw reply
* Re: gitweb ignores git-daemon-export-ok and displays all repositories
From: Jakub Narebski @ 2011-11-26 14:37 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Erik Wenzel, John 'Warthog9' Hawley,
Matthias Lederhofer, Sitaram Chamarty
In-Reply-To: <20111116224706.GA17851@elie.hsd1.il.comcast.net>
I'm sorry for the delay in responding.
On Wed, 16 Nov 2011, Jonathan Nieder wrote:
> Erik Wenzel wrote (http://bugs.debian.org/648561):
>> Am 13.11.2011 um 02.19 schrieb Jonathan Nieder:
>>
>>> The git-daemon(1) manpage describes git daemon, not gitweb, so I guess
>>> you mean that
>>>
>>> # Do not list projects without git-daemon-export-ok in the
>>> # projects list.
>>> our $export_ok = "git-daemon-export-ok";
>>>
>>> # Do not allow access to projects not listed in the projects
>>> # list.
>>> our $strict_export = 1;
>>>
>>> should be the default.
>> [...]
>> Because I think this is the way a user is expecting the behavior of gitweb.
>> As I do. Don't let gitweb overwrite the meaning of "git-daemon-export-ok".
>> Just act like git-daemon. Keep it simple and stupid.
>
> My first thought was that if we could turn back time, it would
> probably be best for both git-daemon and gitweb to look for a file
> named git-export-ok. In the present world, maybe git-daemon-export-ok
> is a good substitute for that.
>
> What do you think? Should the default in the makefile be changed? If
> so, how could we go about it to avoid disturbing existing
> installations? (For example, in a system where no repositories have
> the export-ok files and "git daemon" is configured to run with
> --export-all, the effect would be to make repos suddenly disappear
> from the projects list in gitweb. Unpleasant.)
I think the best solution would be to leave it up to the tool that
manages both git-daemon and git (manages access to git repositories),
like e.g. gitolite. It would be this tool that is to be responsible
for synchronizing git-daemon and gitweb behavior, i.e. make either
both use 'git-daemon-export-ok', or both export all.
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH] grep: enable multi-threading for -p and -W
From: René Scharfe @ 2011-11-26 12:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <4ECFC320.4030003@lsrfire.ath.cx>
Move attribute reading, which is not thread-safe, into add_work(), under
grep_mutex. That way we can stop turning off multi-threading if -p or -W
is given, as the diff attribute for each file is gotten safely now.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Goes on top of your patch. Needs testing..
builtin/grep.c | 33 +++++++++++++++++++++++++--------
grep.c | 25 +++++++------------------
grep.h | 5 +++--
revision.c | 2 +-
4 files changed, 36 insertions(+), 29 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 3d7329d..5534111 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -18,6 +18,7 @@
#include "quote.h"
#include "dir.h"
#include "thread-utils.h"
+#include "userdiff.h"
static char const * const grep_usage[] = {
"git grep [options] [-e] <pattern> [<rev>...] [[--] <path>...]",
@@ -43,6 +44,7 @@ enum work_type {WORK_SHA1, WORK_FILE};
struct work_item {
enum work_type type;
char *name;
+ struct userdiff_driver *userdiff_driver;
/* if type == WORK_SHA1, then 'identifier' is a SHA1,
* otherwise type == WORK_FILE, and 'identifier' is a NUL
@@ -114,7 +116,17 @@ static pthread_cond_t cond_result;
static int skip_first_line;
-static void add_work(enum work_type type, char *name, void *id)
+/* Reading attributes is not thread-safe, so callers need to lock. */
+static struct userdiff_driver *get_userdiff_driver(struct grep_opt *opt,
+ const char *name)
+{
+ if (opt->funcbody || opt->funcname)
+ return userdiff_find_by_path(name);
+ return NULL;
+}
+
+static void add_work(struct grep_opt *opt, enum work_type type, char *name,
+ void *id)
{
grep_lock();
@@ -124,6 +136,7 @@ static void add_work(enum work_type type, char *name, void *id)
todo[todo_end].type = type;
todo[todo_end].name = name;
+ todo[todo_end].userdiff_driver = get_userdiff_driver(opt, name);
todo[todo_end].identifier = id;
todo[todo_end].done = 0;
strbuf_reset(&todo[todo_end].out);
@@ -158,13 +171,13 @@ static void grep_sha1_async(struct grep_opt *opt, char *name,
unsigned char *s;
s = xmalloc(20);
memcpy(s, sha1, 20);
- add_work(WORK_SHA1, name, s);
+ add_work(opt, WORK_SHA1, name, s);
}
static void grep_file_async(struct grep_opt *opt, char *name,
const char *filename)
{
- add_work(WORK_FILE, name, xstrdup(filename));
+ add_work(opt, WORK_FILE, name, xstrdup(filename));
}
static void work_done(struct work_item *w)
@@ -212,24 +225,26 @@ static void *run(void *arg)
struct grep_opt *opt = arg;
while (1) {
+ struct userdiff_driver *drv;
struct work_item *w = get_work();
if (!w)
break;
opt->output_priv = w;
+ drv = w->userdiff_driver;
if (w->type == WORK_SHA1) {
unsigned long sz;
void* data = load_sha1(w->identifier, &sz, w->name);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, drv, data, sz);
free(data);
}
} else if (w->type == WORK_FILE) {
size_t sz;
void* data = load_file(w->identifier, &sz);
if (data) {
- hit |= grep_buffer(opt, w->name, data, sz);
+ hit |= grep_buffer(opt, w->name, drv, data, sz);
free(data);
}
} else {
@@ -417,10 +432,11 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
int hit;
unsigned long sz;
void *data = load_sha1(sha1, &sz, name);
+ struct userdiff_driver *drv = get_userdiff_driver(opt, name);
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, drv, data, sz);
free(data);
free(name);
@@ -479,10 +495,11 @@ static int grep_file(struct grep_opt *opt, const char *filename)
int hit;
size_t sz;
void *data = load_file(filename, &sz);
+ struct userdiff_driver *drv = get_userdiff_driver(opt, name);
if (!data)
hit = 0;
else
- hit = grep_buffer(opt, name, data, sz);
+ hit = grep_buffer(opt, name, drv, data, sz);
free(data);
free(name);
@@ -1001,7 +1018,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
opt.regflags |= REG_ICASE;
#ifndef NO_PTHREADS
- if (online_cpus() == 1 || !grep_threads_ok(&opt))
+ if (online_cpus() == 1)
use_threads = 0;
if (use_threads) {
diff --git a/grep.c b/grep.c
index 7a070e9..ff14a98 100644
--- a/grep.c
+++ b/grep.c
@@ -942,25 +942,13 @@ static int look_ahead(struct grep_opt *opt,
return 0;
}
-int grep_threads_ok(const struct grep_opt *opt)
-{
- /* If this condition is true, then we may use the attribute
- * machinery in grep_buffer_1. The attribute code is not
- * thread safe, so we disable the use of threads.
- */
- if ((opt->funcname || opt->funcbody)
- && !opt->unmatch_name_only && !opt->status_only && !opt->name_only)
- return 0;
-
- return 1;
-}
-
static void std_output(struct grep_opt *opt, const void *buf, size_t size)
{
fwrite(buf, size, 1, stdout);
}
static int grep_buffer_1(struct grep_opt *opt, const char *name,
+ struct userdiff_driver *drv,
char *buf, unsigned long size, int collect_hits)
{
char *bol = buf;
@@ -1011,7 +999,6 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
if ((opt->funcname || opt->funcbody)
&& !opt->unmatch_name_only && !opt->status_only &&
!opt->name_only && !binary_match_only && !collect_hits) {
- struct userdiff_driver *drv = userdiff_find_by_path(name);
if (drv && drv->funcname.pattern) {
const struct userdiff_funcname *pe = &drv->funcname;
xdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);
@@ -1167,23 +1154,25 @@ static int chk_hit_marker(struct grep_expr *x)
}
}
-int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size)
+int grep_buffer(struct grep_opt *opt, const char *name,
+ struct userdiff_driver *userdiff_driver,
+ char *buf, unsigned long size)
{
/*
* we do not have to do the two-pass grep when we do not check
* buffer-wide "all-match".
*/
if (!opt->all_match)
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, userdiff_driver, buf, size, 0);
/* Otherwise the toplevel "or" terms hit a bit differently.
* We first clear hit markers from them.
*/
clr_hit_marker(opt->pattern_expression);
- grep_buffer_1(opt, name, buf, size, 1);
+ grep_buffer_1(opt, name, userdiff_driver, buf, size, 1);
if (!chk_hit_marker(opt->pattern_expression))
return 0;
- return grep_buffer_1(opt, name, buf, size, 0);
+ return grep_buffer_1(opt, name, userdiff_driver, buf, size, 0);
}
diff --git a/grep.h b/grep.h
index a652800..20224b5 100644
--- a/grep.h
+++ b/grep.h
@@ -121,14 +121,15 @@ struct grep_opt {
void *output_priv;
};
+struct userdiff_driver;
+
extern void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);
extern void append_grep_pattern(struct grep_opt *opt, const char *pat, const char *origin, int no, enum grep_pat_token t);
extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const char *);
extern void compile_grep_patterns(struct grep_opt *opt);
extern void free_grep_patterns(struct grep_opt *opt);
-extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size);
+extern int grep_buffer(struct grep_opt *opt, const char *name, struct userdiff_driver *userdiff_driver, char *buf, unsigned long size);
extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);
-extern int grep_threads_ok(const struct grep_opt *opt);
#endif
diff --git a/revision.c b/revision.c
index 8764dde..95cb8c2 100644
--- a/revision.c
+++ b/revision.c
@@ -2126,7 +2126,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
return 1;
return grep_buffer(&opt->grep_filter,
- NULL, /* we say nothing, not even filename */
+ NULL, NULL, /* we say nothing, not even filename */
commit->buffer, strlen(commit->buffer));
}
--
1.7.7
^ permalink raw reply related
* Re: [PATCH] honour GIT_ASKPASS for querying username in git-svn
From: Sven Strickroth @ 2011-11-26 11:33 UTC (permalink / raw)
To: git; +Cc: gitster
In-Reply-To: <CABPQNSbfM0JRVPk3fxfSEq7QaO-fynHM8FBGpPribdgeRqpZKA@mail.gmail.com>
Hi,
there's also another point where git-svn doesn't honour GIT_ASKPASS:
>From 632c264d0de127c35fbe45866ed81e832f357d56 Mon Sep 17 00:00:00 2001
From: Sven Strickroth <email@cs-ware.de>
Date: Sat, 26 Nov 2011 12:01:19 +0100
Subject: [PATCH] honour GIT_ASKPASS for querying further actions on unknown
certificates
git-svn reads user answers from an interactive terminal.
This behavior cause GUIs to hang waiting for git-svn to
complete.
Signed-off-by: Sven Strickroth <email@cs-ware.de>
---
git-svn.perl | 9 +++++++++--
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index e30df22..d7aeb11 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4361,7 +4361,14 @@ prompt:
"(R)eject, accept (t)emporarily or accept (p)ermanently? " :
"(R)eject or accept (t)emporarily? ";
STDERR->flush;
- $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ if (exists $ENV{GIT_ASKPASS}) {
+ print STDERR "\n";
+ open(PH, "-|", $ENV{GIT_ASKPASS}, "Certificate unknown");
+ $choice = lc(substr(<PH> || 'R', 0, 1));
+ close(PH);
+ } else {
+ $choice = lc(substr(<STDIN> || 'R', 0, 1));
+ }
if ($choice =~ /^t$/i) {
$cred->may_save(undef);
} elsif ($choice =~ /^r$/i) {
--
1.7.7.1.msysgit.0
--
Best regards,
Sven Strickroth
ClamAV, a GPL anti-virus toolkit http://www.clamav.net
PGP key id F5A9D4C4 @ any key-server
^ permalink raw reply related
* Re: git branch -M" regression in 1.7.7?
From: Jonathan Nieder @ 2011-11-26 8:54 UTC (permalink / raw)
To: Conrad Irwin
Cc: ☂Josh Chia (谢任中), git,
Soeren Sonnenburg
In-Reply-To: <CAOTq_pv4dyAkbqye+diK9mTTsrTg9OKg0tExKcfDgs8RfiTwTQ@mail.gmail.com>
Conrad Irwin wrote:
> I thought after reading your patch about making it just do:
>
> if (!strcmp(oldname, newname))
> exit(0);
>
> but I guess it would then not mark an entry in the reflog that people
> could be relying on...
Ah, this deserves a comment.
I thought about doing the same thing, and then didn't do it because I
wanted to make sure that
git branch -M nonexistent nonexistent
does not succeed. Which presumably warrants another test:
test_expect_success "rename-to-self dwimery doesn't hide nonexistent ref" '
test_must_fail git branch -M nonexistent nonexistent &&
test_must_fail git rev-parse --verify nonexistent
'
Sloppy of me.
>> + clobber_head_ok = !strcmp(oldname, newname);
>> +
>> + validate_new_branchname(newname, &newref, force, clobber_head_ok);
>
> This looks ok, and will be improvable if the NEEDSWORK in branch.h is done.
Thanks for looking it over.
> The other thing I wonder is whether "git checkout -B master HEAD" or
> "git branch -f master master" should have the same short-cut?
I think "git branch -M" is the only one buildbot was relying on.
As an aside, I'm not convinced the check is needed for checkout -B at
all. In an ideal world, the order of operations would be:
1. look up commit argument
2. merge working tree
3. update branch to match commit
4. update HEAD symref to point to branch
In other words, when on master, "git checkout -B master <commit>"
would be another way to say "git reset --keep <commit>", except that
it also sets up tracking.
Surprisingly, switch_branches() seems to match that pretty well already.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
branch.c | 6 ++++--
branch.h | 3 ++-
builtin/branch.c | 2 +-
builtin/checkout.c | 15 +++++++++++----
t/t2018-checkout-branch.sh | 9 +++++----
5 files changed, 23 insertions(+), 12 deletions(-)
diff --git a/branch.c b/branch.c
index 025a97be..f85c4382 100644
--- a/branch.c
+++ b/branch.c
@@ -160,7 +160,8 @@ int validate_new_branchname(const char *name, struct strbuf *ref,
void create_branch(const char *head,
const char *name, const char *start_name,
- int force, int reflog, enum branch_track track)
+ int force, int reflog, int clobber_head,
+ enum branch_track track)
{
struct ref_lock *lock = NULL;
struct commit *commit;
@@ -175,7 +176,8 @@ void create_branch(const char *head,
explicit_tracking = 1;
if (validate_new_branchname(name, &ref, force,
- track == BRANCH_TRACK_OVERRIDE)) {
+ track == BRANCH_TRACK_OVERRIDE ||
+ clobber_head)) {
if (!force)
dont_change_ref = 1;
else
diff --git a/branch.h b/branch.h
index 1285158d..e125ff4c 100644
--- a/branch.h
+++ b/branch.h
@@ -13,7 +13,8 @@
* branch for (if any).
*/
void create_branch(const char *head, const char *name, const char *start_name,
- int force, int reflog, enum branch_track track);
+ int force, int reflog,
+ int clobber_head, enum branch_track track);
/*
* Validates that the requested branch may be created, returning the
diff --git a/builtin/branch.c b/builtin/branch.c
index 51ca6a02..730f9139 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -730,7 +730,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
if (kinds != REF_LOCAL_BRANCH)
die(_("-a and -r options to 'git branch' do not make sense with a branch name"));
create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
- force_create, reflog, track);
+ force_create, reflog, 0, track);
} else
usage_with_options(builtin_branch_usage, options);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2a807724..ca00a853 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -540,7 +540,9 @@ static void update_refs_for_switch(struct checkout_opts *opts,
else
create_branch(old->name, opts->new_branch, new->name,
opts->new_branch_force ? 1 : 0,
- opts->new_branch_log, opts->track);
+ opts->new_branch_log,
+ opts->new_branch_force ? 1 : 0,
+ opts->track);
new->name = opts->new_branch;
setup_branch_path(new);
}
@@ -565,8 +567,12 @@ static void update_refs_for_switch(struct checkout_opts *opts,
create_symref("HEAD", new->path, msg.buf);
if (!opts->quiet) {
if (old->path && !strcmp(new->path, old->path)) {
- fprintf(stderr, _("Already on '%s'\n"),
- new->name);
+ if (opts->new_branch_force)
+ fprintf(stderr, _("Reset branch '%s'\n"),
+ new->name);
+ else
+ fprintf(stderr, _("Already on '%s'\n"),
+ new->name);
} else if (opts->new_branch) {
if (opts->branch_exists)
fprintf(stderr, _("Switched to and reset branch '%s'\n"), new->name);
@@ -1057,7 +1063,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)
struct strbuf buf = STRBUF_INIT;
opts.branch_exists = validate_new_branchname(opts.new_branch, &buf,
- !!opts.new_branch_force, 0);
+ !!opts.new_branch_force,
+ !!opts.new_branch_force);
strbuf_release(&buf);
}
diff --git a/t/t2018-checkout-branch.sh b/t/t2018-checkout-branch.sh
index 75874e85..27412623 100755
--- a/t/t2018-checkout-branch.sh
+++ b/t/t2018-checkout-branch.sh
@@ -189,12 +189,13 @@ test_expect_success 'checkout -b <describe>' '
test_cmp expect actual
'
-test_expect_success 'checkout -B to the current branch fails before merging' '
+test_expect_success 'checkout -B to the current branch works' '
git checkout branch1 &&
+ git checkout -B branch1-scratch &&
+
setup_dirty_mergeable &&
- git commit -mfooble &&
- test_must_fail git checkout -B branch1 initial &&
- test_must_fail test_dirty_mergeable
+ git checkout -B branch1-scratch initial &&
+ test_dirty_mergeable
'
test_done
--
1.7.8.rc3
^ permalink raw reply related
* [announce] gitpod -- local caching server for git
From: Sitaram Chamarty @ 2011-11-26 8:26 UTC (permalink / raw)
To: Git Mailing List
Hi all,
If you have a bandwidth constrained site with multiple local users all
cloning/fetching the same set of large repos over a slow WAN link,
this may be useful. It can be set up to be accessed via git://,
ssh://, or both.
You can get it from https://github.com/sitaramc/gitpod
--
Sitaram
^ permalink raw reply
* Re: git branch -M" regression in 1.7.7?
From: Conrad Irwin @ 2011-11-26 7:05 UTC (permalink / raw)
To: Jonathan Nieder
Cc: ☂Josh Chia (谢任中), git,
Soeren Sonnenburg
In-Reply-To: <20111126023002.GA17652@elie.hsd1.il.comcast.net>
On Fri, Nov 25, 2011 at 6:30 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> A reproduction recipe (preferrably in the form of a patch to
> t/t3200-branch.sh would be welcome.
Sent in a separate email. Feel free to add a "Tested-by:" header to
your patch if you want :).
>
> -- >8 --
> Subject: treat "git branch -M master master" as a no-op again
>
> Before v1.7.7-rc2~1^2~2 (Prevent force-updating of the current branch,
> 2011-08-20), commands like "git branch -M topic master" could be used
> even when "master" was the current branch, with the somewhat
> counterintuitive result that HEAD would point to some place new while
> the index and worktree kept the content of the old commit. This is
> not a very sensible operation and the result is what almost nobody
> would expect, so erroring out in this case was a good change.
>
> However, there is one exception to the "it's usually not obvious what
> it would mean to overwrite the current branch by another one" rule.
> Namely:
>
> git branch -M master master
>
> is clearly meant to be a no-op, even if you are on the master branch.
Agreed. I thought after reading your patch about making it just do:
if (!strcmp(oldname, newname))
exit(0);
but I guess it would then not mark an entry in the reflog that people
could be relying on...
> + clobber_head_ok = !strcmp(oldname, newname);
> +
> + validate_new_branchname(newname, &newref, force, clobber_head_ok);
This looks ok, and will be improvable if the NEEDSWORK in branch.h is done.
The other thing I wonder is whether "git checkout -B master HEAD" or
"git branch -f master master" should have the same short-cut?
Conrad
^ permalink raw reply
* Re: [PATCH] Test renaming a branch to itself
From: Jonathan Nieder @ 2011-11-26 6:59 UTC (permalink / raw)
To: Conrad Irwin
Cc: git, Soeren Sonnenburg,
☂Josh Chia (谢任中)
In-Reply-To: <1322290364-16207-1-git-send-email-conrad.irwin@gmail.com>
Conrad Irwin wrote:
> Test for a regression introduced in v1.7.7-rc2~1^2~2.
Thanks! I take it that means you like the patch. :)
The tests look sane fwiw (and it looks like the tests you wrote before
cover the "branch -M" safety valve pretty well).
^ permalink raw reply
* [PATCH] Test renaming a branch to itself
From: Conrad Irwin @ 2011-11-26 6:52 UTC (permalink / raw)
To: Jonathan Nieder
Cc: git, Soeren Sonnenburg,
☂Josh Chia (谢任中), Conrad Irwin
In-Reply-To: <20111126023002.GA17652@elie.hsd1.il.comcast.net>
Test for a regression introduced in v1.7.7-rc2~1^2~2.
Requested-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>
---
t/t3200-branch.sh | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index bc73c20..7690332 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -115,6 +115,22 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
git branch -M baz bam
'
+test_expect_success 'git branch -M master should work when master is checked out' '
+ git checkout master &&
+ git branch -M master
+'
+
+test_expect_success 'git branch -M master master should work when master is checked out' '
+ git checkout master &&
+ git branch -M master master
+'
+
+test_expect_success 'git branch -M master2 master2 should work when master is checked out' '
+ git checkout master &&
+ git branch master2 &&
+ git branch -M master2 master2
+'
+
test_expect_success 'git branch -v -d t should work' '
git branch t &&
test_path_is_file .git/refs/heads/t &&
--
1.7.7.1.433.ga2d04a
^ permalink raw reply related
* Re: git branch -M" regression in 1.7.7?
From: Jonathan Nieder @ 2011-11-26 2:30 UTC (permalink / raw)
To: ☂Josh Chia (谢任中)
Cc: git, Soeren Sonnenburg, Conrad Irwin
In-Reply-To: <CALxtSbRbwkVDKJcXiKY9rHYCjA3XGgCytbXQnRhQvbEnY8SpjA@mail.gmail.com>
Hi,
Josh Chia (谢任中) wrote:
> On git 1.7.7.3, when I try to "git branch -M master" when I'm already
> on a branch 'master', I get this error message:
> Cannot force update the current branch
>
> On 1.7.6.4, the command succeeds.
>
> Is this intended?
Yes, but it probably wasn't a good idea. How about this patch?
A reproduction recipe (preferrably in the form of a patch to
t/t3200-branch.sh would be welcome.
-- >8 --
Subject: treat "git branch -M master master" as a no-op again
Before v1.7.7-rc2~1^2~2 (Prevent force-updating of the current branch,
2011-08-20), commands like "git branch -M topic master" could be used
even when "master" was the current branch, with the somewhat
counterintuitive result that HEAD would point to some place new while
the index and worktree kept the content of the old commit. This is
not a very sensible operation and the result is what almost nobody
would expect, so erroring out in this case was a good change.
However, there is one exception to the "it's usually not obvious what
it would mean to overwrite the current branch by another one" rule.
Namely:
git branch -M master master
is clearly meant to be a no-op, even if you are on the master branch.
And in the latter case, it can be abbreviated:
git branch -M master
This seems like a valuable exception to allow, because then "git
branch -M foo" would _always_ be allowed --- either 'foo' is not the
current branch, and it does the obvious thing, or 'foo' is the current
branch, and nothing happens.
Buildbot uses this idiom and was broken in 1.7.7 (it would emit the
message "Cannot force update the current branch").
Reported-by: Soeren Sonnenburg <sonne@debian.org>
Reported-by: Josh Chia (谢任中)
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
builtin/branch.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 51ca6a02..24f33b24 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -568,6 +568,7 @@ static void rename_branch(const char *oldname, const char *newname, int force)
unsigned char sha1[20];
struct strbuf oldsection = STRBUF_INIT, newsection = STRBUF_INIT;
int recovery = 0;
+ int clobber_head_ok;
if (!oldname)
die(_("cannot rename the current branch while not on any."));
@@ -583,7 +584,13 @@ static void rename_branch(const char *oldname, const char *newname, int force)
die(_("Invalid branch name: '%s'"), oldname);
}
- validate_new_branchname(newname, &newref, force, 0);
+ /*
+ * A command like "git branch -M currentbranch currentbranch" cannot
+ * cause the worktree to become inconsistent with HEAD, so allow it.
+ */
+ clobber_head_ok = !strcmp(oldname, newname);
+
+ validate_new_branchname(newname, &newref, force, clobber_head_ok);
strbuf_addf(&logmsg, "Branch: renamed %s to %s",
oldref.buf, newref.buf);
--
1.7.8.rc3
^ permalink raw reply related
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