* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Holger Hellmuth @ 2011-10-13 16:32 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T171530-970@post.gmane.org>
On 13.10.2011 17:53, arQon wrote:
>> git st
> # On branch foo
> # Changes not staged for commit:
> # (use "git add<file>..." to update what will be committed)
> # (use "git checkout --<file>..." to discard changes in working directory)
> #
> # modified: file1.txt
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> What makes this really interesting though is this: I tried to switch to
> master to see if that gave the same warning, and NOW, I get the correct
> error.
>
>> git co master
> error: Your local changes to the following files would be overwritten by
> checkout:
> file1.txt
> Please, commit your changes or stash them before you can switch branches.
> Aborting
At the end of your example (in a previous email) you were on branch
master, now in the beginning you are on foo. So you at least changed
branch again inbetween. maybe you also committed something? Check out
git log or gitk
I tried your example and I can checkout master and foo again and again
and I never see the error message.
> Lucky you. :P The most likely reason for me is, I'm working on something
> and I get interrupted and have to switch. Since the code may well not even
> compile at this point, the last thing I want to do is commit it. git's
> ability for that commit to be local is half the reason I'm trying to switch
> to it. (I'm not particularly keen on having to commit broken code to even a
> local repo, but that's still a hell of a lot better than having it pushed
> upstream as well).
As Alexey already said, just commit and later amend. Or stash. Git
encourages you to commit small changes you can put a name to. You never
should delay a commit because it produces unworkable code. Instead have
a master branch (or branches) that always compiles and branches for the
unfinished stuff. Then it won't matter if some branch is only half working.
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Carlos Martín Nieto @ 2011-10-13 17:04 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T171530-970@post.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4828 bytes --]
On Thu, 2011-10-13 at 15:53 +0000, arQon wrote:
> Carlos Martín Nieto <cmn <at> elego.de> writes:
> > I have not seen a revert command in any of your messages. If a revert on
> > one branch changes another one, that would be a bug, but you haven't
> > shown this to happen.
>
> Sorry, it was in prose in the original post (near the end)
> "At this point, reverting the master with "checkout --" also wipes out the
> changes on the other branch. It's like the merge symlinked the two branches
> rather than, well, merging them."
>
> Based on the explanations here, and the git *st* message, it wiping out the
> other branch is to be expected, because it's "the working directory", not
> "the branch".
>
> >git st
> # On branch foo
> # Changes not staged for commit:
> # (use "git add <file>..." to update what will be committed)
> # (use "git checkout -- <file>..." to discard changes in working directory)
> #
> # modified: file1.txt
> #
> no changes added to commit (use "git add" and/or "git commit -a")
>
> What makes this really interesting though is this: I tried to switch to
> master to see if that gave the same warning, and NOW, I get the correct
> error.
>
> >git co master
> error: Your local changes to the following files would be overwritten by
> checkout:
> file1.txt
> Please, commit your changes or stash them before you can switch branches.
> Aborting
>
> I'm sure if I thought about it enough (ie re-read Andreas's post a couple
> more times) I'd be able to understand why git gets it right sometimes but
> not other times, but I'm too tired right now. Even when I *am* awake and
> grok it properly, I'm still going to be annoyed that it's so inconsistent,
> but I can live with that if I have to.
If file1.txt in the foo branch is different from the one in the master
branch, git will refuse to switch branches. 'git diff foo master' should
show that those two files are different.
>
> > The reason this happens both in svn and git is that the most likely
> > cause for someone to change a branch mid-edit is that they decide
> > they're doing the changes on the wrong branch.
>
> Lucky you. :P The most likely reason for me is, I'm working on something
> and I get interrupted and have to switch. Since the code may well not even
> compile at this point, the last thing I want to do is commit it. git's
> ability for that commit to be local is half the reason I'm trying to switch
> to it. (I'm not particularly keen on having to commit broken code to even a
> local repo, but that's still a hell of a lot better than having it pushed
> upstream as well).
Yes, this is a great feature of distributed systems. A local repo is
where you experiment. Treat it as your own personal space to play around
with things. Committing non-working code is fine, as long as you don't
push it out.
>
> > svn doesn't tell you about the modifications being carried over
> > (presumably you're meant to use status and diff to figure out what's
> > going on). Therefore, the same workflow (with the only difference being
> > how to create and switch branches) works for svn and git in this case.
>
> I expect part of my confusion comes from using different workdirs for svn
> branches, ie "clone" rather than "branch", because branching in svn is such
> a PITA I just don't bother with it unless the branch is going to be
> "heavyweight" enough to warrant a "proper" branch.
Then the issue is that you've changed the workflow but haven't adjusted
for it. You can do this as well with the git-new-workdir. As I mentioned
it has a few rough edges, but if you're going to use it to have a
checkout of a particular branch, it shouldn't present any problems. That
would be like your current workflow.
Another option is to clone with a reference which will create a brand
new clone but will use the objects that you've already downloaded (or
just clone locally). This can be more comfortable than using the
new-workdir and will hardly put any strain on the filesystem.
The bigger problem seems to be your reluctance to accept that git is
different from subversion, as you keep saying "that's just how git is"
to back your claim that you can't trust git on a feature where
subversion behaves the same way. If you'd rather use different
directories for different branches, you can. That is not an aspect which
you can point to and say that you can't migrate to git for that reason.
If you're more comfortable with subversion, that's fine also, it's an
excellent piece of software[0], but don't go around saying that git
corrupts branches when that's blatantly not true.
cmn
[0] Whatever one may think about the merits of CVCS vs DVCS; that
shouldn't come into the quality of the software.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [CLOSED] git checkout <branch> allowed with uncommitted changes
From: arQon @ 2011-10-13 17:09 UTC (permalink / raw)
To: git
In-Reply-To: <1318514356.4646.16.camel@centaur.lab.cmartin.tk>
Carlos Martín Nieto <cmn <at> elego.de> writes:
> When you changed branches, git told you that a file had been changed in
> the working tree.
That's a very good point, if it was actually documented at all that that's
what it meant; and / or presented some warning that "BTW, if you edit this
file again now, it'll screw up your whole tree" instead of an innocuous "M";
and didn't appear to contradict what the manpage says about "local
modifications", we could probably have avoided half of this confusion. :P
As it was, when I saw that M suddenly appear after several days of bouncing
between branches without it and with everything working, I just thought
"oh great, git's managed to break this tree", because I remembered the same
thing from the previous trial run.
That there IS an indication though might just be enough for me to be able to
deal with it. Realistically, switching branches with uncommitted changes
(unless you're doing it because you've ALREADY screwed up and are changing
the wrong branch) is basically a trainwreck waiting to happen.
git stash appears to be useless for any nontrivial change on the *other*
branch, since there's no indication when you return to the stashed branch
there's a stash sitting around, which is not something you're going to
remember the next morning if fixing the master took the rest of the day,
and you're not going to use "stash list" by then either.
But as long as you get the "warning", an alias that does a "commit -am 'temp
commit to avoid git breaking the tree'" is something I think I can probably
live with.
Thanks for all the help guys - very much appreciated.
As far as I'm concerned, this topic's done.
(Though if someone can come up with a script / hook / whatever that improves
the "visibility" of stash, that would be awesome. Or one that makes the
refusal to switch branches consistent).
Looking at the manpage for checkout in the hope that there might be a "--safe"
switch, I don't understand why
"-f Proceed even if the index *or the working tree* differs from HEAD."
even exists, since it proceeds under those conditions anyway.
"--safe" appears to be exactly what the behavior should be if you DON'T
specify -f, except that -f nukes the working tree outright rather than just
bleeding it across. Hopefully it'll be clearer after some sleep. :)
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Sergei Organov @ 2011-10-13 17:06 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T171530-970@post.gmane.org>
arQon <arqon@gmx.com> writes:
[...]
> Lucky you. :P The most likely reason for me is, I'm working on something
> and I get interrupted and have to switch. Since the code may well not even
> compile at this point, the last thing I want to do is commit it.
'git stash' is exactly what you need then.
-- Sergei.
^ permalink raw reply
* Re: [PATCH 2/9] completion: optimize refs completion
From: Junio C Hamano @ 2011-10-13 17:28 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git, Shawn O. Pearce, Jonathan Nieder
In-Reply-To: <20111013104047.GA15379@goldbirke>
SZEDER Gábor <szeder@ira.uka.de> writes:
> ... If we use the default IFS containing an SP and append a SP to
> possible completion words by 'compgen -S " "' (or by word="$word ", as
> in __gitcomp_1()), then that SP will be promply stripped off when
> compgen's output is stored in the COMPREPLY array. Using an IFS
> without SP keeps those SP suffixes.
Ahh, then I am perfectly fine with the "nl" in its name.
This is a function that unconditionally completes with trailing separator,
avoiding the cost of having to inspect each element, but the user needs to
keep in mind that the elements in the input must be separated with NL, so
ideally the name needs to convey both of these points---dropping "nl" was
a bad suggestion.
Thanks for a clear explanation.
^ permalink raw reply
* Re: Git attributes ignored for root directory
From: Junio C Hamano @ 2011-10-13 17:38 UTC (permalink / raw)
To: Gioele Barabucci; +Cc: Michael Haggerty, git
In-Reply-To: <4E96C220.5050601@svario.it>
Gioele Barabucci <gioele@svario.it> writes:
> On 13/10/2011 00:12, Junio C Hamano wrote:
> ...
>> How would you say the same thing if the directory to be ignored weren't
>> "foo" but at the top-level of the working tree? There is no such level
>> higher than the top-level where you can say "<the name of your project>/"
>> in its .gitignore file.
>
> Shouldn't `/.` or `/./` work?
>
> I see that `/*/` in `.gitignores` successfully ignores all the
> non-hidden directories in the root project directory. Another
> accidental success? :)
Hannes correctly explained how /*/ works already; armed with that
knowledge, we can understand that:
/. would mean "what matches dot only in this directory."
/./ would mean "what matches dot only in this directory
but the thing that matches must be a directory."
./ would mean "what matches dot in this directory and its
subdirectories, but the thing that matches must be a
directory."
Given that "." does _not_ match the directory that has the .gitignore or
the .gitattribute file with the current system, none of the above patterns
can be used to match everything in the top level directory in a way
similar to how you can say "foo/" to match everything in "foo" directory
from the top-level directory.
The answer to your question is no, it shouldn't work.
Without adding a rule to the "Pattern Format" section as I suggested in my
message that Michael quoted in his question, that is.
^ permalink raw reply
* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Junio C Hamano @ 2011-10-13 18:01 UTC (permalink / raw)
To: Jeff King; +Cc: Kirill Likhodedov, git
In-Reply-To: <20111013155923.GA13134@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Changing these scripts to use require_work_tree_exists is
> easy to verify. We immediately call cd_to_toplevel, anyway.
> Therefore no matter which function we use, the state
> afterwards is one of:
>
> 1. We have a work tree, and we are at the top level.
>
> 2. We don't have a work tree, and we have died.
>
> The only catch is that we must also make sure no code that
> ran before the cd_to_toplevel assumed that we were already
> in the working tree.
>
> In this case, we will only have included shell libraries and
> called set_reflog_action, neither of which care about the
> current working directory at all.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is the low-hanging, obviously correct fruit.
I am not absolutely sure about "obviously correct", given that you assume
that cd_to_toplevel does what its name makes you think it does. I've been
wondering if we want to do give a bit more sanity to "cd_to_toplevel" and
"rev-parse --show-toplevel".
$ pwd
/srv/project/git/git.git
$ (cd Documentation/howto && git rev-parse --show-toplevel); echo $?
/srv/project/git/git.git
0
So far so good, however:
$ (cd .git/refs/heads && git rev-parse --show-toplevel); echo $?
0
I do not think this is quite right.
We would probably want to add "rev-parse --show-work-tree", but we would
need to audit the users of cd_to_toplevel before starting to use it. I
wouldn't be surprised if there is a script that creates a temporary work
tree in .git/some/where and runs the scripted Porcelains without setting
GIT_WORK_TREE, relying on the historical behaviour of cd_to_toplevel that
does not really go to the top level.
^ permalink raw reply
* Re: [PATCH 0/2] Do not search submodules in deep recursive merge
From: Junio C Hamano @ 2011-10-13 18:15 UTC (permalink / raw)
To: Brad King; +Cc: git, Junio C Hamano, Heiko Voigt
In-Reply-To: <cover.1318509069.git.brad.king@kitware.com>
Brad King <brad.king@kitware.com> writes:
> On 10/12/2011 2:48 PM, Junio C Hamano wrote:
>> [Stalled]
>>
>> * hv/submodule-merge-search (2011-08-26) 5 commits
>> - submodule: Search for merges only at end of recursive merge
>> - allow multiple calls to submodule merge search for the same path
>> - submodule: Demonstrate known breakage during recursive merge
>> - push: Don't push a repository with unpushed submodules
>> - push: teach --recurse-submodules the on-demand option
>> (this branch is tangled with fg/submodule-auto-push.)
>
> AFAICT these two topics are tangled due revision traversal interactions.
> I've untangled the two "submodule:" commits from this stalled topic and
> rebased on master (34c4461a) resolving one conflict.
Thanks.
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: arQon @ 2011-10-13 18:19 UTC (permalink / raw)
To: git
In-Reply-To: <1318525486.4646.53.camel@centaur.lab.cmartin.tk>
Carlos Martín Nieto <cmn <at> elego.de> writes:
> If file1.txt in the foo branch is different from the one in the master
> branch, git will refuse to switch branches. 'git diff foo master' should
> show that those two files are different.
Right, but only for a definition of "branch" that is actually "a fully
committed branch", hence the confusion and the mention of "uncommitted
changes" in the topic.
An expectation that "co branch" should be analogous to "cd ../branch/" is by
no means unreasonable. YOU may know better, but it's surprisingly non-obvious,
especially considering the -f option on checkout and the wording of -m, both
of which strongly suggest that, in the absence of either of those flags, git
WILL preserve the worktree by refusing to switch until that potentially-
harmful situation is resolved by the user.
> Committing non-working code is fine, as long as you don't push it out.
Right, but for the problem I was describing it's actually "committing
non-working code is a requirement, in this situation, if you don't want your
tree to get eaten". Going from "you absolutely must not do this" to "you must
do this" takes some mental adjustment, but you also have to be *aware* that
you now have to do something that was previously prohibited, which I wasn't.
> The bigger problem seems to be your reluctance to accept that git is
> different from subversion
Not at all. If I didn't WANT something different, I wouldn't have been trying
to move to git in the first place. :)
> but don't go around saying that git
> corrupts branches when that's blatantly not true.
See my first para in this post (or indeed, the original post). It's "not true"
provided all branches are fully committed when you switch between them.
It blatantly IS true if you switch from a dirty branch.
Redefining "branch" to mean "fully committed branch" makes it "not true" in
that context, but so does redefining green to be red and saying that grass is
red in that context: it may be correct from a certain POV, but it's
incomprehensible to anyone who isn't aware of that semantic change.
Anyway, I think we're done with this thread. Thanks for your (key) observation
earlier and several clarifications, and hopefully this will help the next guy
who runs into this problem and gets confused like I did. :)
^ permalink raw reply
* Re: [PATCH] daemon: return "access denied" if a service is not allowed
From: Jeff King @ 2011-10-13 18:28 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy
Cc: git, Ilari Liusvaara, Junio C Hamano, Johannes Sixt,
Jonathan Nieder
In-Reply-To: <20111013044544.GA27890@duynguyen-vnpc.dek-tpc.internal>
On Thu, Oct 13, 2011 at 03:45:44PM +1100, Nguyen Thai Ngoc Duy wrote:
> On Wed, Oct 12, 2011 at 04:09:16PM -0400, Jeff King wrote:
> > On Tue, Oct 04, 2011 at 08:55:09AM +1100, Nguyen Thai Ngoc Duy wrote:
> >
> > > The message is chosen to avoid leaking information, yet let users know
> > > that they are deliberately not allowed to use the service, not a fault
> > > in service configuration or the service itself.
> >
> > I do think this is an improvement, but I wonder if the verbosity should
> > be configurable. Then open sites like kernel.org could be friendlier to
> > their users. Something like this instead:
>
> How about allow users to select which messages they want to print? We
> can even go further, allowing users to specify the messages themselves..
I thought about that, but it just seemed like it was making things way
more complex than it needed to be. GitHub does do this kind of
customization, but we also have a custom layer that intercepts git://
connections, anyway, so we added the relevant code there.
I don't know if medium-sized sites (i.e., ones that aren't so big they
are running custom proxies on the frontend) would care about adding
custom messages here or not.
> I don't know. I'm not a real server admin so maybe I'm just too
> paranoid. Any admins care to speak up?
I doubt anybody would care that much about turning individual messages
on and off. I think the real value is in being able to say "don't push
by git://. The right way to push to this site is...".
But your patch kind of falls short of what people would want to do for
two reasons:
1. The message isn't dynamic at all. So I can't say:
You tried to push to git://host.tld/foo.git. The right way to do
that is:
git push https://host.tld/foo.git
That's what the GitHub message does if you try to push over git://;
it gives you a new remote name that will actually work, customized
to the repo you wanted to push to.
2. Tweaking just the message for anything but "service not enabled"
isn't all that useful. What do you say about "no such repository"
in a simple message, even with placeholders?
If you _really_ want to get fancy, a server could do a fuzzy
search on the available repos and say "did you mean...?".
But now we are talking about hooking arbitrary code into the
message.
So if we want to do anything, I would think it would be a hook. Except
that we may or may not have a repo, so it would not be a hook in
$GIT_DIR/hooks, but rather some script to be run passed on the command
line, like:
git daemon --informative-errors=/path/to/hook
-Peff
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: Junio C Hamano @ 2011-10-13 18:28 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T193054-868@post.gmane.org>
arQon <arqon@gmx.com> writes:
> ..., in the absence of either of those flags, git
> WILL preserve the worktree by refusing to switch until that potentially-
> harmful situation is resolved by the user.
Perhaps you can prepare a documentation patch to make it clear that git
WILL preserve the LOCAL CHANGES to the working tree?
As it would already be clear to anybody reading this thread so far, local
changes made to the working tree do not belong to any particular branch.
They are floating on top, and it is up to the user what to do with these
floating changes when they conflict with the differences between the
branches you are switching across (i.e. you cannot switch so you need to
clean up by either committing, stashing, or deciding not to switch and
instead complete the work before you switch), and when they do not
conflict with the differences between the branches you are switching
across (i.e. you will carry them to the new working tree. It may be that
you made these changes and then realized that they do not belong to the
goal the current branch aims to achieve and that is why you decided to
switch to another branch, in which case you do not have to do anything
special in order to continue to work and complete it to commit to the
switched branch. It may be that you made these changes but needed to tend
to unrelated business on an unrelated branch and that is why you switched,
in which case you would want to clear them away, which is exactly what
stash was invented for).
^ permalink raw reply
* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Jeff King @ 2011-10-13 18:37 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kirill Likhodedov, git
In-Reply-To: <7vbotk6aae.fsf@alter.siamese.dyndns.org>
On Thu, Oct 13, 2011 at 11:01:13AM -0700, Junio C Hamano wrote:
> I am not absolutely sure about "obviously correct", given that you assume
> that cd_to_toplevel does what its name makes you think it does. I've been
> wondering if we want to do give a bit more sanity to "cd_to_toplevel" and
> "rev-parse --show-toplevel".
>
> $ pwd
> /srv/project/git/git.git
> $ (cd Documentation/howto && git rev-parse --show-toplevel); echo $?
> /srv/project/git/git.git
> 0
>
> So far so good, however:
>
> $ (cd .git/refs/heads && git rev-parse --show-toplevel); echo $?
> 0
>
> I do not think this is quite right.
Ugh. You are right. I for some reason assumed that cd_to_toplevel would,
of all things, cd to the toplevel. I think the right solution is to
introduce a "cd_to_work_tree_toplevel" (or similarly named) command that
always moves to the root of the work tree.
And then convert the two scripts in my patch to use it (along with the
change to require_work_tree_exists). That would make my prior analysis
hold, then, as the annoying do-nothing behavior of "cd_to_toplevel" only
kicks in when we are outside the work tree (i.e., it could not have
happened before in those scripts, because the existing require_work_tree
call would cause us to die).
> We would probably want to add "rev-parse --show-work-tree", but we would
> need to audit the users of cd_to_toplevel before starting to use it. I
> wouldn't be surprised if there is a script that creates a temporary work
> tree in .git/some/where and runs the scripted Porcelains without setting
> GIT_WORK_TREE, relying on the historical behaviour of cd_to_toplevel that
> does not really go to the top level.
Right. I suspect the proposed behavior for cd_to_toplevel is what they
all would want eventually, but some scripts may need minor tweaks. I
think we should follow the same path as require_work_tree_exists, and
introduce the new function, use it where we know it's safe, and then
eventually get rid of the old one.
The real trick is coming up with a good name, because cd_to_toplevel is
already taken. :)
-Peff
^ permalink raw reply
* Re: [PATCH 01/14] cache.h: add comments for git_path() and git_path_submodule()
From: Junio C Hamano @ 2011-10-13 18:37 UTC (permalink / raw)
To: mhagger
Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski,
Heiko Voigt, Johan Herland, Julian Phillips
In-Reply-To: <1318492715-5931-2-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> +
> +/*
> + * Return the path of a file within get_git_dir(). The arguments
> + * should be printf-like arguments that produce the filename relative
> + * to get_git_dir(). Return the resulting path, or "/bad-path/" if
> + * there is an error.
> + */
> extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
Ok.
> +/*
> + * Return the path of a file within the submodule located at path.
This is confusing. Does this "file within the submodule" refer to files
like "Makefile" tracked in a submodule at "dir"? Your description for
git_path() above makes it clear that the function is about files like
"index" and "HEAD" that are part of the control information for the
current project, but the above gives an impression that you are talking
about files in the working tree of the submodule.
> + * The other arguments should be printf-like arguments that produce
> + * the filename relative to "<path>/.git". If "<path>/.git" is a
And the reader is puzzled by the sudden mention of <path>/.git here.
> + * gitlink file, follow it to find the actual submodule git path.
> + * Return the resulting path, or "/bad-path/" if there is an error.
> + */
> extern char *git_path_submodule(const char *path, const char *fmt, ...)
> __attribute__((format (printf, 2, 3)));
^ permalink raw reply
* Re: [PATCH 02/14] struct ref_list: document name member
From: Junio C Hamano @ 2011-10-13 18:37 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1318492715-5931-3-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 409314d..e4e4bcd 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -12,6 +12,7 @@ struct ref_entry {
> unsigned char flag; /* ISSYMREF? ISPACKED? */
> unsigned char sha1[20];
> unsigned char peeled[20];
> + /* The full name of the reference (e.g., "refs/heads/master"): */
> char name[FLEX_ARRAY];
> };
Thanks. Needs retitling the patch, though.
^ permalink raw reply
* Re: [PATCH 04/14] refs: rename some parameters result -> sha1
From: Junio C Hamano @ 2011-10-13 18:42 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1318492715-5931-5-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
[PATCH 03/14] was about a similar topic and explained itself a lot
better.
Even though it hinted as if it may be incomplete by saying "some" in the
subject, it was clear from the description that it consistently renamed
all the "name"s that are about references, not just "some" randomly chosen
ones. It would have been better if the subject did not say "some" to avoid
such implications.
Please give similar love to these sha1[] object names in this patch.
> ---
> refs.c | 16 ++++++++--------
> refs.h | 2 +-
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 2ae5d0d..c466fcd 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -398,7 +398,7 @@ static struct ref_array *get_loose_refs(const char *submodule)
> #define MAXREFLEN (1024)
>
> static int resolve_gitlink_packed_ref(char *name, int pathlen,
> - const char *refname, unsigned char *result)
> + const char *refname, unsigned char *sha1)
> {
> int retval = -1;
> struct ref_entry *ref;
> @@ -406,14 +406,14 @@ static int resolve_gitlink_packed_ref(char *name, int pathlen,
>
> ref = search_ref_array(array, refname);
> if (ref != NULL) {
> - memcpy(result, ref->sha1, 20);
> + memcpy(sha1, ref->sha1, 20);
> retval = 0;
> }
> return retval;
> }
>
> static int resolve_gitlink_ref_recursive(char *name, int pathlen,
> - const char *refname, unsigned char *result,
> + const char *refname, unsigned char *sha1,
> int recursion)
> {
> int fd, len = strlen(refname);
> @@ -424,7 +424,7 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
> memcpy(name + pathlen, refname, len+1);
> fd = open(name, O_RDONLY);
> if (fd < 0)
> - return resolve_gitlink_packed_ref(name, pathlen, refname, result);
> + return resolve_gitlink_packed_ref(name, pathlen, refname, sha1);
>
> len = read(fd, buffer, sizeof(buffer)-1);
> close(fd);
> @@ -435,7 +435,7 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
> buffer[len] = 0;
>
> /* Was it a detached head or an old-fashioned symlink? */
> - if (!get_sha1_hex(buffer, result))
> + if (!get_sha1_hex(buffer, sha1))
> return 0;
>
> /* Symref? */
> @@ -445,10 +445,10 @@ static int resolve_gitlink_ref_recursive(char *name, int pathlen,
> while (isspace(*p))
> p++;
>
> - return resolve_gitlink_ref_recursive(name, pathlen, p, result, recursion+1);
> + return resolve_gitlink_ref_recursive(name, pathlen, p, sha1, recursion+1);
> }
>
> -int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *result)
> +int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *sha1)
> {
> int len = strlen(path), retval;
> char *gitdir;
> @@ -472,7 +472,7 @@ int resolve_gitlink_ref(const char *path, const char *refname, unsigned char *re
> }
> gitdir[len] = '/';
> gitdir[++len] = '\0';
> - retval = resolve_gitlink_ref_recursive(gitdir, len, refname, result, 0);
> + retval = resolve_gitlink_ref_recursive(gitdir, len, refname, sha1, 0);
> free(gitdir);
> return retval;
> }
> diff --git a/refs.h b/refs.h
> index 13e2aa3..c6b8749 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -133,7 +133,7 @@ extern char *shorten_unambiguous_ref(const char *refname, int strict);
> extern int rename_ref(const char *oldref, const char *newref, const char *logmsg);
>
> /** resolve ref in nested "gitlink" repository */
> -extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *result);
> +extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *sha1);
>
> /** lock a ref and then write its file */
> enum action_on_err { MSG_ON_ERR, DIE_ON_ERR, QUIET_ON_ERR };
^ permalink raw reply
* Re: [PATCH 05/14] clear_ref_list(): rename from free_ref_list()
From: Junio C Hamano @ 2011-10-13 18:43 UTC (permalink / raw)
To: mhagger
Cc: Junio C Hamano, git, Jeff King, Drew Northup, Jakub Narebski,
Heiko Voigt, Johan Herland, Julian Phillips
In-Reply-To: <1318492715-5931-6-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
> Rename the function since it doesn't actually free the array object
> that is passed to it.
The commit log message correctly refers to the "array-ness" of the object
being cleared. Needs retitling the patch to match.
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> ---
> refs.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index c466fcd..a2e48e4 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -149,7 +149,7 @@ static struct ref_entry *current_ref;
>
> static struct ref_array extra_refs;
>
> -static void free_ref_array(struct ref_array *array)
> +static void clear_ref_array(struct ref_array *array)
> {
> int i;
> for (i = 0; i < array->nr; i++)
> @@ -162,14 +162,14 @@ static void free_ref_array(struct ref_array *array)
> static void clear_cached_packed_refs(struct cached_refs *refs)
> {
> if (refs->did_packed)
> - free_ref_array(&refs->packed);
> + clear_ref_array(&refs->packed);
> refs->did_packed = 0;
> }
>
> static void clear_cached_loose_refs(struct cached_refs *refs)
> {
> if (refs->did_loose)
> - free_ref_array(&refs->loose);
> + clear_ref_array(&refs->loose);
> refs->did_loose = 0;
> }
>
> @@ -256,7 +256,7 @@ void add_extra_ref(const char *refname, const unsigned char *sha1, int flag)
>
> void clear_extra_refs(void)
> {
> - free_ref_array(&extra_refs);
> + clear_ref_array(&extra_refs);
> }
>
> static struct ref_array *get_packed_refs(const char *submodule)
^ permalink raw reply
* Re: [PATCH 06/14] resolve_gitlink_ref(): improve docstring
From: Junio C Hamano @ 2011-10-13 18:48 UTC (permalink / raw)
To: mhagger
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1318492715-5931-7-git-send-email-mhagger@alum.mit.edu>
mhagger@alum.mit.edu writes:
> From: Michael Haggerty <mhagger@alum.mit.edu>
>
>
> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
> -/** resolve ref in nested "gitlink" repository */
> +/**
> + * Resolve refname in the nested "gitlink" repository that is located
> + * at name. If the resolution is successful, return 0 and set sha1 to
> + * the name of the object; otherwise, return a non-zero value.
> + */
It is clear that "refname" would refer to things like "refs/heads/master",
but "name" is still not clear enough with the description. 'repository
that is located at name' hints that we may be dealing with more than one
repository and 'name' is a way to identify which one, but perhaps "path"
or "submodule" a much clearer way to indicate what the code is doing.
At the UI level, a submodule has "name" and "path" that are often the same
but can be different (e.g. when the superproject moves a submodule that
used to be bound to path "dir" to a different location, only the latter
should change). I do not think resolve_gitlink_ref() takes the submodule
name, but it takes the path to the submodule in the superproject. In that
sense, "submodule_path" would be the clearest descriptive name for this
parameter.
> extern int resolve_gitlink_ref(const char *name, const char *refname, unsigned char *sha1);
^ permalink raw reply
* Re: [PATCH 07/14] is_refname_available(): remove the "quiet" argument
From: Junio C Hamano @ 2011-10-13 18:49 UTC (permalink / raw)
To: Drew Northup
Cc: mhagger, git, Jeff King, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <1318509685.7231.6.camel@drew-northup.unet.maine.edu>
Drew Northup <drew.northup@maine.edu> writes:
> On Thu, 2011-10-13 at 09:58 +0200, mhagger@alum.mit.edu wrote:
>> From: Michael Haggerty <mhagger@alum.mit.edu>
>>
>> quiet was always set to 0, so get rid of it. Add a function docstring
>> for good measure.
>
> I would like to know if perhaps it was an unfinished project somewhere
> to propagate the "quiet" option down to this level before removing the
> function argument. Comments?
Have you tried blaming?
^ permalink raw reply
* Re: [CLOSED] git checkout <branch> allowed with uncommitted changes
From: Alexey Shumkin @ 2011-10-13 18:56 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T181801-923@post.gmane.org>
> (Though if someone can come up with a script / hook / whatever that
> improves the "visibility" of stash, that would be awesome.
>
"git-completion" for Bash/ZSH gives such an opportunity
I use it
take a look into
<git-sources>/contrib/completion/git-completion.bash
-- >8 --
# 3) Consider changing your PS1 to also show the current branch:
# Bash: PS1='[\u@\h \W$(__git_ps1 " (%s)")]\$ '
# ZSH: PS1='[%n@%m %c$(__git_ps1 " (%s)")]\$ '
#
# The argument to __git_ps1 will be displayed only if you
# are currently in a git repository. The %s token will be
# the name of the current branch.
#
# In addition, if you set GIT_PS1_SHOWDIRTYSTATE to a nonempty
# value, unstaged (*) and staged (+) changes will be shown next
# to the branch name. You can configure this per-repository
# with the bash.showDirtyState variable, which defaults to true
# once GIT_PS1_SHOWDIRTYSTATE is enabled.
#
# You can also see if currently something is stashed, by setting
# GIT_PS1_SHOWSTASHSTATE to a nonempty value. If something is
stashed, # then a '$' will be shown next to the branch name.
#
# If you would like to see if there're untracked files, then you
can # set GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If
there're # untracked files, then a '%' will be shown next to the
branch name. #
# If you would like to see the difference between HEAD and its
# upstream, set GIT_PS1_SHOWUPSTREAM="auto". A "<" indicates
# you are behind, ">" indicates you are ahead, and "<>"
# indicates you have diverged.
-- >8 --
my .bashrc contains (shortly)
PS1='\[\e]0;\w [$(__git_ps1 "%s")]\a\]\n\[\e[32m\]\u@\h'
PS1=$PS1' \[\e[33m\]\w\[\e[0m\]\n[$(__git_ps1 "%s")]\n\$'
export PS1
export GIT_PS1_SHOWDIRTYSTATE=1
export GIT_PS1_SHOWSTASHSTATE=1
export GIT_PS1_SHOWUPSTREAM="auto"
and console prompt with all possible cases looks like
<username>@<hostname> ~/Git-src.git/contrib/completion
[post-receive-email *+$>]
$
* - I have unstaged changes
+ - I have staged changes
$ - I have stashed changes (ta-daaa!)
> - I have commits ahead upsteam (named branch I branched from)
P.S.
And JFYI, it is a good form in mailing lists to CC (Reply to all)
participants
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: arQon @ 2011-10-13 18:56 UTC (permalink / raw)
To: git
In-Reply-To: <7vzkh44ug1.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster <at> pobox.com> writes:
> Perhaps you can prepare a documentation patch to make it clear that git
> WILL preserve the LOCAL CHANGES to the working tree?
Or to put that another way, git WILL NOT "rewind" those local changes when
switching branches (which I still think is the more common case for new users
than failing to branch before editing files). Or refuse to switch if you have
some. Except for when it does.
I'll give a shot, though I don't know how good it'll be. Off the top of my
head, I don't see any good way to explain the inconsistency with LOCAL CHANGES
sometimes preventing switches and sometimes not, based on what is to the user
an arbitrary set of rules that has nothing to do with the *current state* of
the worktree, but rather the state of those files in prior commits.
But sure, I'll see if I can come up with something. If nothing else, having the
manpage at least explain what "M" means; that it can be potentially disastrous;
and what you need to do to avoid it, would be a definite plus.
^ permalink raw reply
* Re: [CLOSED] git checkout <branch> allowed with uncommitted changes
From: Jakub Narebski @ 2011-10-13 19:01 UTC (permalink / raw)
To: arQon; +Cc: git
In-Reply-To: <loom.20111013T181801-923@post.gmane.org>
arQon <arqon@gmx.com> writes:
> (Though if someone can come up with a script / hook / whatever that improves
> the "visibility" of stash, that would be awesome. Or one that makes the
> refusal to switch branches consistent).
Well, if you use __git_ps1 from contrib/completion/git-completion.bash
(installed with git-core package for some time), there is an option to
add '$' to branch name if stash is non-empty (though it doesn't actually
check if stash was on said branch).
> Looking at the manpage for checkout in the hope that there might be a "--safe"
> switch, I don't understand why
>
> "-f Proceed even if the index *or the working tree* differs from HEAD."
>
> even exists, since it proceeds under those conditions anyway.
> "--safe" appears to be exactly what the behavior should be if you DON'T
> specify -f, except that -f nukes the working tree outright rather than just
> bleeding it across. Hopefully it'll be clearer after some sleep. :)
Without '-f' git-checkout would switch branches only if uncomitted
changes (which do not belong to any branch) could be "floated" on top
of new branch.
If branch you are switching to has differences from current branch
that conflict with uncomitted changes, git would refuse switching
branches. Now '-f' would get rid of your uncomitted changes, and '-m'
try to merge it with changes brought by new branch.
HTH
--
Jakub Narębski
^ permalink raw reply
* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Junio C Hamano @ 2011-10-13 19:06 UTC (permalink / raw)
To: Jeff King; +Cc: Kirill Likhodedov, git
In-Reply-To: <20111013183709.GB17573@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> And then convert the two scripts in my patch to use it (along with the
> change to require_work_tree_exists). That would make my prior analysis
> hold, then, as the annoying do-nothing behavior of "cd_to_toplevel" only
> kicks in when we are outside the work tree (i.e., it could not have
> happened before in those scripts, because the existing require_work_tree
> call would cause us to die).
> ...
> Right. I suspect the proposed behavior for cd_to_toplevel is what they
> all would want eventually, but some scripts may need minor tweaks. I
> think we should follow the same path as require_work_tree_exists, and
> introduce the new function, use it where we know it's safe, and then
> eventually get rid of the old one.
>
> The real trick is coming up with a good name, because cd_to_toplevel is
> already taken. :)
It is not as simple as that I am afraid. We could introduce cd_to_top with
the new semantics and use it in pull and rebase, but a case that would
break is for a script (let's call that hypothetical operation "git svn
dcommit", even though I do not know if dcommit uses the real working tree
or a temporary one) that prepares a temporary working tree inside .git/svn/
and run "git rebase" there without setting GIT_WORKING_TREE to point at
the temporary directory.
With cd_to_toplevel, such a "rebase" would work and "git svn dcommit" can
take that result and do whatever it wants to the real working tree after
it finishes. When we start using cd_to_top in the updated "rebase", such a
script suddenly breaks, as we would start touching the real working tree.
So I do not think it makes much sense to add cd_to_top with updated
semantics while keeping cd_to_toplevel.
What we could do is to update cd_to_toplevel so that it would notice and
warn when the results between the historical incorrect behaviour and the
updated behaviour would be different. The warning can first read "You are
running 'rebase' somewhere in $GIT_DIR without setting $GIT_WORK_TREE; we
historically used the directory you started 'rebase' as the top level of
the working tree, and this version continues to do so, but it will change
to work on the real working tree associated with your $GIT_DIR in future
versions of git. Update your script to correctly set $GIT_WORK_TREE", and
then we transition to start using the new semantics while rewording the
warning message, and then later remove the warning altogether.
^ permalink raw reply
* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Jeff King @ 2011-10-13 19:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kirill Likhodedov, git
In-Reply-To: <7v62js4sop.fsf@alter.siamese.dyndns.org>
On Thu, Oct 13, 2011 at 12:06:46PM -0700, Junio C Hamano wrote:
> It is not as simple as that I am afraid. We could introduce cd_to_top with
> the new semantics and use it in pull and rebase, but a case that would
> break is for a script (let's call that hypothetical operation "git svn
> dcommit", even though I do not know if dcommit uses the real working tree
> or a temporary one) that prepares a temporary working tree inside .git/svn/
> and run "git rebase" there without setting GIT_WORKING_TREE to point at
> the temporary directory.
I didn't think that could happen now, because you would not be in the
working tree, and therefore require_work_tree would fail. E.g., with
current git I get:
$ mkdir .git/tmp
$ cd .git/tmp
$ git rebase
fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase
cannot be used without a working tree.
So that case is already broken. The only change this would make is that
what used to fail would not actually take them to the top-level of the
working tree[1].
-Peff
[1] Actually, I am not sure it would do that. If we are in $GIT_DIR, do
we necessarily know where the working tree is? I guess in a non-bare
repo, we assume it is $GIT_DIR/..?
^ permalink raw reply
* Re: [Bug] git pull doesn't recognize --work-tree parameter
From: Jeff King @ 2011-10-13 19:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kirill Likhodedov, git
In-Reply-To: <20111013191457.GA18460@sigill.intra.peff.net>
On Thu, Oct 13, 2011 at 03:14:57PM -0400, Jeff King wrote:
> On Thu, Oct 13, 2011 at 12:06:46PM -0700, Junio C Hamano wrote:
>
> > It is not as simple as that I am afraid. We could introduce cd_to_top with
> > the new semantics and use it in pull and rebase, but a case that would
> > break is for a script (let's call that hypothetical operation "git svn
> > dcommit", even though I do not know if dcommit uses the real working tree
> > or a temporary one) that prepares a temporary working tree inside .git/svn/
> > and run "git rebase" there without setting GIT_WORKING_TREE to point at
> > the temporary directory.
>
> I didn't think that could happen now, because you would not be in the
> working tree, and therefore require_work_tree would fail. E.g., with
> current git I get:
>
> $ mkdir .git/tmp
> $ cd .git/tmp
> $ git rebase
> fatal: fatal: /home/peff/local/git/private/libexec/git-core/git-rebase
> cannot be used without a working tree.
>
> So that case is already broken. The only change this would make is that
> what used to fail would not actually take them to the top-level of the
> working tree[1].
Ugh. It does work if you do:
mkdir .git/tmp
cd .git/tmp
GIT_DIR=$PWD/.. git rebase
What a god-awful mess our initialization rules are.
-Peff
^ permalink raw reply
* Re: [BUG] git checkout <branch> allowed with uncommitted changes
From: PJ Weisberg @ 2011-10-13 19:44 UTC (permalink / raw)
To: git
In-Reply-To: <loom.20111013T171530-970@post.gmane.org>
On Thu, Oct 13, 2011 at 8:53 AM, arQon <arqon@gmx.com> wrote:
>>git co master
> error: Your local changes to the following files would be overwritten by
> checkout:
> file1.txt
> Please, commit your changes or stash them before you can switch branches.
> Aborting
>
> I'm sure if I thought about it enough (ie re-read Andreas's post a couple
> more times) I'd be able to understand why git gets it right sometimes but
> not other times, but I'm too tired right now. Even when I *am* awake and
Git gets it "right" (by your definition) when file1.txt on one branch
is different from file1.txt on the other branch. That means that
switching branches would require changing the file, so it refuses to
overwrite your changes by doing so. If it CAN switch branches without
losing your changes, it does.
The fundamental problem is that you're thinking of the changes to the
working tree (which aren't commited) as being "on" some branch. Until
they're committed, changes in the working tree are only in the working
tree. That's basically the difference between "committed" and "not
committed".
-PJ
^ 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