Git development
 help / color / mirror / Atom feed
* Re: If you would write git from scratch now, what would you change?
From: Adam Roben @ 2007-11-26  6:36 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Steven Walter, Pierre Habouzit, Jakub Narebski, git
In-Reply-To: <7vejedh6xl.fsf@gitster.siamese.dyndns.org>

Junio C Hamano wrote:
> Steven Walter <stevenrwalter@gmail.com> writes:
>   
>> Heartily seconded.  I think checkout is the most egregrious of the
>> three.  git-checkout can be used to:
>>
>>     * Switch branches
>>     * Create a branch
>>     * Change the state of all files to a particular commit
>>     * Change the state of a particular file to that of the index
>>     * Change the state of a particular file (and index) to a particular
>>       commit
>>     
>
> Come on.  The second one is just to give a short-hand side-effet for
> commonly used operation and you do not have to use it nor learn it.
>   

I think the overwhelming majority of git users learn `git checkout -b`. 
The cases where you do want to switch to a branch you just created seem 
far more common than the cases where you don't (particularly for new 
users), which is the whole reason the -b option exists in the first 
place. So I don't think it's reasonable to say "you can choose not to be 
confused by ignoring this incredibly useful command."

> Let's clear the confusion.  Although it is not bad like the above
> "random 5 different operations", checkout does serve 2 quite different
> purposes:
>
>  (1) checkout a revision.
>  (2) checkout selected paths out of a commit (or the index).
>   

Given the above, I'd argue that it serves 3 purposes:

   (1) check out a revision
   (2) check out selected paths out of a commit (or the index)
   (3) start working on a new branch

It's true that (1) and (3) are very closely related, but I think in the 
minds of many git users (particularly new ones) they are distinct. (2) 
really seems the most out of place here, and has the most potential for 
finding a new home (perhaps within git-reset).

-Adam

^ permalink raw reply

* Re: [PATCH] Use is_absolute_path() in diff-lib.c, lockfile.c, setup.c, trace.c
From: Steffen Prohaska @ 2007-11-26  6:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7clhd9z.fsf@gitster.siamese.dyndns.org>


On Nov 26, 2007, at 4:54 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> Using the helper function to test for absolute paths makes porting  
>> easier.
>
> These probably make sense.  I obviously do not see any downside  
> from the
> POSIX side, and can imagine that treating "C:\" prefix as "absolute
> paths" at these four places will not have any ill effect on the  
> Windows
> side (IOW, the codepaths that follow these four places seem to do a
> sensible thing even if the "absolute path" prefix is not a single '/',
> but would work fine as-is).
>
> I am a bit surprised that there are only four places you needed to
> touch, though.

Yes, I was a bit surprised, too.  I used grep to find these places.
Maybe my regular expression was not good enough.  On the other side,
is_absolute_path() is already used at 11 places before this patch.
I also cross checked with the msysgit code base.  It does not use
is_absolute_path() at more places.


>> BTW, what happend to the msysgit related patches:
>>
>> [PATCH 1/3] sha1_file.c: Fix size_t related printf format warnings
>> [PATCH 2/3] builtin-init-db: use get_git_dir() instead of getenv()
>>
>> I never received comments about them, nor do I find them on pu.
>
> Lack of comments was probably due to mixture of bad timing and general
> lack of interests.  Many people are busy working on their turkeys than
> hacking this time of the year ;-)

Yeah, list traffic was quiet low.


> I am reluctant to queue msysgit/gitwin related patches without seeing
> positive comments from other people involved on the Windows side,  
> unless
> they are trivial and obvious improvements.
>
>  * [1/3] seems without harm but on the other hand does not seem so
>    urgent either.

I did not find a simpler way to achieve a compile free warning on
mingw, without introducing more complex ifdefs.  I'm currently trying
to reduce the differences between git.git, mingw, and msysgit.


>  * [2/3] may introduce chicken-and-egg problem (use of get_git_dir()
>    inside git-init feels quite iffy, as it calls setup_git_env(),  
> which
>    does repository discovery), without an obvious and clear advantage.

I see.  I'll rethink 2/3 and 3/3.  Either I come up with more
convincing arguments or I'll try if the changes can be reverted
in the msysgit code base.


> For these reasons, both of them disqualify from being trivial and
> obvious improvements, so I did not pick them up unilaterally before
> seeing positive comments from other people.

	Steffen

^ permalink raw reply

* Re: [PATCH] Use is_absolute_path() in diff-lib.c, lockfile.c, setup.c, trace.c
From: Johannes Sixt @ 2007-11-26  7:45 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: git
In-Reply-To: <11960297431954-git-send-email-prohaska@zib.de>

Steffen Prohaska schrieb:
> Using the helper function to test for absolute paths makes porting easier.

The patch looks good.

> --- a/setup.c
> +++ b/setup.c
> @@ -59,7 +59,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
>  const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
>  {
>  	static char path[PATH_MAX];
> -	if (!pfx || !*pfx || arg[0] == '/')
> +	if (!pfx || !*pfx || is_absolute_path(arg))
>  		return arg;
>  	memcpy(path, pfx, pfx_len);
>  	strcpy(path + pfx_len, arg);

This instance, however, will be reworked for Windows anyway because we must 
do '\\' to '/' conversion even if the path is absolute (and, hence, we 
cannot just return the input).

But disregarding this note, the change makes the code more readable, IMO, so 
I'm all for it.

-- Hannes

^ permalink raw reply

* Re: git-svn fetch failure: index.lock file exists
From: Ollie Wild @ 2007-11-26  7:59 UTC (permalink / raw)
  To: Eric Wong; +Cc: git
In-Reply-To: <20071117211549.GC31598@mayonaise>

Thanks, Eric.  I managed to get things working before your response
which means that (a) I'm a bit fuzzy on what I did and (b) I didn't
record all the various messages that might be useful for debugging.

I ended up having to delete the entire .git/svn/libbid-last-merge
directory, as the fetch failed even with the lockfile removed.  After
that, git-svn fetch retrieved a lot of old changes which had already
been synched before but seems to have gotten itself back into a
consistent state.

As for whether or not I interrupted  a previous fetch, it's possible,
but I don't remember doing so.  I'd been doing mostly rebases against
the trunk, though, so any corruption could have occurred some time
back.

If I run into any more problems, I'll let you know and provide more
detailed feedback about the types of errors I'm seeing and the steps
required to correct it.  Otherwise, I'll chalk this up to a one-off
failure due to a poorly timed interrupt.

Ollie

On Nov 17, 2007 1:15 PM, Eric Wong <normalperson@yhbt.net> wrote:
>
> Ollie Wild <aaw@google.com> wrote:
> > Hi,
> >
> > I've been using git-svn for a while now to work on gcc.  Last night, I did a
> >
> >   git svn fetch
> >
> > and got the following failure:
> >
> >   Found possible branch point: svn+ssh://aaw@gcc.gnu.org/svn/gcc/trunk =>
> >       svn+ssh://aaw@gcc.gnu.org/svn/gcc/tags/libbid-last-merge, 128810
> >   Found branch parent: (tags/libbid-last-merge)
> >       789aa951bbc6a49f791bf5109136335fc33222c5
> >   fatal: unable to create
> > '.git/svn/tags/libbid-last-merge/index.lock': File exists
> >   read-tree 789aa951bbc6a49f791bf5109136335fc33222c5:
> >       command returned error: 128
> >
> > Naively, it looks to me like I've just got a stale lock file from a
> > previous run.  However, I have no idea what the correct recovery
> > strategy is.  Can I just delete the lock file?  Do I need to do some
> > sort of data validation?
> >
> > I'm using git version 1.5.2.5 with subversion version 1.4.5 (r25188).
>
> Did you interrupt git-svn in a previous run?  But you should be able to
> just remove it; git-svn should verify that the index is in a consistent
> state before it attempts to fetch again.
>
> --
> Eric Wong
>

^ permalink raw reply

* Re: [StGit PATCH 07/10] Teach the new infrastructure about the index and worktree
From: Karl Hasselström @ 2007-11-26  8:31 UTC (permalink / raw)
  To: Catalin Marinas; +Cc: git, David Kågedal
In-Reply-To: <20071125205140.7823.46991.stgit@yoghurt>

On 2007-11-25 21:51:40 +0100, Karl Hasselström wrote:

> And use the new powers to make "stg coalesce" able to handle
> arbitrary patches, not just consecutive applied patches.

Obviously, this should come with an updated help text for "stg
coalesce". Will amend the patch.

diff --git a/stgit/commands/coalesce.py b/stgit/commands/coalesce.py
index 2b121a9..6c0ae6d 100644
--- a/stgit/commands/coalesce.py
+++ b/stgit/commands/coalesce.py
@@ -27,8 +27,11 @@ help = 'coalesce two or more patches into one'
 usage = """%prog [options] <patches>
 
 Coalesce two or more patches, creating one big patch that contains all
-their changes. The patches must all be applied, and must be
-consecutive."""
+their changes.
+
+If there are conflicts when reordering the patches to match the order
+you specify, you will have to resolve them manually just as if you had
+done a sequence of pushes and pops yourself."""
 
 directory = common.DirectoryHasRepositoryLib()
 options = [make_option('-n', '--name', help = 'name of coalesced patch'),

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply related

* [PATCH] Fix typo in 1.5.4 release notes
From: Wincent Colaiuta @ 2007-11-26  8:34 UTC (permalink / raw)
  To: git; +Cc: gitster, Wincent Colaiuta

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 Documentation/RelNotes-1.5.4.txt |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Documentation/RelNotes-1.5.4.txt b/Documentation/RelNotes-1.5.4.txt
index ff4d3d8..c9c5376 100644
--- a/Documentation/RelNotes-1.5.4.txt
+++ b/Documentation/RelNotes-1.5.4.txt
@@ -26,7 +26,7 @@ Updates since v1.5.3
  * In git-rebase, when you decide not to replay a particular change
    after the command stopped with a conflict, you can say "git-rebase
    --skip" without first running "git reset --hard", as the command now
-   run it for you.
+   runs it for you.
 
  * git-remote knows --mirror mode.
 
-- 
1.5.3.6.950.g92b7b

^ permalink raw reply related

* Re: [RFC] best way to show diff of commit
From: Pierre Habouzit @ 2007-11-26  8:42 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Git ML, Kristian Høgsberg
In-Reply-To: <20071126002519.GA11133@efreet.light.src>

[-- Attachment #1: Type: text/plain, Size: 4079 bytes --]

On Mon, Nov 26, 2007 at 12:25:19AM +0000, Jan Hudec wrote:
> On Sun, Nov 25, 2007 at 22:18:31 +0100, Pierre Habouzit wrote:
> >   My question is: what do you think is the best way to do that, and
> > where ?
> 
> Are you talking about the ftplugins/git.vim script from vim-scripts package,
> right?

  Yes, its author.

> All it needs to do is call 'git diff --cached'. The below patch seems to work
> here (tried commit, commit -a, commit --amend, commit -a --amend and commit
> from subdirectory and they seemed to all work).

  Damn if that's so simple, I really feel like a big fool now.

> >   [0] the issue with this approach is that it's completely broken in
> >       amending mode (does not shows the proper thing), and the generated
> 
> I didn't find any issue with amending mode (showed incremental diff for me
> all right). The problem was when a file was being touched by the commit but
> work tree had unstaged changes. Such changes were shown when they shouldn't.

  Yep, that's the point.

> >       diffs aren't excellent, because as an editor plugin, it's hard to
> >       treat renames and copies easily, so I generate really really nasty
> >       diffs in that case too.
> 
> With --cached, detecting copies is just a matter of adding -C. In the patch
> below I added only one -C, but it should probably be user-configurable to use
> no, one or two (equivalent of --find-copies-harder).

  Sure, I'll do that obviously.

  Thanks a lot for the tip.
> 
> -- 
> 						 Jan 'Bulb' Hudec <bulb@ucw.cz>
> 
> ---8<---
> --- vim-scripts/ftplugin/git.vim.orig	2007-07-24 09:46:19.000000000 +0200
> +++ vim-scripts/ftplugin/git.vim	2007-11-26 01:01:59.000000000 +0100
> @@ -14,41 +14,6 @@
>  "{{{ function Git_diff_windows
>  
>  function! Git_diff_windows(vertsplit, auto)
> -    let i = 0
> -    let list_of_files = ''
> -
> -    " drop everything until '#  (will commit)' and the next empty line
> -    while i <= line('$')
> -        let line = getline(i)
> -        if line == '# Changes to be committed:'
> -            let i = i + 3
> -            break
> -        endif
> -
> -        let i = i + 1
> -    endwhile
> -
> -    " read file names until we have EOF or an empty line
> -    while i <= line('$')
> -        let line = getline(i)
> -        if line =~ '^#\s*[a-z ]*:.*->.*$'
> -            let file = substitute(line, '^#[^:]*:.*->\s*\(.*\)\s*$', '\1', '')
> -            let list_of_files = list_of_files . ' '.file
> -            let file = substitute(line, '^#[^:]*:\s*\(.*\)\s*->.*$', '\1', '')
> -            let list_of_files = list_of_files . ' '.file
> -        elseif line =~ '^#\s*[a-z ]*:'
> -            let file = substitute(line, '^#[^:]*:\s*\(.*\)\s*$', '\1', '')
> -            let list_of_files = list_of_files . ' '.file
> -        elseif line =~ '^#\s*$'
> -            break
> -        endif
> -
> -        let i = i + 1
> -    endwhile
> -
> -    if list_of_files == ""
> -        return
> -    endif
>  
>      if a:vertsplit
>          rightbelow vnew
> @@ -56,15 +21,8 @@
>          rightbelow new
>      endif
>      silent! setlocal ft=diff previewwindow bufhidden=delete nobackup noswf nobuflisted nowrap buftype=nofile
> -    let gitDir = system('git rev-parse --git-dir 2>/dev/null')
> -    let gitDir = substitute(gitDir, '.git\n', '', '')
> -    let wd = getcwd()
> -    if gitDir != ''
> -        exe 'cd '.gitDir
> -    endif
> -    exe 'normal :r!LANG=C git diff HEAD -- ' . list_of_files . "\n1Gdd"
> -    exe 'normal :r!LANG=C git diff --stat HEAD -- ' . list_of_files . "\no\<esc>1GddO\<esc>"
> -    exe 'cd '.wd
> +    exe "normal :r!LANG=C git diff --cached -C\n1Gdd"
> +    exe "normal :r!LANG=C git diff --stat --cached -C\no\<esc>1GddO\<esc>"
>      setlocal nomodifiable
>      noremap <buffer> q :bw<cr>
>      if a:auto

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [RFC] best way to show diff of commit
From: Pierre Habouzit @ 2007-11-26  8:47 UTC (permalink / raw)
  To: Jan Hudec; +Cc: Git ML, Kristian Høgsberg
In-Reply-To: <20071126002519.GA11133@efreet.light.src>

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

On Mon, Nov 26, 2007 at 12:25:19AM +0000, Jan Hudec wrote:
> Hm, looking at the builtin-commit.c that is currently cooking in next, it
> does not seem to be the case there. It probably should, so the editor can
> inspect what will be commited.

  I confirm that it's broken now, but that it works at the top of main.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [StGit PATCH 07/10] Teach the new infrastructure about the index and worktree
From: David Kågedal @ 2007-11-26  8:56 UTC (permalink / raw)
  To: Catalin Marinas, Karl Hasselström; +Cc: git
In-Reply-To: <20071125205140.7823.46991.stgit@yoghurt>

Karl Hasselström <kha@treskal.com> writes:

> --- a/stgit/lib/git.py
> +++ b/stgit/lib/git.py
> @@ -95,6 +95,8 @@ class Commitdata(Repr):
>          return type(self)(committer = committer, defaults = self)
>      def set_message(self, message):
>          return type(self)(message = message, defaults = self)
> +    def is_empty(self):
> +        return self.tree == self.parent.data.tree
>      def __str__(self):
>          if self.tree == None:
>              tree = None

But a Commitdata describes a Git commit, right? And not an StGit
patch. So what does it mean to say that a commit is empty? I could
just as well mean that the tree is the null tree.

I think I would have called this "is_nochange" or something.

-- 
David Kågedal

^ permalink raw reply

* [PATCH] Make builtin-commit.c export GIT_INDEX_FILE for launch_editor as well.
From: Pierre Habouzit @ 2007-11-26  8:59 UTC (permalink / raw)
  To: Jan Hudec, Git ML, Kristian Høgsberg, Junio C Hamano
In-Reply-To: <20071126084731.GB23373@artemis.corp>

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---

  And here's the fix for this tiny problem …



 builtin-commit.c |    9 ++++++---
 builtin-tag.c    |    6 +++---
 strbuf.h         |    2 +-
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/builtin-commit.c b/builtin-commit.c
index 5d27102..6e6b9f2 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -748,9 +748,12 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	/* Get the commit message and validate it */
 	header_len = sb.len;
-	if (!no_edit)
-		launch_editor(git_path(commit_editmsg), &sb);
-	else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
+	if (!no_edit) {
+		char index[PATH_MAX];
+		const char *env[2] = { index, NULL };
+		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
+		launch_editor(git_path(commit_editmsg), &sb, env);
+	} else if (strbuf_read_file(&sb, git_path(commit_editmsg), 0) < 0) {
 		rollback_index_files();
 		die("could not read commit message\n");
 	}
diff --git a/builtin-tag.c b/builtin-tag.c
index e89b201..8cc7f9c 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -23,7 +23,7 @@ static const char * const git_tag_usage[] = {
 
 static char signingkey[1000];
 
-void launch_editor(const char *path, struct strbuf *buffer)
+void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
 {
 	const char *editor, *terminal;
 
@@ -49,7 +49,7 @@ void launch_editor(const char *path, struct strbuf *buffer)
 	if (strcmp(editor, ":")) {
 		const char *args[] = { editor, path, NULL };
 
-		if (run_command_v_opt(args, 0))
+		if (run_command_v_opt_cd_env(args, 0, NULL, env))
 			die("There was a problem with the editor %s.", editor);
 	}
 
@@ -318,7 +318,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 			write_or_die(fd, tag_template, strlen(tag_template));
 		close(fd);
 
-		launch_editor(path, buf);
+		launch_editor(path, buf, NULL);
 
 		unlink(path);
 		free(path);
diff --git a/strbuf.h b/strbuf.h
index 8334a9b..36d61db 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -117,6 +117,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 
 extern void stripspace(struct strbuf *buf, int skip_comments);
-extern void launch_editor(const char *path, struct strbuf *buffer);
+extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
 
 #endif /* STRBUF_H */
-- 
1.5.3.6.2015.g9baba-dirty


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Rebase/cherry-picking idea
From: Wincent Colaiuta @ 2007-11-26  9:02 UTC (permalink / raw)
  To: Git Mailing List

In using "git-rebase --interactive" to re-order commits you  
occasionally get conflicts and will see a message like this:

	When commiting, use the option '-c %s' to retain authorship and message

I was thinking that it might be nice to stash away this commit id  
somewhere in GIT_DIR so that the user didn't have to explicitly  
remember it, and add a new switch to git-commit that could be used to  
automatically use that stashed commit id, something like:

	git commit --retain

Although I most often see this kind of message in interactive  
rebasing, the message is generated in builtin-revert.c when cherry- 
picking, so you can also see it in any other situation where you're  
cherry picking and there's a conflict.

What do people think? Would this be a nice usability improvement? Or  
is it adding clutter?

Cheers,
Wincent

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Jakub Narebski @ 2007-11-26  9:03 UTC (permalink / raw)
  To: git
In-Reply-To: <alpine.LFD.0.99999.0711252324360.9605@xanadu.home>

Nicolas Pitre wrote:

> On Mon, 26 Nov 2007, J. Bruce Fields wrote:
> 
>> I do find that trying to work on top of a constantly rebased branch is
>> annoying no matter how I do it.  So I sometimes wonder if we shouldn't
>> instead be finding ways to avoid the practice.
> 
> I don't think it can't be avoided in many cases.  Some stuff gets 
> rebased because it has to be refined before it is merged in a more 
> stable and more "official" repository.  Working on top of a rebased 
> branch could be much easier if there was a dedicated command to perform 
> the local rebase of one's work after a fetch, just like the pull command 
> does a merge after a fetch, at which point both work flows would be 
> almost equivalent wrt ease of use.

There was idea of 'rebase' merge strategy (which was in some form
implemented once under another name: check archives if you want).
And there is an idea of --rebase switch git git-pull.

What is left is the implementation ;-)

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: What's cooking in git.git (topics)
From: Andreas Ericsson @ 2007-11-26  9:09 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git
In-Reply-To: <fie23u$5tc$1@ger.gmane.org>

Jakub Narebski wrote:
> Nicolas Pitre wrote:
> 
>> On Mon, 26 Nov 2007, J. Bruce Fields wrote:
>>
>>> I do find that trying to work on top of a constantly rebased branch is
>>> annoying no matter how I do it.  So I sometimes wonder if we shouldn't
>>> instead be finding ways to avoid the practice.
>> I don't think it can't be avoided in many cases.  Some stuff gets 
>> rebased because it has to be refined before it is merged in a more 
>> stable and more "official" repository.  Working on top of a rebased 
>> branch could be much easier if there was a dedicated command to perform 
>> the local rebase of one's work after a fetch, just like the pull command 
>> does a merge after a fetch, at which point both work flows would be 
>> almost equivalent wrt ease of use.
> 
> There was idea of 'rebase' merge strategy (which was in some form
> implemented once under another name: check archives if you want).
> And there is an idea of --rebase switch git git-pull.
> 
> What is left is the implementation ;-)
> 

"git pull --rebase" already has an implementation. Dscho cooked one up
which I've been using since then. It works nicely.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: Rebase/cherry-picking idea
From: Benoit Sigoure @ 2007-11-26  9:32 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Git Mailing List
In-Reply-To: <109026BC-408F-451A-8F7C-A4012DD8DBDF@wincent.com>

[-- Attachment #1: Type: text/plain, Size: 1175 bytes --]

On Nov 26, 2007, at 10:02 AM, Wincent Colaiuta wrote:

> In using "git-rebase --interactive" to re-order commits you  
> occasionally get conflicts and will see a message like this:
>
> 	When commiting, use the option '-c %s' to retain authorship and  
> message
>
> I was thinking that it might be nice to stash away this commit id  
> somewhere in GIT_DIR so that the user didn't have to explicitly  
> remember it, and add a new switch to git-commit that could be used  
> to automatically use that stashed commit id, something like:
>
> 	git commit --retain
>
> Although I most often see this kind of message in interactive  
> rebasing, the message is generated in builtin-revert.c when cherry- 
> picking, so you can also see it in any other situation where you're  
> cherry picking and there's a conflict.
>
> What do people think? Would this be a nice usability improvement?  
> Or is it adding clutter?


I'm not sure but I think this message is just some unwanted  
(misleading) noise, since when you rebase, once you solve the  
conflicts, you git-rebase --continue, you don't git-commit.

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Re: [PATCH] Make builtin-commit.c export GIT_INDEX_FILE for launch_editor as well.
From: Pierre Habouzit @ 2007-11-26  9:38 UTC (permalink / raw)
  To: Git ML
In-Reply-To: <20071126085927.GC23373@artemis.corp>

[-- Attachment #1: Type: text/plain, Size: 268 bytes --]

  and here's the updated git.vim:
http://www.vim.org/scripts/script.php?script_id=1934

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [StGit PATCH 07/10] Teach the new infrastructure about the index and worktree
From: Karl Hasselström @ 2007-11-26 10:44 UTC (permalink / raw)
  To: David Kågedal; +Cc: Catalin Marinas, git
In-Reply-To: <874pf9bd11.fsf@lysator.liu.se>

On 2007-11-26 09:56:42 +0100, David Kågedal wrote:

> Karl Hasselström <kha@treskal.com> writes:
>
> > --- a/stgit/lib/git.py
> > +++ b/stgit/lib/git.py
> > @@ -95,6 +95,8 @@ class Commitdata(Repr):
> >          return type(self)(committer = committer, defaults = self)
> >      def set_message(self, message):
> >          return type(self)(message = message, defaults = self)
> > +    def is_empty(self):
> > +        return self.tree == self.parent.data.tree
> >      def __str__(self):
> >          if self.tree == None:
> >              tree = None
>
> But a Commitdata describes a Git commit, right? And not an StGit
> patch.

Yes.

> So what does it mean to say that a commit is empty? I could just as
> well mean that the tree is the null tree. I think I would have
> called this "is_nochange" or something.

Good point.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle

^ permalink raw reply

* Re: Rebase/cherry-picking idea
From: Wincent Colaiuta @ 2007-11-26 11:27 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Git Mailing List
In-Reply-To: <C3971B37-F75A-40EE-B30A-E88E5DAFAD55@lrde.epita.fr>

El 26/11/2007, a las 10:32, Benoit Sigoure escribió:

> On Nov 26, 2007, at 10:02 AM, Wincent Colaiuta wrote:
>
>> In using "git-rebase --interactive" to re-order commits you  
>> occasionally get conflicts and will see a message like this:
>>
>> 	When commiting, use the option '-c %s' to retain authorship and  
>> message
>>
>> I was thinking that it might be nice to stash away this commit id  
>> somewhere in GIT_DIR so that the user didn't have to explicitly  
>> remember it, and add a new switch to git-commit that could be used  
>> to automatically use that stashed commit id, something like:
>>
>> 	git commit --retain
>>
>> Although I most often see this kind of message in interactive  
>> rebasing, the message is generated in builtin-revert.c when cherry- 
>> picking, so you can also see it in any other situation where you're  
>> cherry picking and there's a conflict.
>>
>> What do people think? Would this be a nice usability improvement?  
>> Or is it adding clutter?
>
>
> I'm not sure but I think this message is just some unwanted  
> (misleading) noise, since when you rebase, once you solve the  
> conflicts, you git-rebase --continue, you don't git-commit.

Looks like you're right. I just did a simple test and it turns out  
that after a conflict, this:

	git commit -c ...
	git rebase --continue

Produces exactly the same history as this:

	git rebase --continue

So I think that misleading noise needs to be suppressed or reworded  
when rebasing. Will look into it.

Cheers,
Wincent

^ permalink raw reply

* Re: Rebase/cherry-picking idea
From: Wincent Colaiuta @ 2007-11-26 12:34 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Benoit Sigoure, Git Mailing List
In-Reply-To: <D21294CA-6FD0-40F5-B0D6-5155865DA69A@wincent.com>

El 26/11/2007, a las 12:27, Wincent Colaiuta escribió:

> El 26/11/2007, a las 10:32, Benoit Sigoure escribió:
>
>> On Nov 26, 2007, at 10:02 AM, Wincent Colaiuta wrote:
>>
>>> In using "git-rebase --interactive" to re-order commits you  
>>> occasionally get conflicts and will see a message like this:
>>>
>>> 	When commiting, use the option '-c %s' to retain authorship and  
>>> message
>>>
>>> I was thinking that it might be nice to stash away this commit id  
>>> somewhere in GIT_DIR so that the user didn't have to explicitly  
>>> remember it, and add a new switch to git-commit that could be used  
>>> to automatically use that stashed commit id, something like:
>>>
>>> 	git commit --retain
>>>
>>> Although I most often see this kind of message in interactive  
>>> rebasing, the message is generated in builtin-revert.c when cherry- 
>>> picking, so you can also see it in any other situation where  
>>> you're cherry picking and there's a conflict.
>>>
>>> What do people think? Would this be a nice usability improvement?  
>>> Or is it adding clutter?
>>
>>
>> I'm not sure but I think this message is just some unwanted  
>> (misleading) noise, since when you rebase, once you solve the  
>> conflicts, you git-rebase --continue, you don't git-commit.
>
> Looks like you're right. I just did a simple test and it turns out  
> that after a conflict, this:
>
> 	git commit -c ...
> 	git rebase --continue
>
> Produces exactly the same history as this:
>
> 	git rebase --continue
>
> So I think that misleading noise needs to be suppressed or reworded  
> when rebasing. Will look into it.

How about something like this? It would obviously be nice if we could  
avoid adding another option to builtin-revert; perhaps when/if git- 
rebase becomes a builtin we can avoid that. The other alternative, and  
probably one I like I bit more, would be to auto-detect that a rebase  
is in progress by looking inside the GIT_DIR, although that would also  
alter the behaviour of manual invocations of git-revert and git-cherry- 
pick during an interactive rebase (do people actually do that?). What  
do you think?

diff --git a/builtin-revert.c b/builtin-revert.c
index a0586f9..36e36c3 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -30,7 +30,7 @@ static const char * const cherry_pick_usage[] = {
  	NULL
  };

-static int edit, no_replay, no_commit, mainline;
+static int edit, no_replay, no_commit, rebasing, mainline;
  static enum { REVERT, CHERRY_PICK } action;
  static struct commit *commit;

@@ -50,6 +50,7 @@ static void parse_args(int argc, const char **argv)
  		OPT_BOOLEAN('e', "edit", &edit, "edit the commit message"),
  		OPT_BOOLEAN('x', NULL, &no_replay, "append commit name when cherry- 
picking"),
  		OPT_BOOLEAN('r', NULL, &noop, "no-op (backward compatibility)"),
+		OPT_BOOLEAN(0, "rebasing", &rebasing, "use rebase mode"),
  		OPT_INTEGER('m', "mainline", &mainline, "parent number"),
  		OPT_END(),
  	};
@@ -352,11 +353,16 @@ static int revert_or_cherry_pick(int argc, const  
char **argv)
  		}
  		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
  			die ("Error wrapping up %s", defmsg);
+		if (rebasing)
+			message = "run 'git rebase --continue' "
+			    "or 'git rebase --abort'";
+		else
+			message = "commit the result";
  		fprintf(stderr, "Automatic %s failed.  "
  			"After resolving the conflicts,\n"
  			"mark the corrected paths with 'git add <paths>' "
-			"and commit the result.\n", me);
-		if (action == CHERRY_PICK) {
+			"and %s.\n", me, message);
+		if (action == CHERRY_PICK && !rebasing) {
  			fprintf(stderr, "When commiting, use the option "
  				"'-c %s' to retain authorship and message.\n",
  				find_unique_abbrev(commit->object.sha1,
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bf44b6a..5afb843 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -117,7 +117,7 @@ pick_one () {
  		sha1=$(git rev-parse --short $sha1)
  		output warn Fast forward to $sha1
  	else
-		output git cherry-pick "$@"
+		output git cherry-pick --rebasing "$@"
  	fi
  }

@@ -187,7 +187,7 @@ pick_one_preserving_merges () {
  			fi
  			;;
  		*)
-			output git cherry-pick "$@" ||
+			output git cherry-pick --rebasing "$@" ||
  				die_with_patch $sha1 "Could not pick $sha1"
  			;;
  		esac

^ permalink raw reply related

* Re: Rebase/cherry-picking idea
From: Benoit Sigoure @ 2007-11-26 12:39 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Git Mailing List
In-Reply-To: <CDF48716-F198-4B33-A5F5-8A2DE1F177EB@wincent.com>

On Nov 26, 2007, at 1:34 PM, Wincent Colaiuta wrote:

> How about something like this? It would obviously be nice if we  
> could avoid adding another option to builtin-revert; perhaps when/ 
> if git-rebase becomes a builtin we can avoid that. The other  
> alternative, and probably one I like I bit more, would be to auto- 
> detect that a rebase is in progress by looking inside the GIT_DIR,  
> although that would also alter the behaviour of manual invocations  
> of git-revert and git-cherry-pick during an interactive rebase (do  
> people actually do that?). What do you think?


Hmm yeah, I agree that it's a little bit of a dirty workaround but,  
as you pointed out, until rebase is builtinified, this looks like the  
best/easiest alternative.

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory

^ permalink raw reply

* Re: [PATCH] Add 'git fast-export', the sister of 'git fast-import'
From: Johannes Schindelin @ 2007-11-26 12:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbq9hiz6a.fsf@gitster.siamese.dyndns.org>

Hi,

On Sun, 25 Nov 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > diff --git a/Documentation/git-fast-export.txt b/Documentation/git-fast-export.txt
> > new file mode 100644
> > index 0000000..073ff7f
> > --- /dev/null
> > +++ b/Documentation/git-fast-export.txt
> > ...
> > +DESCRIPTION
> > +-----------
> > +This program dumps the given revisions in a form suitable to be piped
> > +into gitlink:git-fast-import[1].
> > +
> > +You can use it as a human readable bundle replacement (see
> > +gitlink:git-bundle[1]), or as a kind of an interactive
> > +gitlink:git-filter-branch[1].
> > +
> > +
> > +OPTIONS
> > +-------
> > +--progress=<n>::
> > +	Insert 'progress' statements every <n> objects, to be shown by
> > +	gitlink:git-fast-import[1] during import.
> > +
> > +--signed-tags=(ignore|warn|strip|abort)::
> > +	Specify how to handle signed tags.  Since any transformation
> > +	after the export can change the tag names (which can also happen
> > +	when excluding revisions) the signatures will not match.
> > ++
> > +When asking to 'abort' (which is the default), this program will die
> > +when encountering a signed tag.  With 'strip', the tags will be made
> > +unsigned, with 'ignore', they will be silently ignored (i.e. not exported)
> > +and with 'warn', they will be exported, but you will see a warning.
> 
> I am not sure if abort should be the default.

I tried to be conservative.

> If a straight dump-restore is made without rewriting, the result will be 
> identical to the original, right?

Yep.

> The reason I mention a straight dump-restore is because ...
> 
> > +$ git fast-export master~5..master |
> > +	sed "s|refs/heads/master|refs/heads/other|" |
> > +	git fast-import
> 
> ... I find this a quite unrealistic example to assume that the data
> stream does not have some string and convert blindly without parsing.

That's what the warning after the example is about.  For quick and dirty 
operations, it is quite adequate.

Besides, I have the feeling that some people are more comfortable dumping 
the whole repository into a file, editing it, and fast-importing it.  That 
is what I referred to when I said "think of it as kind of an interactive 
filter-branch".

> On the other hand, we _could_ also have a separate filter that works on 
> input stream for fast-import, but that filter should know what the 
> fast-import input stream looks like (a simple sed does not cut it).

I agree that for most serious operations sed is not good enough.

> So unless the future direction is to deprecate filter-branch and replace 
> it with such a fast-import based filter in between fast-export and 
> fast-import, I think the use of fast-export is to make verbatim copy 
> without munging the contents, which leads me to think --signed-tag 
> option should default to "export it as-is".
> 
> ... which seem to be missing from the available values to the option.

You mean something like "--signed-tag=warn 2> /dev/null"?  How about a 
"--signed-tag=quiet" mode?

> > diff --git a/builtin-fast-export.c b/builtin-fast-export.c
> > new file mode 100755
> > index 0000000..48d0c54
> > --- /dev/null
> > +++ b/builtin-fast-export.c
> > ...
> > +/*
> > + * TODO:
> > + * - tags (--signed-tags=(ignore|warn|strip|abort)
> > + */
> 
> This comment is stale?

Correct.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] gitweb: the commitdiff is very commonly used, it's needed on search page, too
From: Denis Cheng @ 2007-11-26 12:42 UTC (permalink / raw)
  To: git; +Cc: dengxw

---
 gitweb/gitweb.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 491a3f4..ff5daa7 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3786,6 +3786,8 @@ sub git_search_grep_body {
 		      "<td class=\"link\">" .
 		      $cgi->a({-href => href(action=>"commit", hash=>$co{'id'})}, "commit") .
 		      " | " .
+		      $cgi->a({-href => href(action=>"commitdiff", hash=>$co{'id'})}, "commitdiff") .
+		      " | " .
 		      $cgi->a({-href => href(action=>"tree", hash=>$co{'tree'}, hash_base=>$co{'id'})}, "tree");
 		print "</td>\n" .
 		      "</tr>\n";
-- 
1.5.3.4

^ permalink raw reply related

* Re: Rebase/cherry-picking idea
From: Johannes Sixt @ 2007-11-26 12:51 UTC (permalink / raw)
  To: Wincent Colaiuta; +Cc: Benoit Sigoure, Git Mailing List
In-Reply-To: <CDF48716-F198-4B33-A5F5-8A2DE1F177EB@wincent.com>

Wincent Colaiuta schrieb:
> El 26/11/2007, a las 12:27, Wincent Colaiuta escribió:
>> So I think that misleading noise needs to be suppressed or reworded 
>> when rebasing. Will look into it.
> 
> How about something like this? It would obviously be nice if we could 
> avoid adding another option to builtin-revert; perhaps when/if 
> git-rebase becomes a builtin we can avoid that. The other alternative, 
> and probably one I like I bit more, would be to auto-detect that a 
> rebase is in progress by looking inside the GIT_DIR, although that would 
> also alter the behaviour of manual invocations of git-revert and 
> git-cherry-pick during an interactive rebase (do people actually do 
> that?). What do you think?

Introduce an environment variable _GIT_CHERRY_PICK_HELP (note the leading 
underscore), which git-rebase sets; if it's set, git-cherry-pick uses that 
text instead of the usual one.

-- Hannes

^ permalink raw reply

* Re: Reordering lines in "git-rebase -i" task sheet
From: Andy Parkins @ 2007-11-26 13:14 UTC (permalink / raw)
  To: git
In-Reply-To: <fi64ge$u1g$1@ger.gmane.org>

Andy Parkins wrote:

> I'm not convinced that this is a great idea.

I've just tried it and (for me) I don't like it.

 - It's now harder to find the commits I want to edit, the cursor still
   doesn't start in the right place, but now I can't get to the right place
   quickly (the right place being the bottom of the list) because the
   interesting bits are neither at the top nor at the bottom
 - vim no longer automatically loads a reasonable syntax highlighting mode
   making the comment lines appear the same as the pick lines.
 - On a long rebase list the important message explaining what I'm looking
   at isn't on screen.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIET
andyparkins@gmail.com

^ permalink raw reply

* Re: Rebase/cherry-picking idea
From: Wincent Colaiuta @ 2007-11-26 13:15 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Benoit Sigoure, Git Mailing List
In-Reply-To: <474AC136.8060906@viscovery.net>

El 26/11/2007, a las 13:51, Johannes Sixt escribió:

> Wincent Colaiuta schrieb:
>> El 26/11/2007, a las 12:27, Wincent Colaiuta escribió:
>>> So I think that misleading noise needs to be suppressed or  
>>> reworded when rebasing. Will look into it.
>> How about something like this? It would obviously be nice if we  
>> could avoid adding another option to builtin-revert; perhaps when/ 
>> if git-rebase becomes a builtin we can avoid that. The other  
>> alternative, and probably one I like I bit more, would be to auto- 
>> detect that a rebase is in progress by looking inside the GIT_DIR,  
>> although that would also alter the behaviour of manual invocations  
>> of git-revert and git-cherry-pick during an interactive rebase (do  
>> people actually do that?). What do you think?
>
> Introduce an environment variable _GIT_CHERRY_PICK_HELP (note the  
> leading underscore), which git-rebase sets; if it's set, git-cherry- 
> pick uses that text instead of the usual one.

Good idea, quite a bit less cruddy:

diff --git a/builtin-revert.c b/builtin-revert.c
index a0586f9..5a57574 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -229,7 +229,7 @@ static int revert_or_cherry_pick(int argc, const  
char **argv)
  	unsigned char head[20];
  	struct commit *base, *next, *parent;
  	int i;
-	char *oneline, *reencoded_message = NULL;
+	char *oneline, *reencoded_message = NULL, *help_message;
  	const char *message, *encoding;
  	const char *defmsg = xstrdup(git_path("MERGE_MSG"));

@@ -352,11 +352,13 @@ static int revert_or_cherry_pick(int argc, const  
char **argv)
  		}
  		if (close(msg_fd) || commit_lock_file(&msg_file) < 0)
  			die ("Error wrapping up %s", defmsg);
+		help_message = getenv("_GIT_CHERRY_PICK_HELP");
  		fprintf(stderr, "Automatic %s failed.  "
  			"After resolving the conflicts,\n"
  			"mark the corrected paths with 'git add <paths>' "
-			"and commit the result.\n", me);
-		if (action == CHERRY_PICK) {
+			"and %s.\n", me,
+			help_message ? help_message : "commit the result");
+		if (action == CHERRY_PICK && !help_message) {
  			fprintf(stderr, "When commiting, use the option "
  				"'-c %s' to retain authorship and message.\n",
  				find_unique_abbrev(commit->object.sha1,
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index bf44b6a..e5f9810 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -117,6 +117,7 @@ pick_one () {
  		sha1=$(git rev-parse --short $sha1)
  		output warn Fast forward to $sha1
  	else
+		export _GIT_CHERRY_PICK_HELP="run 'git rebase --continue'"
  		output git cherry-pick "$@"
  	fi
  }
@@ -187,6 +188,7 @@ pick_one_preserving_merges () {
  			fi
  			;;
  		*)
+			export _GIT_CHERRY_PICK_HELP="run 'git rebase --continue'"
  			output git cherry-pick "$@" ||
  				die_with_patch $sha1 "Could not pick $sha1"
  			;;

^ permalink raw reply related

* Re: Rebase/cherry-picking idea
From: Johannes Schindelin @ 2007-11-26 13:26 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Wincent Colaiuta, Git Mailing List
In-Reply-To: <C3971B37-F75A-40EE-B30A-E88E5DAFAD55@lrde.epita.fr>

Hi,

On Mon, 26 Nov 2007, Benoit Sigoure wrote:

> On Nov 26, 2007, at 10:02 AM, Wincent Colaiuta wrote:
> 
> > In using "git-rebase --interactive" to re-order commits you occasionally get
> > conflicts and will see a message like this:
> > 
> > 	When commiting, use the option '-c %s' to retain authorship and
> > message
> > 
> > I was thinking that it might be nice to stash away this commit id somewhere
> > in GIT_DIR so that the user didn't have to explicitly remember it, and add a
> > new switch to git-commit that could be used to automatically use that
> > stashed commit id, something like:
> > 
> > 	git commit --retain
> > 
> > Although I most often see this kind of message in interactive rebasing, the
> > message is generated in builtin-revert.c when cherry-picking, so you can
> > also see it in any other situation where you're cherry picking and there's a
> > conflict.
> > 
> > What do people think? Would this be a nice usability improvement? Or is it
> > adding clutter?
> 
> 
> I'm not sure but I think this message is just some unwanted (misleading)
> noise, since when you rebase, once you solve the conflicts, you git-rebase
> --continue, you don't git-commit.

Yep.  It is on my TODO list since a long time, but I am just as glad 
somebody else is doing it.  But I have to agree with Hannes that using an 
environment variable is cleaner, more elegant and shorter.

Ciao,
Dscho

^ 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