Git development
 help / color / mirror / Atom feed
* [PATCH 1/3] rev-list: fix finish_object() call
From: Junio C Hamano @ 2011-08-17 21:30 UTC (permalink / raw)
  To: git
In-Reply-To: <1313616635-25331-1-git-send-email-gitster@pobox.com>

The callback to traverse_commit_list() are to take linked name_path and
a string for the last path component.

If the callee used its parameters, it would have seen duplicated leading
paths. In this particular case, the callee does not use this argument but
that is not a reason to leave the call broken.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/rev-list.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 56727e8..d789279 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -182,7 +182,7 @@ static void show_object(struct object *obj, const struct name_path *path, const
 	 */
 	const char *ep = strchr(name, '\n');
 
-	finish_object(obj, path, name);
+	finish_object(obj, path, component);
 	if (ep) {
 		printf("%s %.*s\n", sha1_to_hex(obj->sha1),
 		       (int) (ep - name),
-- 
1.7.6.472.g4bfe7c

^ permalink raw reply related

* [PATCH 0/3] Minor "list-objects" updates
From: Junio C Hamano @ 2011-08-17 21:30 UTC (permalink / raw)
  To: git

Unify two codepaths that list objects in "rev-list --objects" format, and
get rid of unnecessary malloc()/free() done per each line of output.

Junio C Hamano (3):
  rev-list: fix finish_object() call
  revision.c: add show_object_with_name() helper function
  revision.c: update show_object_with_name() without using malloc()

 builtin/rev-list.c |   17 ++---------------
 revision.c         |   41 +++++++++++++++++++++++++++++++++++++++++
 revision.h         |    2 ++
 upload-pack.c      |   15 +--------------
 4 files changed, 46 insertions(+), 29 deletions(-)

-- 
1.7.6.472.g4bfe7c

^ permalink raw reply

* Re: Branches & directories
From: Hilco Wijbenga @ 2011-08-17 21:23 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Evan Shelhamer, Git Mailing List
In-Reply-To: <7vvctvdf5r.fsf@alter.siamese.dyndns.org>

On 17 August 2011 12:14, Junio C Hamano <gitster@pobox.com> wrote:
> Evan Shelhamer <shelhamer@imaginarynumber.net> writes:
>
>> (in repo on your branch)
>> mkdir test_dir
>> touch test_dir/.gitkeep
>> touch test_file
>> git add *
>> git commit -m "test commit (with directory)"
>>
>> Will commit the directory as expected in your branch, and when you go
>> to checkout another branch it will not exist.
>
> ... unless you have untracked files in test_dir/, that is.
>
> If the branch you are switching to does not have anything at test_dir, you
> will not lose the untracked file because git does not remove test_dir/ nor
> its untracked contents.
>
> Depending on what you have at the path "test_dir" in the branch you are
> switching to, the actual rules are a bit more subtle than that, though.

Yes, I should have thought of that. The problem is with .gitignore. I
have a number of files ignored that are important but can't be shared.
So when I switch to 'master' the directory is not removed because it
contains untracked, ignored files. And, of course, git status does not
complain about them either because they are in .gitignore. :-)

This is still very annoying but hardly Git's fault (Git is working as
intended). It would be really nice, though, if Git could somehow
"stash" such files when checking out a different branch. In general, I
would prefer if uncommitted changes and untracked and/or ignored files
stuck to the branch where they were created. If I want to take them
with me when I switch branches that should be possible but not the
default behaviour. Is anything like that possible? Are there
configuration options for this?

^ permalink raw reply

* Re: [RFC/PATCH v3] git-svn: New flag to emulate empty directories
From: Eric Wong @ 2011-08-17 20:39 UTC (permalink / raw)
  To: Ray Chen; +Cc: git, Michael Haggerty, Michael J Gruber
In-Reply-To: <1311201446-26895-1-git-send-email-rchen@cs.umd.edu>

Ray Chen <rchen@cs.umd.edu> wrote:
> Is anybody else willing to review this patch?  I'd feel better if we had more
> than two sets of eyes look this over.

We haven't heard anything in a few weeks.  Everything looks acceptable
to me.  I'm inclined to just accept it since it's unlikely to break
existing functionality.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH v2] git svn : hook before 'git svn dcommit'
From: Eric Wong @ 2011-08-17 20:37 UTC (permalink / raw)
  To: Frédéric Heitzmann; +Cc: git, Junio C Hamano
In-Reply-To: <CALeToSUidWgiJ+PxuphnBZOQGNStNOO9==EswfLDpFr2GYy=nA@mail.gmail.com>

Frédéric Heitzmann <frederic.heitzmann@gmail.com> wrote:
> 4) Before merging back to master and commitng to SVN, it is necessary
> to remove commits with reference data (git rebase -i --onto master
> master topic ...)
> 5) merge topic branch with master and git svn dcommit
> 
> -- end --
> 
> It is very easy to forget step 4, and svn commit lots of useless data.

I agree.

> Proposal 1)
> * commit reference data with some specific mark in the commit message
> (e.g. "NO_SVN")
> * use pre-svn-dcommit hook to detect such commits

The problem with this is hook standardization across committers and even
across different machines/directories a committer may use.

> Proposal 2) (not fully feasable for what I know)
> * git svn clone to a bare repo
> * clone a working repo from the the bare repo.
> * steps 2, 3, maybe 3bis, ... then 4
> * push commits to the bare repo, while using pre-receive or update
> hook to look for wrong commits, and abort if so.
> * use post-receive hook to trigger git svn dcommit
> 
> Main drawback for proposal 2 (appart from needing 2 repo instead of
> one) is that each time you want to update your working  repo, you have
> to git svn rebase the bare repo, then git pull.

Proposal 2 is way too complicated, I hate it.

> All things begin equal, proposal 1 seems to be the easier path, but it
> is highly debatable.

I had Proposal 3 in my original response:

> 2011/8/17 Eric Wong <normalperson@yhbt.net> wrote:
> > Perhaps an interactive option for dcommit would be just as useful?

1 and 3 can both implemented, but I think 3 would be easier to
use/setup/standardize.  I suspect it's also easier to train oneself to
always use "dcommit -i".  Perhaps even default to interactive mode
like git-send-email does nowadays.

Unfortunately interactive dcommit requires more effort to implement.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH 7/7] sequencer: Remove sequencer state after final commit
From: Ramkumar Ramachandra @ 2011-08-17 20:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Jonathan Nieder, Git List, Christian Couder, Daniel Barkalow,
	Jeff King
In-Reply-To: <7v7h6eld2c.fsf@alter.siamese.dyndns.org>

Hi Junio,

Junio C Hamano writes:
> After sleeping on this, here are my random thoughts on this topic.

They're a bit much for me to absorb so quickly.  I'll think about this
again before posting a more elaborate reply.

Thanks for your patience.

-- Ram

^ permalink raw reply

* [PATCH RFC] gitk: Allow commit editing
From: Michal Sojka @ 2011-08-17 19:56 UTC (permalink / raw)
  To: git; +Cc: paulus, Michal Sojka

Hi, this is a proof of concept patch that allows editing of commits
from gitk. I often review patches before pushing in gitk and if I
would like to have an easy way of fixing typos in commit messages etc.

So the patch adds "Edit this commit" item to gitk's context menu and
the actual editing is done by non-interactively invoking interactive
rebase :-) and git gui.

There is almost no error checking etc., but before learning TCL and
coding this properly, I'd like to get some feedback.

-Michal

Signed-off-by: Michal Sojka <sojka@os.inf.tu-dresden.de>
---
 gitk-git/gitk |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4cde0c4..121b926 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2497,6 +2497,7 @@ proc makewindow {} {
 	{mc "Return to mark" command gotomark}
 	{mc "Find descendant of this and mark" command find_common_desc}
 	{mc "Compare with marked commit" command compare_commits}
+	{mc "Edit this commit" command edit_commit}
     }
     $rowctxmenu configure -tearoff 0
 
@@ -9102,6 +9103,21 @@ proc cherrypick {} {
     notbusy cherrypick
 }
 
+proc edit_commit {} {
+    global rowmenuid
+
+    nowbusy edit [mc "Editing commit"]
+    if {[catch {exec sh -c "(GIT_EDITOR='sed -ie 1s/^pick/edit/' git rebase -i $rowmenuid^ && git gui citool --amend && git rebase --continue) 2>&1"} err]} {
+	notbusy edit
+	error_popup $err
+	exec git rebase --abort
+	return
+    }
+    notbusy edit
+    run updatecommits
+}
+
+
 proc resethead {} {
     global mainhead rowmenuid confirm_ok resettype NS
 
-- 
1.7.5.4

^ permalink raw reply related

* Реклама на теле купить за 10 тысяч рублей?  Фотоаппарат… или...
From: Aleksashin @ 2011-08-17 20:49 UTC (permalink / raw)
  To: git

Реклама на теле теперь и в России
http://s47.radikal.ru/i118/1108/95/f3a1dfde4e70.jpg

Что можно купить за 10 тысяч рублей?
Мобильный телефон…
Или кусок человеческой кожи……..

10 тысяч рублей - достаточная сумма, чтобы украсить себя рекламой?

Узнать больше об акции можно здесь: 
http://clck.ru/I_xI

^ permalink raw reply

* Re: [PATCH] update-index: add --swap to swap index and worktree content
From: Jeff King @ 2011-08-17 19:46 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Nguyen Thai Ngoc Duy, Junio C Hamano, git
In-Reply-To: <CAOeW2eFx-ETS+1a5b2bUXeT8JJ0WGudKGyF6mxuqyK2OM35qQA@mail.gmail.com>

On Wed, Aug 17, 2011 at 10:13:08AM -0400, Martin von Zweigbergk wrote:

> >> Two reasons. I already mentioned the ability to quickly checkout index
> >> for a quick test (though there could be more problems down that road).
> >
> > That's a good thought. However, in practice, I find I also need other
> > files from the index to do a successful test. So I end up just
> > committing what I think is right, and then afterwards do:
> 
> Maybe a stupid question, but "update-index --swap" would swap all
> files, right? So what "other files from the index" would there be?

Hmm. I hadn't really considered swapping everything. I suppose that
would work, though I do worry about getting into a confused state when
you have swapped one or more files individually, and then want to swap
the rest out for testing.

That is, there is no flag to say "this file was swapped", so there is no
way to say "swap all of the index files out, except those I have already
swapped". But if you were disciplined to only do:

  git update-index --swap
  $EDITOR foo.c
  make test
  git update-index --swap

and never:

  git update-index --swap foo.c
  $EDITOR foo.c
  git update-index --swap ;# oops, we put the WT foo.c back
  make test
  git update-index --swap

then it would work.

-Peff

^ permalink raw reply

* Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision
From: Elijah Newren @ 2011-08-17 19:36 UTC (permalink / raw)
  To: Owen Stephens; +Cc: git
In-Reply-To: <CAORuUR154Dhg5vDojga-01bDxxf+=R2X-oJK-0417CgmqxCwvA@mail.gmail.com>

Hi,

On Wed, Aug 17, 2011 at 10:21 AM, Owen Stephens <git@owenstephens.co.uk> wrote:
> fast-export gives an invalid branch name for commits, if the revision specified
> is not HEAD or a branch name.
>
> $ git --version
> git version 1.7.6
>
> $ # Create
> $ git init export_test && cd export_test
> $ touch a && git add a && git commit -m 'Add a'
> $ touch b && git add b && git commit -m 'Add b'
> $ git branch branch1
>
> $ # ok
> $ git fast-export HEAD
>
> $ # also ok
> $ git fast-export branch1
>
> $ # uses HEAD~1 instead of refs/heads/master
> $ git fast-export HEAD~1
>
> blob
> mark :1
> data 0
>
> reset HEAD~1
> commit HEAD~1

Thanks for the report.  It turns out this bug has been reported and is
in the testsuite as t9350.19 -- currently marked as expected to fail.
I looked at the problem a couple years ago for a little bit but never
finished that particular patch and never got back around to it.

^ permalink raw reply

* Re: Branches & directories
From: Junio C Hamano @ 2011-08-17 19:14 UTC (permalink / raw)
  To: Evan Shelhamer; +Cc: Hilco Wijbenga, Git Mailing List
In-Reply-To: <CABNdCrCbSqup1=D2eEbGDhw3JzZGYHWLVqZFsB6GDO4Vk7HRxg@mail.gmail.com>

Evan Shelhamer <shelhamer@imaginarynumber.net> writes:

> (in repo on your branch)
> mkdir test_dir
> touch test_dir/.gitkeep
> touch test_file
> git add *
> git commit -m "test commit (with directory)"
>
> Will commit the directory as expected in your branch, and when you go
> to checkout another branch it will not exist.

... unless you have untracked files in test_dir/, that is.

If the branch you are switching to does not have anything at test_dir, you
will not lose the untracked file because git does not remove test_dir/ nor
its untracked contents.

Depending on what you have at the path "test_dir" in the branch you are
switching to, the actual rules are a bit more subtle than that, though.

^ permalink raw reply

* Re: [PATCH] log: decorate grafted commits with "grafted"
From: Junio C Hamano @ 2011-08-17 18:48 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1313593326-12077-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> @@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
>  	log.parent = NULL;
>  	opt->loginfo = &log;
>  
> +	if (!commit->parents && lookup_commit_graft(commit->object.sha1))
> +		add_name_decoration(DECORATION_GRAFTED, "grafted",
> +				    &commit->object);

I am not very enthused about this change.

We have already looked up the commit when we parsed it, and then we again
have to call lookup_commit_graft() which would yield false for most of the
commits?

Does this work with replacements and shallows, by the way?

^ permalink raw reply

* Re: Branches & directories
From: Evan Shelhamer @ 2011-08-17 18:47 UTC (permalink / raw)
  To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi3Eg88i+1s+CcW3+W0WNZ-NYUQb1EV55oh+g1Od78AByQ@mail.gmail.com>

Hey Hilco,

I'm not sure exactly what you did because you didn't give a list of
git commands, but I'm guessing you ran into the fact that git doesn't
track empty directories.
For instance if I do:

(in repo on your branch)
mkdir test_dir
touch test_file
git add *
git commit -m "test commit"

test_file will be tracked but test_dir will not (because it is empty).
To avoid this problem, a convention is to add an empty ".gitkeep" file
to directories. For example:

(in repo on your branch)
mkdir test_dir
touch test_dir/.gitkeep
touch test_file
git add *
git commit -m "test commit (with directory)"

Will commit the directory as expected in your branch, and when you go
to checkout another branch it will not exist.

Hope that helps. Sorry if you already know this and I misunderstood
your question.
Evan Shelhamer

On Wed, Aug 17, 2011 at 2:35 PM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> Hi all,
>
> I have been noticing strange behaviour that I would like to be able to
> explain or report as a bug as the case may be.
>
> What happens is that I create and commit a new directory in branch
> 'next' and then when I checkout 'master' this new directory is still
> there. I think this is wrong as this new directory does not exist yet
> in 'master'. Is my understanding correct?
>
> I tried recreating this scenario in a clean Git repo with a simple
> mkdir and commit but when I did a checkout of 'master' the new
> directory was removed. So the basic scenario seems to work the way I
> expect it to.
>
> Assuming I ran into a bug, I would like some suggestions to properly
> investigate this. Clearly, I'm doing something else that triggers the
> behaviour I'm seeing but I'm not sure what it is. What might trigger
> Git "remembering" a directory? Or what would prevent it from removing
> a directory when checking out a different branch?
>
> Extra information: "git status" (in 'master') yields nothing. But
> after adding a new file in the directory-that-should-not-be-there, Git
> treats the entire directory as untracked and new (as one would
> expect). I can also safely remove the directory with no (obvious) ill
> effects.
>
> Cheers,
> Hilco
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH] stash: Utilize config variable pager.stash.list in stash list command
From: Junio C Hamano @ 2011-08-17 18:44 UTC (permalink / raw)
  To: Ingo Brückl; +Cc: git
In-Reply-To: <4e4b8418.068889fb.bm001@wupperonline.de>

At least "pager.stash.list" should be spelled as "pager.stashList" or
something. It is not like there are multitude of arbigrary choices that
may match "pager.*.list" pattern.

Also a variable can be set to false by setting it to 0, no, etc., so you
need to inspect it with "git config --bool" to get the canonical version
of its value.

What's so difficult to say "git stash list | less" or even "git -p stash list"?

^ permalink raw reply

* Re: [PATCH] Disallow creating ambiguous branch names by default
From: Junio C Hamano @ 2011-08-17 18:41 UTC (permalink / raw)
  To: Conrad Irwin; +Cc: git
In-Reply-To: <1313569298-3879-1-git-send-email-conrad.irwin@gmail.com>

Conrad Irwin <conrad.irwin@gmail.com> writes:

> Before this change, it was comparatively easy to create a confusingly

Drop everything before the ", ".

> named branch (like "origin/master" or "tag.1"). The former case is
> particularly biting to newcomers, who suddenly find themselves needing
> to handle nuances of the refs namespaces.

If you start forbidding certain names, newcomers will need to be exposed
the same nuances to understand why what they wanted to do is not allowed,
so that is not an argument.

My preferences (take them as "the ground rules" if you want) are:

 - We don't disallow what we have long allowed, without a good reason;
 - We make sure new people will get a warning with useful advice.

I would be happy to see the end result that warns when the end user
creates a branch (or a tag) that is ambiguous _when_ it is created (not
"much later, when we noticed there are ambiguous refs"), and offers an
advice message to use "branch -m" to rename it away (control the message
with a new "advice.*" configuration and unless explicitly declined with
it, always give the advice).

> In both cases, git commands would omit a warning about "ambiguous refs"
> if they noticed that this had occurred,

Assuming that you meant s/omit/emit/, I do agree that what we do right now
is suboptimal. I just tried these two:

	$ git branch v1.0.0
        $ git checkout v1.0.0
        warning: refname 'v1.0.0' is ambigous.
	$ git branch -m v1.0.0-branch

        $ git checkout -b v1.0.0
        Switched to a new branch 'v1.0.0'
	$ git checkout v1.0.0
        warning: refname 'v1.0.0' is ambigous.
	Already on 'v1.0.0'
	$ git branch -m v1.0.0-branch-2

We should be giving these warning messages immediately after creating
potentially problematic refs, i.e. just after "git branch v1.0.0" and
"git checkout -b v1.0.0". The user experience should look like this
instead:

	$ git branch v1.0.0
        warning: refname 'v1.0.0' is ambiguous.
        advice: you may want to rename it to an unambigous name with
        advice: git branch -m v1.0.0 v1.0.0-branch
	$ git branch -m v1.0.0 v1.0.0-branch ;# thanks for an advice

        $ git checkout -b v1.0.0
        warning: refname 'v1.0.0' is ambiguous.
        advice: you may want to rename it to an unambigous name with
        advice: git branch -m v1.0.0-branch-2
	$ git branch -m v1.0.0-branch-2 ;# thanks for an advice

^ permalink raw reply

* Branches & directories
From: Hilco Wijbenga @ 2011-08-17 18:35 UTC (permalink / raw)
  To: Git Users

Hi all,

I have been noticing strange behaviour that I would like to be able to
explain or report as a bug as the case may be.

What happens is that I create and commit a new directory in branch
'next' and then when I checkout 'master' this new directory is still
there. I think this is wrong as this new directory does not exist yet
in 'master'. Is my understanding correct?

I tried recreating this scenario in a clean Git repo with a simple
mkdir and commit but when I did a checkout of 'master' the new
directory was removed. So the basic scenario seems to work the way I
expect it to.

Assuming I ran into a bug, I would like some suggestions to properly
investigate this. Clearly, I'm doing something else that triggers the
behaviour I'm seeing but I'm not sure what it is. What might trigger
Git "remembering" a directory? Or what would prevent it from removing
a directory when checking out a different branch?

Extra information: "git status" (in 'master') yields nothing. But
after adding a new file in the directory-that-should-not-be-there, Git
treats the entire directory as untracked and new (as one would
expect). I can also safely remove the directory with no (obvious) ill
effects.

Cheers,
Hilco

^ permalink raw reply

* Re: [PATCH] update-index: add --swap to swap index and worktree content
From: Junio C Hamano @ 2011-08-17 18:26 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Jeff King, Nguyen Thai Ngoc Duy, git
In-Reply-To: <CAOeW2eFx-ETS+1a5b2bUXeT8JJ0WGudKGyF6mxuqyK2OM35qQA@mail.gmail.com>

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> Maybe the above shows that I'm missing something. Still, would it be
> possible to achieve the use case with something like the following?
>
> git stash --keep-index

While I sense a vague aversion to committing in general in this
discussion, which I am not particularly fond of, the whole point of
"stash" is to avoid the mental burden of going over the "hump" of
committing something not ready and to replace it with a "save it as a
temporary state" that technically has the same overhead of committing but
has a lot less mental overhead. Perhaps "swapping the state of all and/or
selected paths" fits better in "stash", not "update-index"?

I dunno, but in general, a new feature to cater to _common_ end user needs
should fit in the Porcelain layer. We would be doing something wrong if we
need to teach an obscure option of lower plumbing to end users.

I personally used to feel that "ls-files -u" during a conflicted merge to
be the single sore-thumb that stuck out from this point of view, but these
days "status -s" gives the same information in a more useful way to the
end users, and I am reasonably happy with that. It may be that the end
user operation (perhaps "stash --swap", but I am not married to that) that
fits well in common workflows ends up using "update-index --swap" as an
underlying implementation detail, but I'd prefer to see how the final end
user experience using Porcelain would look like first.

^ permalink raw reply

* Re: [PATCH v2] commit: accept tag objects in HEAD/MERGE_HEAD
From: Junio C Hamano @ 2011-08-17 17:59 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1313545369-7096-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> HEAD, MERGE_HEAD (or any other branches) should only have SHA-1 of a
> commit object. However broken tools can put a tag object there. While
> it's wrong, it'd be better to tolerate the situation and move on

The best part in your patch is that you made it _warn_ when it happens; I
would suggest rewording this with s/situation/&, warn/.

> ("commit" is an often used operation, unable to commit could be bad).

Neither "often used" nor "unable to commit" is a good reason for this
added leniency. The real reason is that such a condition left by broken
tools is cumbersome to fix by an end user with:

	$ git update-ref HEAD $(git rev-parse HEAD^{commit})

which may look like a magic to a new person.

By the way, what happens when you try to merge when HEAD points at a tag
that points at a commit? Would we end up creating a commit that points at
a bogus parent?

> diff --git a/builtin/commit.c b/builtin/commit.c
> index 2088b6b..f327595 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1387,6 +1387,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  	unsigned char commit_sha1[20];
>  	struct ref_lock *ref_lock;
>  	struct commit_list *parents = NULL, **pptr = &parents;
> +	struct commit *commit;
>  	struct stat statbuf;
>  	int allow_fast_forward = 1;
>  	struct wt_status s;

Here, you are being inconsistent with your own argument you made in your
previous message "later somebody may forget to update the former while
updating the latter" when I suggested to separate the two logically
separate operations (grab the head_commit object when necessary, and
decide how the commit is made). By hoisting of the scope of "commit", you
made the variable undefined when dealing with the initial_commit, exposing
the code to the same risk that somebody may try to use "commit" variable
after the if/elseif/... cascade, where it may or may not be defined.

Not that I buy your previous argument in this case---it's not like we have
deeply nested callchain that sometimes sets a variable and sometimes
doesn't. It's all there for the updater to see in a single function.

> @@ -1423,12 +1424,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  			reflog_msg = "commit (initial)";
>  	} else if (amend) {
>  		struct commit_list *c;
> -		struct commit *commit;
>  
>  		if (!reflog_msg)
>  			reflog_msg = "commit (amend)";
> -		commit = lookup_commit(head_sha1);
> -		if (!commit || parse_commit(commit))
> +		commit = lookup_expect_commit(head_sha1, "HEAD");
> +		if (parse_commit(commit))
>  			die(_("could not parse HEAD commit"));

Is this still necessary? I think your lookup_expect_commit() already
checks this condition and barfs.

^ permalink raw reply

* Re: [PATCH] checkout-index: remove obsolete comment
From: Junio C Hamano @ 2011-08-17 17:40 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1313583733-7514-1-git-send-email-pclouds@gmail.com>

Thanks; this will go traight to 'maint'.

^ permalink raw reply

* Malformed branch name in fast-export when specifying non-HEAD/branch revision
From: Owen Stephens @ 2011-08-17 16:21 UTC (permalink / raw)
  To: git
In-Reply-To: <CAORuUR1viqG27+dYOFS_5SLxFOE2wHJqAQ3i3RByg_fbWACh-Q@mail.gmail.com>

Hi,

fast-export gives an invalid branch name for commits, if the revision specified
is not HEAD or a branch name.

$ git --version
git version 1.7.6

$ # Create
$ git init export_test && cd export_test
$ touch a && git add a && git commit -m 'Add a'
$ touch b && git add b && git commit -m 'Add b'
$ git branch branch1

$ # ok
$ git fast-export HEAD

$ # also ok
$ git fast-export branch1

$ # uses HEAD~1 instead of refs/heads/master
$ git fast-export HEAD~1

blob
mark :1
data 0

reset HEAD~1
commit HEAD~1
mark :2
author Owen Stephens <git@owenstephens.co.uk> 1313597436 +0100
committer Owen Stephens <git@owenstephens.co.uk> 1313597436 +0100
data 6
Add a
M 100644 :1 a

$ # Uses the commit hash rather than refs/heads/master
$ git fast-export $(git rev-parse HEAD~1)

blob
mark :1
data 0

reset 5a8a8abb1ab44890501c64f2d51f671ab2108d60
commit 5a8a8abb1ab44890501c64f2d51f671ab2108d60
mark :2
author Owen Stephens <git@owenstephens.co.uk> 1313597436 +0100
committer Owen Stephens <git@owenstephens.co.uk> 1313597436 +0100
data 6
Add a
M 100644 :1 a

$ # fast-import cannot handle this stream:
$ git init subgit
$ git fast-export HEAD~1 | (cd subgit/ && git fast-import)
fatal: Branch name doesn't conform to GIT standards: HEAD~1
fast-import: dumping crash report to .git/fast_import_crash_26863

Regards,
Owen.

^ permalink raw reply

* Re: [PATCH v2] xdiff/xprepare: improve O(n*m) performance in xdl_cleanup_records()
From: Tay Ray Chuan @ 2011-08-17 15:55 UTC (permalink / raw)
  To: Jeff King; +Cc: Git Mailing List, Junio C Hamano, Marat Radchenko
In-Reply-To: <20110817052147.GA11253@sigill.intra.peff.net>

On Wed, Aug 17, 2011 at 1:21 PM, Jeff King <peff@peff.net> wrote:
> Wait, what? It was using 0 seconds of user time before, but still taking
> 8.5 seconds? What was it doing? Did you actually warm up your disk cache
> before taking these measurements?

Three runs on the same machine, after a restart.

  $ time git show >/dev/null

  real    0m6.505s
  user    0m0.031s
  sys     0m0.015s

  rctay@TEST-123 /tmp/slono
  $ time git show >/dev/null

  real    0m5.429s
  user    0m0.000s
  sys     0m0.031s

  rctay@TEST-123 /tmp/slono
  $ time git show >/dev/null

  real    0m5.336s
  user    0m0.000s
  sys     0m0.016s

-- 
Cheers,
Ray Chuan

^ permalink raw reply

* [PATCH] log: decorate grafted commits with "grafted"
From: Nguyễn Thái Ngọc Duy @ 2011-08-17 15:02 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This would help decide whether to deepen some more on shallow
repositories, or the branch really has really ends there.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 I briefly considered wrapping _() around "grafted" text, but
 decided that making it a keyword might help searching

 log-tree.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index e945701..c469341 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -18,6 +18,7 @@ enum decoration_type {
 	DECORATION_REF_TAG,
 	DECORATION_REF_STASH,
 	DECORATION_REF_HEAD,
+	DECORATION_GRAFTED
 };
 
 static char decoration_colors[][COLOR_MAXLEN] = {
@@ -27,6 +28,7 @@ static char decoration_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_BOLD_YELLOW,	/* REF_TAG */
 	GIT_COLOR_BOLD_MAGENTA,	/* REF_STASH */
 	GIT_COLOR_BOLD_CYAN,	/* REF_HEAD */
+	GIT_COLOR_BOLD_BLUE,	/* GRAFTED */
 };
 
 static const char *decorate_get_color(int decorate_use_color, enum decoration_type ix)
@@ -638,6 +640,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	log.parent = NULL;
 	opt->loginfo = &log;
 
+	if (!commit->parents && lookup_commit_graft(commit->object.sha1))
+		add_name_decoration(DECORATION_GRAFTED, "grafted",
+				    &commit->object);
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
-- 
1.7.4.74.g639db

^ permalink raw reply related

* Re: shallow clone not very shallow due to tags
From: Nguyen Thai Ngoc Duy @ 2011-08-17 14:35 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: git
In-Reply-To: <CAJo=hJvySfuWmz6vGUveC5KoR1NZGC4ji0saByVX40UxLqHqdg@mail.gmail.com>

On Wed, Aug 17, 2011 at 9:19 PM, Shawn Pearce <spearce@spearce.org> wrote:
> Of course a placeholder ref cannot be used as a starting point to
> checkout something, can it? It has no commit object available. So if
> you did decide to checkout a placeholder, you would need to first
> fetch that placeholder. So why not use git ls-remote to list the
> remotely available branches?

Ah.. ls-remote is the answer. Thanks, placeholders are gone (for me).
-- 
Duy

^ permalink raw reply

* Re: [PATCH v2] git svn : hook before 'git svn dcommit'
From: Frédéric Heitzmann @ 2011-08-17 14:35 UTC (permalink / raw)
  To: Eric Wong; +Cc: git, Junio C Hamano
In-Reply-To: <CALeToSWJNK=q4iPwxNvgGin0T61oLKJd=b9F3cSSo0vVebrhhQ@mail.gmail.com>

Hi all.

Maybe I should give some more context to explain why a hook could be a
potential improvement.

Let's consider the following workflow :
1) git svn clone from the SVN server, then git checkout -b topic
2) git commit some "reference data", before starting some optimization
or code refactoring.
** These reference data are not supposed to find their way to the SVN server **
Committing such "reference data" is just a convenience because git
does a great job to show how these data may or may not change during
the development process.
3) hack, test, commit ...
3 bis) it may happen that reference data change for some very good
reason (for instance some protocol change)
New reference data are then commited.

back to 3 ...

4) Before merging back to master and commitng to SVN, it is necessary
to remove commits with reference data (git rebase -i --onto master
master topic ...)
5) merge topic branch with master and git svn dcommit

-- end --

It is very easy to forget step 4, and svn commit lots of useless data.

Proposal 1)
* commit reference data with some specific mark in the commit message
(e.g. "NO_SVN")
* use pre-svn-dcommit hook to detect such commits

Proposal 2) (not fully feasable for what I know)
* git svn clone to a bare repo
* clone a working repo from the the bare repo.
* steps 2, 3, maybe 3bis, ... then 4
* push commits to the bare repo, while using pre-receive or update
hook to look for wrong commits, and abort if so.
* use post-receive hook to trigger git svn dcommit

Main drawback for proposal 2 (appart from needing 2 repo instead of
one) is that each time you want to update your working  repo, you have
to git svn rebase the bare repo, then git pull.

Proposal 2bis)
* add a pre-send hook on the bare repo, and trigger some git svn
rebase with this hook.
I am not sure to see all the potential consequences of such a hook though.

All things begin equal, proposal 1 seems to be the easier path, but it
is highly debatable.

--
Fred

ps : I had to resend this email because first attempt included HTML
(sic). Very sorry if you receive it twice.

2011/8/17 Eric Wong <normalperson@yhbt.net>
>
> Junio C Hamano <gitster@pobox.com> wrote:
> > Frédéric Heitzmann  <frederic.heitzmann@gmail.com> writes:
> >
> > > The 'pre-svn-dcommit' hook is called before 'git svn dcommit', which aborts
> > > if return value is not zero. The only parameter given to the hook is the
> > > reference given to 'git svn dcommit'. If no paramter was used, hook gets HEAD
> > > as its only parameter.
> >
> > It appears that this is in the same spirit as the pre-commit hook used in
> > "git commit", so it may not hurt but I do not know if having a separate
> > hook is the optimal approach to achieve what it wants to do.
> >
> > I notice that git-svn users have been happily using the subsystem without
> > need for any hook (not just pre-commit). Does "git svn" need an equivalent
> > of pre-commit hook? If so, does it need equivalents to other hooks as
> > well? I am not suggesting you to add support for a boatload of other hooks
> > in this patch---I am trying to see if this is really a necessary change to
> > begin with.
> >
> > Eric, do you want this one?
>
> I'm not sure.  I feel hooks should be avoided whenever possible, and
> a git-svn-specific hook for dcommit wouldn't place the same restriction
> as a server-side SVN hook for svn(1) users.
>
> Preventing certain commits from accidentally hitting the SVN server can
> be useful, I think.  On the other hand, I'm not sure if people who run
> accidental dcommits would remember to the pre-dcommit hook, either.
>
> Perhaps an interactive option for dcommit would be just as useful?
>
> Test cases are required for any new features of git-svn, though.
>
> > > +   system($hook, $head);
> > > +   if ($? == -1) {
> > > +           print "[pre_svn_dcommit_hook] failed to execute $hook: $!\n";
> > > +           return 1;
> > > +   } elsif ($? & 127) {
> > > +           printf "[pre_svn_dcommit_hook] child died with signal %d, %s coredump\n",
> > > +           ($? & 127),  ($? & 128) ? 'with' : 'without';
> > > +           return 1;
> > > +   } else {
> > > +           return $? >> 8;
> > > +   }
> > > +}
> >
> > Should these messages go to the standard output?
>
> Failure messages should definitely go to stderr.
>
> --
> Eric Wong

^ permalink raw reply

* Re: [PATCH] update-index: add --swap to swap index and worktree content
From: Nguyen Thai Ngoc Duy @ 2011-08-17 14:32 UTC (permalink / raw)
  To: Martin von Zweigbergk; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <CAOeW2eFx-ETS+1a5b2bUXeT8JJ0WGudKGyF6mxuqyK2OM35qQA@mail.gmail.com>

On Wed, Aug 17, 2011 at 9:13 PM, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> Hi,
>
> On Tue, Aug 16, 2011 at 10:17 PM, Jeff King <peff@peff.net> wrote:
>> On Wed, Aug 17, 2011 at 09:11:32AM +0700, Nguyen Thai Ngoc Duy wrote:
>>
>>> Two reasons. I already mentioned the ability to quickly checkout index
>>> for a quick test (though there could be more problems down that road).
>>
>> That's a good thought. However, in practice, I find I also need other
>> files from the index to do a successful test. So I end up just
>> committing what I think is right, and then afterwards do:
>
> Maybe a stupid question, but "update-index --swap" would swap all
> files, right? So what "other files from the index" would there be?

That's some of the "problems down that road", another one being
removed files in index. Still the ability to do a quick compile on a
single file is cool.

> Maybe the above shows that I'm missing something. Still, would it be
> possible to achieve the use case with something like the following?
>
> git stash --keep-index
> # fix up files, test etc.
> git add ....
> git commit
> git cherry-pick --strategy=theirs stash && git stash drop

Pretty much (if I understand git-stash correctly, I haven't used it
much), except that if I change my mind mid way, I can switch back to
the original state and keep working:

1. git add -p ...
2. git update-index --swap foo # index <-> wt
3. # make some minor edit, "make foo.o", oops failed, I missed some hunks
4. git checkout -p ...
5. # still not happy, more changes should be made on both index and wt
6. git update-index --swap foo # now index is index, wt is wt
7. # edit some more, when ready to commit, repeat steps 2-6 again
8. git commit
-- 
Duy

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox