Git development
 help / color / mirror / Atom feed
* Re: git push bug?
From: Steffen Prohaska @ 2007-10-20  8:38 UTC (permalink / raw)
  To: Shawn O. Pearce, Lars Hjemli, Johannes Schindelin
  Cc: Joakim Tjernlund, Git Mailing List
In-Reply-To: <5513C211-DE33-411C-8EE6-2259B41DC3EA@zib.de>

Shawn,
sp/push-refspec definitely needs more work (see below).


On Oct 20, 2007, at 10:29 AM, Steffen Prohaska wrote:

>
> On Oct 18, 2007, at 11:58 PM, Johannes Schindelin wrote:
>
>>
>> On Thu, 18 Oct 2007, Steffen Prohaska wrote:
>>>
>>> But you need a full refspec starting with 'refs/heads/' if you  
>>> want to
>>> create a new branch on the remote side.
>>
>> No.  Not if the name is the same on the local side.
>
> You're right. The documentation of git-send-pack says what you're
> saying:
>
> [...]
>
> Until you corrected me, I believed that new branches will never
> be created on the remote side unless a full ref is used. That is
> I expected that only
>
>    git push origin refs/heads/work/topic
>
> would work.
>
> I thought this would be another safety net -- kind of a reminder
> not to push the wrong branch by accident.
>
> I still like the idea, but apparently git didn't ever support what
> I thought it would.

And I even fixed the behavior to match my expectation in a patch
which made it to spearce/pu:


d869233c62688742968663c4e8b5ff20a50a5011

     push, send-pack: fix test if remote branch exists for colon-less  
refspec

     A push must fail if the remote ref does not yet exist and the  
refspec
     does not start with refs/. Remote refs must explicitly be  
created with
     their full name.

     This commit adds some tests and fixes the existence check in  
send-pack.


sp/push-refspec definitely needs some more work.

	Steffen

^ permalink raw reply

* Re: git push bug?
From: Steffen Prohaska @ 2007-10-20  8:29 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Joakim Tjernlund, git
In-Reply-To: <Pine.LNX.4.64.0710182258000.25221@racer.site>


On Oct 18, 2007, at 11:58 PM, Johannes Schindelin wrote:

>
> On Thu, 18 Oct 2007, Steffen Prohaska wrote:
>
>> On Oct 18, 2007, at 6:21 PM, Johannes Schindelin wrote:
>>
>>> On Thu, 18 Oct 2007, Joakim Tjernlund wrote:
>>>
>>>> Seems like it is a bit too easy to make mistakes here. Why can I  
>>>> delete
>>>> a branch with :linus but not create one with linus:linus?
>>>
>>> I wonder why you bother with the colon at all.  Just
>>>
>>> 	git push <remote> linus
>>>
>>> and be done with it.  The colon is only there to play interesting  
>>> games,
>>> not something as simple as "push this branch" or "push this tag".
>>
>> But you need a full refspec starting with 'refs/heads/' if you  
>> want to
>> create a new branch on the remote side.
>
> No.  Not if the name is the same on the local side.

You're right. The documentation of git-send-pack says what you're
saying:

'''
When one or more <ref> are specified explicitly, it can be either a  
single pattern, or a pair of such pattern separated by a colon  
":" (this means that a ref name cannot have a colon in it). A single  
pattern <name> is just a shorthand for <name>:<name>
'''

Here it says that <name> is a shorthand for <name>:<name>.
An later it states

'''
If <dst> does not match any remote ref, either
    * it has to start with "refs/"; <dst> is used as the destination  
literally in this case.
    * <src> == <dst> and the ref that matched the <src> must not  
exist in the set of remote refs; the ref matched <src> locally is  
used as the name of the destination.
'''

If <src> == <dst> then <dst> will be created even if it didn't exist.

I think the current implementation though is a bit different.
It will created a new branch for a colon-less refspec, that is

    git push origin work/topic

will create a new ref on the remote. But

    git push origin work/topic:work/topic

will _not_.


Until you corrected me, I believed that new branches will never
be created on the remote side unless a full ref is used. That is
I expected that only

    git push origin refs/heads/work/topic

would work.

I thought this would be another safety net -- kind of a reminder
not to push the wrong branch by accident.

I still like the idea, but apparently git didn't ever support what
I thought it would.

Maybe adding some command line flags making the different tasks
explicit could help:

     git push --create origin work/new-topic
     git push --delete origin work/old-topic
     git push --non-standard origin refs/funny/ref

We already have similar flags

     --all: all branches
     --tags: all tags
     --force: force non-fast-forward.

I haven't fully thought this through. Maybe I'll come up with a patch
later.

	Steffen

^ permalink raw reply

* [PATCH] On error, do not list all commands, but point to --help option.
From: Jari Aalto @ 2007-10-20  8:24 UTC (permalink / raw)
  To: git

- commented out call to list_common_cmds_help()
- Send error message to stderr, not stdout.

Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
 help.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/help.c b/help.c
index 1cd33ec..dc9e59f 100644
--- a/help.c
+++ b/help.c
@@ -185,8 +185,8 @@ static void show_man_page(const char *git_cmd)
 
 void help_unknown_cmd(const char *cmd)
 {
-	printf("git: '%s' is not a git-command\n\n", cmd);
-	list_common_cmds_help();
+	fprintf(stderr, "git: '%s' is not a git-command. See --help\n\n", cmd);
+        /* list_common_cmds_help(); */
 	exit(1);
 }
 
-- 
1.5.3.2.81.g17ed

Welcome to FOSS revolution: we fix and modify until it shines

^ permalink raw reply related

* Re: [PATCH] Deduce exec_path also from calls to git with a  relative path
From: Scott R Parish @ 2007-10-20  8:13 UTC (permalink / raw)
  To: Johannes Schindelin, git, spearce, gitster


Wow, that sure cleaned up nicely! :)

Thanks
sRp


----- Original Message -----
Subject: [PATCH] Deduce exec_path also from calls to git with a relative path
Date: Sat, October 20, 2007 0:21
From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>

> There is already logic in the git wrapper to deduce the exec_path from
> argv[0], when the git wrapper was called with an absolute path.  Extend
> that logic to handle relative paths as well.
>
> For example, when you call "../../hello/world/git", it will not turn
> "../../hello/world" into an absolute path, and use that.
>
> Initial implementation by Scott R Parish.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>
>         On Fri, 19 Oct 2007, Scott R Parish wrote:
>
>         >  Signed-off-by: Scott R Parish <srp@srparish.net>
>
>         That is a little short for a commit message ;-)
>
>         >  git.c |   35 +++++++++++++++++++++++++++++++++--
>         >  1 files changed, 33 insertions(+), 2 deletions(-)
>
>         I had commented on this before.  Probably I did a very bad job
>         at explaining things, so hopefully this is better:
>
>  git.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/git.c b/git.c
> index d7c6bca..1dad764 100644
> --- a/git.c
> +++ b/git.c
> @@ -414,13 +414,14 @@ int main(int argc, const char **argv)
>          /*
>           * Take the basename of argv[0] as the command
>           * name, and the dirname as the default exec_path
> -         * if it's an absolute path and we don't have
> -         * anything better.
> +         * if we don't have anything better.
>           */
>          if (slash) {
>                  *slash++ = 0;
>                  if (*cmd == '/')
>                          exec_path = cmd;
> +                else
> +                        exec_path = xstrdup(make_absolute_path(cmd));
>                  cmd = slash;
>          }
>
> --
> 1.5.3.4.1287.g8b31e
>
> -
> 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] When exec'ing sub-commands, fall back on execvp (thePATH)
From: Scott R Parish @ 2007-10-20  8:12 UTC (permalink / raw)
  To: Johannes Schindelin, git

Yeah, that seems to work fine. The theoretical drawback to this approach
is that it could possibly effect the order in which the paths are tried.
For instance, if a user did "export GIT_EXEC_PATH=", then the
builtin_exec_path wouldn't be tried before the PATH. (i doubt that it
would be a problem, but thought i should note it)

sRp


----- Original Message -----
Subject: Re: [PATCH] When exec'ing sub-commands, fall back on execvp
(thePATH)
Date: Sat, October 20, 2007 0:30
From: "Johannes Schindelin" <Johannes.Schindelin@gmx.de>

> Hi,
>
> On Fri, 19 Oct 2007, Scott Parish wrote:
>
> > diff --git a/exec_cmd.c b/exec_cmd.c
> > index 9b74ed2..674c9f3 100644
> > --- a/exec_cmd.c
> > +++ b/exec_cmd.c
> > @@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)
> >  {
> >          char git_command[PATH_MAX + 1];
> >          int i;
> > +        int rc;
> >          const char *paths[] = { current_exec_path,
> >                                  getenv(EXEC_PATH_ENVIRONMENT),
> >                                  builtin_exec_path };
> > +        const char *tmp;
> > +        size_t len;
> >
> >          for (i = 0; i < ARRAY_SIZE(paths); ++i) {
> > -                size_t len;
> > -                int rc;
> >                  const char *exec_dir = paths[i];
> > -                const char *tmp;
> >
> >                  if (!exec_dir || !*exec_dir) continue;
> >
> > @@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)
> >
> >                  argv[0] = tmp;
> >          }
> > -        return -1;
> >
> > +        rc = snprintf(git_command, sizeof(git_command), "git-%s",
argv[0]);
> > +        if (rc < 0 || rc >= sizeof(git_command) - len) {
> > +                fprintf(stderr, "git: command name given is too
long.\n");
> > +                return -1;
> > +        }
> > +
> > +        tmp = argv[0];
> > +        argv[0] = git_command;
> > +
> > +        trace_argv_printf(argv, -1, "trace: exec:");
> > +
> > +        /* execve() can only ever return if it fails */
> > +        execvp(git_command, (char **)argv);
> > +
> > +        trace_printf("trace: exec failed: %s\n", strerror(errno));
> > +
> > +        argv[0] = tmp;
> > +
> > +        return -1;
> >  }
>
> I am not sure that this is elegant enough: Something like this (completely
> untested) might be better:
>
> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..c928f37 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
>          int i;
>          const char *paths[] = { current_exec_path,
>                                  getenv(EXEC_PATH_ENVIRONMENT),
> -                                builtin_exec_path };
> +                                builtin_exec_path,
> +                                "" };
>
>          for (i = 0; i < ARRAY_SIZE(paths); ++i) {
>                  size_t len;
> @@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)
>                  const char *exec_dir = paths[i];
>                  const char *tmp;
>
> -                if (!exec_dir || !*exec_dir) continue;
> +                if (!exec_dir) continue;
>
> -                if (*exec_dir != '/') {
> +                if (!*exec_dir)
> +                        /* try PATH */
> +                        *git_command = '\0';
> +                else if (*exec_dir != '/') {
>                          if (!getcwd(git_command, sizeof(git_command))) {
>                                  fprintf(stderr, "git: cannot determine "
>                                          "current directory: %s\n",
> @@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)
>
>                  len = strlen(git_command);
>                  rc = snprintf(git_command + len, sizeof(git_command) -
len,
> -                              "/git-%s", argv[0]);
> +                              "%sgit-%s", *exec_dir ? "/" : "", argv[0]);
>                  if (rc < 0 || rc >= sizeof(git_command) - len) {
>                          fprintf(stderr,
>                                  "git: command name given is too long.\n");
>
> Ciao,
> Dscho
>
> -
> 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: Git User's Survey 2007 unfinished summary continued
From: Andreas Ericsson @ 2007-10-20  8:03 UTC (permalink / raw)
  To: Federico Mena Quintero; +Cc: Johannes Schindelin, Jakub Narebski, git
In-Reply-To: <1192827476.4522.93.camel@cacharro.xalalinux.org>

Federico Mena Quintero wrote:
> 
> "Find out why people find git hard to learn and eliminate those barriers
> to entry" is what we do with usability tests e.g. in GNOME.

And what every major corporation who's serious about UI's do. Windows
works the way it does because that's how idiots expect it to work. Sad but
true. If our aim is world domination, we need not cater to the morons, but
we must make it easier for them to start learning on their own.

>  You ask
> people to use your software to accomplish well-defined tasks ("send a
> mail to foo@bar.com", "using the word processor, copy this fancy printed
> layout").  Then, you see how they *expect* your software to work, you
> see in which places it doesn't behave like that, and you fix it.  This
> produces very good results.  For Git in particular this could be things
> like, "Import this project from SVN, fix a bug, commit the patch", or
> "You are a maintainer, merge in these two branches from two
> contributors".
> 

I like it. So much so that I'll see if I can get a non-programmer at work
to do these tasks. Now... to assemble that task-list. Suggestions welcome.

> 
> It's hard to know where to begin :)  Do I need "git-cherry-pick" or
> "git-cherry"?  Why is the "apply a patch" command called "git-am"?  Why
> is it different from "git-apply"?  From "git-applypatch"?  Etc.
> 

I agree completely. It wouldn't be hard to make git-apply figure out if
it's being fed something that 'am' would normally want, and if it's being
fed it inside a git repo. If so, make it work just like 'am'.
git-applypatch was deprecated a long time ago and has already been removed.

Personally, I can't help but think that the numerous times I've heard "oh
gods, that's a lot of commands" should finally mean something. I've started
taking a look at which of them one can bundle together. If we can drop the
porcelainish commands down to ~30 or so, and hide the plumbing from git-<tab>
listings, the initial hurdle people have to jump would be significantly lower.

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

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Mike Hommey @ 2007-10-20  7:46 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Ari Entlich, git
In-Reply-To: <20071020063014.GU14735@spearce.org>

On Sat, Oct 20, 2007 at 02:30:14AM -0400, Shawn O. Pearce wrote:
> Ari Entlich <lmage11@twcny.rr.com> wrote:
> > On Thu, 2007-10-18 at 21:54 -0400, Shawn O. Pearce wrote:
> > > --index is used in Git for places were we update *both* the index
> > > and the working directory (git-apply --index). So actually I should
> > > have suggested "git-mv --index".  Whoops.
> > 
> > Alright then, I don't know about that particular convention. If this
> > behavior can't be made default, git mv --index should activate it? I
> > there anything else that might be more descriptive?
> 
> That's always the hard part.  I actually think the current behavior
> should be called --index as it does not only the working tree
> update but also stages the whole file into the index, which is what
> git-apply --index does.
> 
> What about just -u for "keep unstaged"?

Why not --staged ? It would be more meaningful than the --cached in some
other commands.

Mike

^ permalink raw reply

* Re: Announcement of Git wikibook
From: Ciprian Dorin Craciun @ 2007-10-20  7:40 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Evan Carroll, git
In-Reply-To: <C0D5CAE0-A152-4572-81D5-AF2A78DD89C6@zib.de>

    There is nothing wrong with either of the two approaches. They
could both coexist but address different needs:
    -- the manual should be more oriented on technical issues and
addresses only the most recent versions;
    -- the book should be more user-oriented, and more general,
explaining how source management should be addressed by using git, and
maybe make comparisons with may other versioning systems. Also the
book could relate to many versions -- both old and new.

    Also I would note that the wiki book is more easy to edit... If
you spot errors or want to add something you just go and edit it and
the effect is immediate. But in contrast sending patches involves some
overhead...

    Ciprian.


On 10/19/07, Steffen Prohaska <prohaska@zib.de> wrote:
>
> On Oct 19, 2007, at 10:21 PM, Evan Carroll wrote:
>
> > I've create a git wikibook if anyone wants to help expand it.
> > http://en.wikibooks.org/wiki/Source_Control_Management_With_Git
>
> I'm just curious. What is the advantage of a wikibook?
>
> We already have a manual
>
> http://www.kernel.org/pub/software/scm/git/docs/user-manual.html
>
> including a todo list
>
> http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#todo
>
> So, why don't you send patches improving the manual, but instead
> started a wiki book from scratch?
>
>         Steffen

^ permalink raw reply

* Re: [PATCH] When exec'ing sub-commands, fall back on execvp (the PATH)
From: Johannes Schindelin @ 2007-10-20  7:30 UTC (permalink / raw)
  To: Scott Parish; +Cc: git
In-Reply-To: <20071020064459.GB2237@srparish.net>

Hi,

On Fri, 19 Oct 2007, Scott Parish wrote:

> diff --git a/exec_cmd.c b/exec_cmd.c
> index 9b74ed2..674c9f3 100644
> --- a/exec_cmd.c
> +++ b/exec_cmd.c
> @@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)
>  {
>  	char git_command[PATH_MAX + 1];
>  	int i;
> +	int rc;
>  	const char *paths[] = { current_exec_path,
>  				getenv(EXEC_PATH_ENVIRONMENT),
>  				builtin_exec_path };
> +	const char *tmp;
> +	size_t len;
>  
>  	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
> -		size_t len;
> -		int rc;
>  		const char *exec_dir = paths[i];
> -		const char *tmp;
>  
>  		if (!exec_dir || !*exec_dir) continue;
>  
> @@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)
>  
>  		argv[0] = tmp;
>  	}
> -	return -1;
>  
> +	rc = snprintf(git_command, sizeof(git_command), "git-%s", argv[0]);
> +	if (rc < 0 || rc >= sizeof(git_command) - len) {
> +		fprintf(stderr, "git: command name given is too long.\n");
> +		return -1;
> +	}
> +
> +	tmp = argv[0];
> +	argv[0] = git_command;
> +
> +	trace_argv_printf(argv, -1, "trace: exec:");
> +
> +	/* execve() can only ever return if it fails */
> +	execvp(git_command, (char **)argv);
> +
> +	trace_printf("trace: exec failed: %s\n", strerror(errno));
> +
> +	argv[0] = tmp;
> +
> +	return -1;
>  }

I am not sure that this is elegant enough: Something like this (completely 
untested) might be better:

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..c928f37 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -36,7 +36,8 @@ int execv_git_cmd(const char **argv)
 	int i;
 	const char *paths[] = { current_exec_path,
 				getenv(EXEC_PATH_ENVIRONMENT),
-				builtin_exec_path };
+				builtin_exec_path,
+				"" };
 
 	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
 		size_t len;
@@ -44,9 +45,12 @@ int execv_git_cmd(const char **argv)
 		const char *exec_dir = paths[i];
 		const char *tmp;
 
-		if (!exec_dir || !*exec_dir) continue;
+		if (!exec_dir) continue;
 
-		if (*exec_dir != '/') {
+		if (!*exec_dir)
+			/* try PATH */
+			*git_command = '\0';
+		else if (*exec_dir != '/') {
 			if (!getcwd(git_command, sizeof(git_command))) {
 				fprintf(stderr, "git: cannot determine "
 					"current directory: %s\n",
@@ -81,7 +85,7 @@ int execv_git_cmd(const char **argv)
 
 		len = strlen(git_command);
 		rc = snprintf(git_command + len, sizeof(git_command) - len,
-			      "/git-%s", argv[0]);
+			      "%sgit-%s", *exec_dir ? "/" : "", argv[0]);
 		if (rc < 0 || rc >= sizeof(git_command) - len) {
 			fprintf(stderr,
 				"git: command name given is too long.\n");

Ciao,
Dscho

^ permalink raw reply related

* Issues for git-submodule
From: Yin Ping @ 2007-10-20  7:31 UTC (permalink / raw)
  To: git
In-Reply-To: <46dff0320710200027o5fe434b4i9bd4f3ffc17f03f6@mail.gmail.com>

1. gti-submodule status
   As the manual says, '+'  is shown if the currently checked out
submodule commit does not match the SHA-1 found in the index of the
containing repository.

  However, not matching has two cases: one is a new commit in the
submodule, the other  is update of index of the submodule after a
command such as "git-pull".

  So which is the case when a '+' is seen? Should i run 'git-commit'
or 'git-submodule update'? As a suggestion, I think git should tell
the user which commit  is newer (the one in supermodule index or the
HEAD of the submodule) and even give the log entry between the two
commits.

2. As I first saw 'git-submodule status submoduepath',  I thought it
should do the following thing
   cd submoduepath && git-status && cd -

   This is actually what i need, espacially when i have a lot of
submodules and has made changes in some modules. How do i know which
modules have been changed and how they have been changed?

  So, In some degree, I even think current 'git-submodule status'
should be replaced by git-status and the right thing 'git-submodule
status' should do is to show the status of submodule itself as i just
said above.


--
franky

^ permalink raw reply

* [PATCH] Deduce exec_path also from calls to git with a relative path
From: Johannes Schindelin @ 2007-10-20  7:21 UTC (permalink / raw)
  To: Scott R Parish; +Cc: git, spearce, gitster
In-Reply-To: <20071020064617.GC2237@srparish.net>


There is already logic in the git wrapper to deduce the exec_path from
argv[0], when the git wrapper was called with an absolute path.  Extend
that logic to handle relative paths as well.

For example, when you call "../../hello/world/git", it will not turn
"../../hello/world" into an absolute path, and use that.

Initial implementation by Scott R Parish.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Fri, 19 Oct 2007, Scott R Parish wrote:

	>  Signed-off-by: Scott R Parish <srp@srparish.net>

	That is a little short for a commit message ;-)

	>  git.c |   35 +++++++++++++++++++++++++++++++++--
	>  1 files changed, 33 insertions(+), 2 deletions(-)

	I had commented on this before.  Probably I did a very bad job 
	at explaining things, so hopefully this is better:

 git.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index d7c6bca..1dad764 100644
--- a/git.c
+++ b/git.c
@@ -414,13 +414,14 @@ int main(int argc, const char **argv)
 	/*
 	 * Take the basename of argv[0] as the command
 	 * name, and the dirname as the default exec_path
-	 * if it's an absolute path and we don't have
-	 * anything better.
+	 * if we don't have anything better.
 	 */
 	if (slash) {
 		*slash++ = 0;
 		if (*cmd == '/')
 			exec_path = cmd;
+		else
+			exec_path = xstrdup(make_absolute_path(cmd));
 		cmd = slash;
 	}
 
-- 
1.5.3.4.1287.g8b31e

^ permalink raw reply related

* Re: Proposed git mv behavioral change
From: Shawn O. Pearce @ 2007-10-20  7:02 UTC (permalink / raw)
  To: Michael Witten; +Cc: git
In-Reply-To: <8D972813-2D7F-4D6A-958F-B76E947E7BC3@MIT.EDU>

Michael Witten <mfwitten@MIT.EDU> wrote:
> 
> On 20 Oct 2007, at 2:36:28 AM, Shawn O. Pearce wrote:
> 
> >Today I move the file, then unstage the hunks I'm not sure about,
> >then go back and restage them.  Annoying.  It really disrupts
> >my workflow.
> 
> I know it's against policy, but the proposed change should be set
> as the default at some point, in my opinion.

Its not so much policy as a timing issue.

Git 1.5.4 shouldn't have major distruptions to end-users in terms
of changing existing behavior to be different than in 1.5.3.
Especially default behavior.

Git 1.6.0 we're a little bit more willing to change default
behavior as it is a full major release.  This is probably quite a
bit off still, unless we started to accumulate a lot of really good
changes that required breaking something in the user interface.
That's what happened with the 1.4.4 series vs. 1.5.0.  Right now
I don't see that happening anytime soon.

> I have a feeling that my suggestion will not go far,
> but I also think that backwards compatibility can
> overstay its welcome.

I agree.

-- 
Shawn.

^ permalink raw reply

* Re: [RFC/PATCH] git-fetch: mega-terse fetch output
From: Shawn O. Pearce @ 2007-10-20  6:58 UTC (permalink / raw)
  To: Jeff King
  Cc: Nicolas Pitre, Johannes Sixt, Theodore Tso, Santi Béjar,
	David Symonds, git
In-Reply-To: <20071020050019.GA27282@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Fri, Oct 19, 2007 at 10:14:59AM -0400, Nicolas Pitre wrote:
> 
> > > ==> git://repo.or.cz/git/spearce.git
> > >  * (new)              gitk -> spearce/gitk
> > >  * 1aa3d01..e7187e4   maint -> spearce/maint
> > >  * de61e42..7840ce6   master -> spearce/master
> > >  * 895be02..2fe5433   next -> spearce/next
> > >  + 89fa332...1e4c517  pu -> spearce/pu
> > >  * (new)              todo -> spearce/todo
> > 
> > Actually I think this is the best format so far: one line per branch, no 
> > terminal width issue (long branch names are simply wrapped), the 
> > old..new info is there also with the single character marker to quickly 
> > notice the type of update.

Yea, I think this is almost the right format.

Nicolas Pitre <nico@cam.org> wrote:
> Agreed.  ' ' = fast forward, '+' = forced update, and '!' = refused.
 
We're probably looking at something like this:

>From git://repo.or.cz/git/spearce.git
   1aa3d01..e7187e4   maint -> spearce/maint
   de61e42..7840ce6   master -> spearce/master
   895be02..2fe5433   next -> spearce/next
   (new)              todo -> spearce/todo
   (new)              tag v1.6.0
 + 89fa332...1e4c517  pu -> spearce/pu  (forced update)
 ! 2b5afb...289840    gitk -> spearce/gitk (non-fast forward)

Notice the sorting order by *type* of update.  I think it makes
the code slightly more complicated in builtin-fetch as we need to
classify each ref into a type of update, then sort them by that
type, but it allows the end-user to see the most "important" (not
simple fast-forward updates) at the end of their terminal window,
especially if there were many fast-forward branches.  Within a
class of update we still sort by ref name.

> Technically speaking, the hash IDs can be up to 80 characters long,
> since they are meant to be unique abbreviations. But in practice, I
> think leaving enough space for 10 + '...' + 10 should accomodate just
> about any project (IIRC, the kernel's longest non-unique is around 9).

Which nicely solves the issue with the window size as we aren't
really worring about it here in this display.

-- 
Shawn.

^ permalink raw reply

* [PATCH] If git is ran with a relative path, try building an absolute exec_path from it
From: Scott R Parish @ 2007-10-20  6:46 UTC (permalink / raw)
  To: git

 Signed-off-by: Scott R Parish <srp@srparish.net>

---
 git.c |   35 +++++++++++++++++++++++++++++++++--
 1 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 9eaca1d..d129ecc 100644
--- a/git.c
+++ b/git.c
@@ -28,6 +28,35 @@ static void prepend_to_path(const char *dir, int len)
 	free(path);
 }
 
+static char *rel_to_abs_exec_path(const char *cmd) {
+	int len, rc;
+	char *exec_path = xmalloc(PATH_MAX + 1);
+
+	if (!getcwd(exec_path, PATH_MAX)) {
+		fprintf(stderr, "git: cannot determine current directory: %s\n",
+			strerror(errno));
+		free(exec_path);
+		return NULL;
+	}
+	len = strlen(exec_path);
+
+	/* Trivial cleanup */
+	while (!prefixcmp(cmd, "./")) {
+		cmd += 2;
+		while (*cmd == '/')
+			cmd++;
+	}
+
+	rc = snprintf(exec_path + len, PATH_MAX - len, "/%s", cmd);
+	if (rc < 0 || rc >= PATH_MAX - len) {
+		fprintf(stderr, "git: command name given is too long.\n");
+		free(exec_path);
+		return NULL;
+	}
+
+	return exec_path;
+}
+
 static int handle_options(const char*** argv, int* argc, int* envchanged)
 {
 	int handled = 0;
@@ -409,13 +438,15 @@ int main(int argc, const char **argv)
 	/*
 	 * Take the basename of argv[0] as the command
 	 * name, and the dirname as the default exec_path
-	 * if it's an absolute path and we don't have
-	 * anything better.
+	 * if we don't have anything better.
 	 */
 	if (slash) {
 		*slash++ = 0;
 		if (*cmd == '/')
 			exec_path = cmd;
+		else
+			exec_path = rel_to_abs_exec_path(cmd);
+
 		cmd = slash;
 	}
 
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-20  6:45 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20071020063628.GV14735@spearce.org>


On 20 Oct 2007, at 2:36:28 AM, Shawn O. Pearce wrote:

> Today I move the file, then unstage the hunks I'm not sure about,
> then go back and restage them.  Annoying.  It really disrupts
> my workflow.

I know it's against policy, but the proposed change should be set
as the default at some point, in my opinion.

Perhaps when the -u flagged is not used, there can be a warning that
states -u will become the default at a certain time.

In fact, -u speaks "update" to me, and I would expect it to signal
the current behavior.

I have a feeling that my suggestion will not go far,
but I also think that backwards compatibility can
overstay its welcome.

^ permalink raw reply

* [PATCH] When exec'ing sub-commands, fall back on execvp (the PATH)
From: Scott Parish @ 2007-10-20  6:44 UTC (permalink / raw)
  To: git

 Signed-off-by: Scott R Parish <srp@srparish.net>

---
 exec_cmd.c |   26 ++++++++++++++++++++++----
 1 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/exec_cmd.c b/exec_cmd.c
index 9b74ed2..674c9f3 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -34,15 +34,15 @@ int execv_git_cmd(const char **argv)
 {
 	char git_command[PATH_MAX + 1];
 	int i;
+	int rc;
 	const char *paths[] = { current_exec_path,
 				getenv(EXEC_PATH_ENVIRONMENT),
 				builtin_exec_path };
+	const char *tmp;
+	size_t len;
 
 	for (i = 0; i < ARRAY_SIZE(paths); ++i) {
-		size_t len;
-		int rc;
 		const char *exec_dir = paths[i];
-		const char *tmp;
 
 		if (!exec_dir || !*exec_dir) continue;
 
@@ -106,8 +106,26 @@ int execv_git_cmd(const char **argv)
 
 		argv[0] = tmp;
 	}
-	return -1;
 
+	rc = snprintf(git_command, sizeof(git_command), "git-%s", argv[0]);
+	if (rc < 0 || rc >= sizeof(git_command) - len) {
+		fprintf(stderr, "git: command name given is too long.\n");
+		return -1;
+	}
+
+	tmp = argv[0];
+	argv[0] = git_command;
+
+	trace_argv_printf(argv, -1, "trace: exec:");
+
+	/* execve() can only ever return if it fails */
+	execvp(git_command, (char **)argv);
+
+	trace_printf("trace: exec failed: %s\n", strerror(errno));
+
+	argv[0] = tmp;
+
+	return -1;
 }
 
 
-- 
1.5.3.GIT

^ permalink raw reply related

* Re: [PATCH] allow git to use the PATH for finding subcommands and help docs
From: Scott Parish @ 2007-10-20  6:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Johannes Sixt, git
In-Reply-To: <Pine.LNX.4.64.0710191616490.16728@wbgn129.biozentrum.uni-wuerzburg.de>

On Fri, Oct 19, 2007 at 04:27:39PM +0200, Johannes Schindelin wrote:

> While reading this, I have to wonder why it is not just simpler to try 
> with builtin_exec_path first, and if that fails, just let exec() find the 
> program in the PATH?

I think you're right; that is a much better way to do this. I've
rewritten this as two patches i'll post shortly. I have mixed
feelings about the MANPATH/PERL5LIB hack, so i'm leaving it out for
now.

sRp

-- 
Scott Parish
http://srparish.net/

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-20  6:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Ari Entlich, Michael Witten, git
In-Reply-To: <20071020063628.GV14735@spearce.org>

On Sat, Oct 20, 2007 at 02:36:28AM -0400, Shawn O. Pearce wrote:

> So lets say I make a change in my Makefile that changes the name
> of a source file to be more descriptive of that file's contents.
> I've reviewed the Makefile change *and* done the file rename,
> but I'm still not done reviewing the stuff in the file.  (Yea,
> maybe that should be two different commits, but maybe not, lets
> not get into that as it depends very much on context.)

Right. So the exact state you have in your index never actually existed
in your working tree. But that's OK if:
  - the changes are trivial and obviously correct
  - you're not actually planning on _committing_ that, you just want to
    build the commit using the index
And in those cases, git-stash is either, respectively, overkill or
totally useless.

Please ignore everything I said on this subject before today...I
obviously hadn't figured out what was going on.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Shawn O. Pearce @ 2007-10-20  6:36 UTC (permalink / raw)
  To: Jeff King; +Cc: Ari Entlich, Michael Witten, git
In-Reply-To: <20071020062400.GA30388@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> No, I meant more that the state you have staged will never have actually
> existed in your working tree, so you could not possibly have tested it.
> However, it may be that the changes you are trying to avoid staging are
> trivial, and you're willing to accept that. And git shouldn't stand in
> your way.
> 
> And it sounds like you don't necessarily want to make a _commit_ out of
> this, you just want to keep working and not have git-mv munge your
> staged state.

Yea, I want the same thing.  :-)

And not because I commit things that weren't tested.  Its because
I do mini code reviews for small hunks at a time.  I stage things
I have looked at, and keep things I'm still unsure about unstaged.
But before I commit I make sure my `git diff` output is empty,
so I know that any tests I do now will match what I have when I
run git-commit.

So lets say I make a change in my Makefile that changes the name
of a source file to be more descriptive of that file's contents.
I've reviewed the Makefile change *and* done the file rename,
but I'm still not done reviewing the stuff in the file.  (Yea,
maybe that should be two different commits, but maybe not, lets
not get into that as it depends very much on context.)

Today I move the file, then unstage the hunks I'm not sure about,
then go back and restage them.  Annoying.  It really disrupts
my workflow.

The killer feature in Git for me is the index.  That sucker saves
me on a daily basis as it lets me organize "this i'm happy with,
this i'm not" while I'm in the middle of making a single new commit.
Today git-mv robs me of it.
 
-- 
Shawn.

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-20  6:36 UTC (permalink / raw)
  To: Michael Witten; +Cc: git
In-Reply-To: <FF5A82CA-8AE3-4E1C-872D-26D938933E1B@MIT.EDU>

On Sat, Oct 20, 2007 at 02:34:49AM -0400, Michael Witten wrote:

> The first method is wrong, because I wasn't considering the fact that
> he may have already staged something.

Yes, I also didn't consider that, which is what led me to wonder what
his use case could possibly be. And thus we were both confused.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Michael Witten @ 2007-10-20  6:34 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20071020062400.GA30388@coredump.intra.peff.net>


On 20 Oct 2007, at 2:24:00 AM, Jeff King wrote:

> Because I was having trouble understanding what Michael was trying to
> accomplish with his "here are two methods, and one is better" since  
> they
> seemed to do different things. But that is perhaps my fault for  
> joining
> the thread in the middle. I may have simply caused more confusion by
> getting involved. :)

The first method is wrong, because I wasn't considering the fact that
he may have already staged something.

Because I thought both methods achieve the same result, I called the  
latter
"better", because it is more efficient.

Michael Witten
mfwitten

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-20  6:34 UTC (permalink / raw)
  To: Ari Entlich; +Cc: Shawn O. Pearce, git
In-Reply-To: <1192859758.13347.148.camel@g4mdd.entnet>

On Sat, Oct 20, 2007 at 01:55:58AM -0400, Ari Entlich wrote:

> Might it be possible to simply get the struct cache_entry for the file
> which we want to rename, change its name property (would this involve
> xreallocing it?), and change the ce_namelen field of ce_flags?

The sort order of entries in the cache is important, so you would have
to move the entry, as well.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-20  6:31 UTC (permalink / raw)
  To: Ari Entlich; +Cc: Wincent Colaiuta, Michael Witten, git
In-Reply-To: <1192859753.13347.147.camel@g4mdd.entnet>

On Sat, Oct 20, 2007 at 01:55:53AM -0400, Ari Entlich wrote:

> git-stash has been suggested to me numerous times, but I really feel
> that there's no need to use it in this case - if the git mv command gave
> adequate control to the user, it would be unnecessary.

Yes, now that I understand what you're doing, git-stash is probably
overkill. It's great if you really want to put aside your current state
and do some other, totally unrelated work.

But in this case, you are talking about tools for managing the index
correctly, which is a totally different use case.

> > git mv --cached foo bar
> Are you talking about REALLY only changing the index? I can't think of
> why you'd want to do this either... After all, wouldn't there be no bar
> file to do git add --interactive on? In addition, I don't think giving
> --interactive a filename is meaningful...

Originally I had thought git-mv --cached would be useful (and a nice
orthogonal design), but it really _does_ seem useless, since it's
awkward to get your working tree changes into the index without actually
moving the working tree file.

> It might be interesting to do some sort of survey of whether people
> depend on this behavior. It seems pretty inconsistent with how git works
> otherwise, and I'd be surprised if a lot of people expect it (kinda like
> the Spanish Inquisition :-P).

The best way to get comments is to make a patch.

As for inconsitency of git-mv, it has always been a bit of an odd duck.
I'm fairly certain neither Linus nor Junio actually use it, preferring
instead to make all changes to the working tree and then update the
index manually. I even seem to recall Linus arguing that git-mv was
stupid and unnecessary a long time ago, but I could be wrong.

-Peff

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Shawn O. Pearce @ 2007-10-20  6:30 UTC (permalink / raw)
  To: Ari Entlich; +Cc: git
In-Reply-To: <1192859758.13347.148.camel@g4mdd.entnet>

Ari Entlich <lmage11@twcny.rr.com> wrote:
> On Thu, 2007-10-18 at 21:54 -0400, Shawn O. Pearce wrote:
> > --index is used in Git for places were we update *both* the index
> > and the working directory (git-apply --index). So actually I should
> > have suggested "git-mv --index".  Whoops.
> 
> Alright then, I don't know about that particular convention. If this
> behavior can't be made default, git mv --index should activate it? I
> there anything else that might be more descriptive?

That's always the hard part.  I actually think the current behavior
should be called --index as it does not only the working tree
update but also stages the whole file into the index, which is what
git-apply --index does.

What about just -u for "keep unstaged"?
 
> Might it be possible to simply get the struct cache_entry for the file
> which we want to rename, change its name property (would this involve
> xreallocing it?), and change the ce_namelen field of ce_flags?

Yes.
 
> In any case, I think the ce_flags would need to be changed to reflect
> the new name length. Also, it seems that the old ce_mtime, ce_dev,
> ce_ino, ce_uid, ce_gid, and ce_size could be used too... ce_ctime would
> need to be updated...

Correct.  The ce_size shouldn't change as a result of the rename but
the other properties of a struct stat may.  I'd like to think that
ce_uid and ce_gid wouldn't change as a result of a rename but there's
probably some bastard filesystem somewhere that that won't hold true.

Just call fill_stat_cache_info() on your xrealloc'd entry and pass
it a stat buffer for the file after the rename and you should be
covered here.

> > I'm sure Junio could probably give you a better starting point
> > than I can, as he's more familiar with this sort of code, but that
> > should still get you looking in the right direction and maybe get
> > a working implementation together that you can share for discussion.
> 
> Yeah, I'd appreciate any help I can get. The people on #git were
> invoking the "the code is the documentation" ideology. :)

It really is in this case.  You are talking about hacking on
Git itself.  For the most part we believe in "the code is the
documentation" for the guts as otherwise the documentation gets
out of sync with the code when the code changes.  User level docs
are different as the user interface needs to stay stable.

> So... Ping Junio...? I actually haven't seen any messages from him since
> I subscribed to this list (I've only gotten about 300 messages so far,
> but I'd expect to see a lot from him as he's the maintainer...). Is he
> away at the moment or something?

Yes.  We run like 100 messages a day so I guess you've only joined
in the past few days.  Welcome to the list.  :-)

Junio has been away for about 2 weeks now.  He's basically off
on leave as he needed some time to take care of some things.
I'm filling in for him until he returns.

-- 
Shawn.

^ permalink raw reply

* Re: Proposed git mv behavioral change
From: Jeff King @ 2007-10-20  6:24 UTC (permalink / raw)
  To: Ari Entlich; +Cc: Michael Witten, git
In-Reply-To: <1192859748.13347.146.camel@g4mdd.entnet>

On Sat, Oct 20, 2007 at 01:55:48AM -0400, Ari Entlich wrote:

> Well, I also want to move the working directory file so that the index
> and the working directory still match up and so that their differences
> are preserved. Therefore, --cached isn't quite right (see Shawn's
> messages and my response to them).

Ah, OK. I understand now. So what you would want could be accomplished
with something like:

  mv A B
  (git-ls-files -s | sed s/A$/B/;
   echo 0 0000000000000000000000000000000000000000	A) \
  | git-update-index --index-info

IOW:
  1. move working tree A to B
  2. create staged entry B, same as staged entry A
  3. remove staged entry A

> Hmm, that's an interesting point. Are you talking about situations in
> which your changes after (1) leave the program in an uncompilable state?

No, I meant more that the state you have staged will never have actually
existed in your working tree, so you could not possibly have tested it.
However, it may be that the changes you are trying to avoid staging are
trivial, and you're willing to accept that. And git shouldn't stand in
your way.

And it sounds like you don't necessarily want to make a _commit_ out of
this, you just want to keep working and not have git-mv munge your
staged state.

> I don't really see why you're analyzing the situations in which this
> would be used. I think it should be obvious from my descriptions of my

Because I was having trouble understanding what Michael was trying to
accomplish with his "here are two methods, and one is better" since they
seemed to do different things. But that is perhaps my fault for joining
the thread in the middle. I may have simply caused more confusion by
getting involved. :)

Now I think I understand exactly what you're trying to accomplish,
and I agree that it makes sense.  Shawn is right that "git-mv --cached"
doesn't do what you want, since to be consistent with other tools it
wouldn't touch the working tree (though I wonder if you would be
satisfied with "git-mv --cached A B; mv A B"). I think at this point the
best way forward is to produce a patch and try to solicit comments (in
particular, which behaviors should be supported (current, --cached, or
what you are proposing) and which should be the default).

> But... that's not even the point I was going to make. I think the
> questions you should be asking are things like "Does this fit with the
> overall architecture?", "Does this or doesn't this provide power and
> flexibility to the user?", etc. Is git being made for "the 80%" that use

There seems to be a strong sentiment among the various gits that you
shouldn't build in flexibility for the sake of flexibility if there's no
good use for it. I tend to be less of that sentiment than some others
here, but I think when proposing a feature it is still worth saying "and
here's why it's useful."

> Sorry if anything I've said here sounded confrontational; that was not
> not my intent at all. I'm just raising some points I think are
> important. I don't want any flame wars...

I think you stated your position fine. And we've had enough flame wars
around here lately to last us a while.

-Peff

^ 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