Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Alex Riesen @ 2007-11-06  7:28 UTC (permalink / raw)
  To: Michael Cohen; +Cc: Gerrit Pape, Fernando J. Pereda, git, Junio C Hamano
In-Reply-To: <635FFEC2-2489-443B-8425-DF2B58BE23C2@mac.com>

Michael Cohen, Tue, Nov 06, 2007 02:41:56 +0100:
> On Nov 5, 2007, at 5:52 PM, Alex Riesen wrote:
>
>> Gerrit Pape, Mon, Nov 05, 2007 13:49:20 +0100:
>>> +	for (i = 0; i < 2; ++i) {
>>> +		snprintf(name, sizeof(name), "%s/%s", path, sub[i]);
>>> +		if ((dir = opendir(name)) == NULL) {
>>> +			error("cannot opendir %s (%s)", name, strerror(errno));
>>> +			return -1;
>>> +		}
>>
>> Why is missing "cur" (or "new", for that matter) a fatal error?
>> Why is it error at all? How about just ignoring the fact?
> In Maildir format, cur and new hold the mails. :P

So? Why *STOP* reading the mails if just one of the directories could
not be opened? IOW, I suggest:

+	for (i = 0; i < 2; ++i) {
+		snprintf(name, sizeof(name), "%s/%s", path, sub[i]);
+		dir = opendir(name);
+		if (!dir)
+			continue;

^ permalink raw reply

* Re: git pull opinion
From: Ralf Wildenhues @ 2007-11-06  7:34 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Aghiles, git list
In-Reply-To: <176851C5-D735-4DDC-B799-A5106CD03989@lrde.epita.fr>

Hello,

* Benoit Sigoure wrote on Tue, Nov 06, 2007 at 06:29:58AM CET:
> On Nov 6, 2007, at 5:16 AM, Aghiles wrote:
>
>>> who will run git stash clear? :)
>>
>> Yes you are right. By the way, in the context of merging into a
>> dirty tree, "git stash clear" seems to be a dangerous command:
>> there is a risk of loosing all your changes without a question
>> asked!

I would love it if for once in the git world, there were a pair of
commands that would do the exact opposite of each other and where the
naive newbie (me) would immediately recognize that from their names:
  git stash push
  git stash pop

Both applied in this order should be a no-op on both the working tree,
the index, and also the stash.  There's room for extensions (pop
--keep-stash to not remove the stashed information), explicit naming of
stashes, doing multiple pops at once, and so on.  Please don't add more
of the git-push/git-pull, git-add/git-rm unsymmetrical interfaces.
Even if they're perfectly clear to git intimates, each one of them
takes precious extra time to learn due to this lack of symmetry.

Since I simply don't have the time resources to just implement that,
I'll thank you for your attention and go back to lurking mode now.

Thanks,
Ralf

^ permalink raw reply

* Re: git pull opinion
From: Alex Riesen @ 2007-11-06  7:38 UTC (permalink / raw)
  To: Pierre Habouzit, Bill Lear, Junio C Hamano, Aghiles, git
In-Reply-To: <20071106004601.GS8939@artemis.corp>

Pierre Habouzit, Tue, Nov 06, 2007 01:46:01 +0100:
> On Tue, Nov 06, 2007 at 12:36:16AM +0000, Bill Lear wrote:
> > On Monday, November 5, 2007 at 15:33:31 (-0800) Junio C Hamano writes:
> > > Stop thinking like "I need to integrate the changes from upstream
> > > into my WIP to keep up to date."
> > >
> > > [...]
> > >
> > > Once you get used to that, you would not have "a dirty directory"
> > > problem.
> > 
> > I respectfully beg to differ.  I think it is entirely reasonable, and
> > not a sign of "centralized" mindset, to want to pull changes others
> > have made into your dirty repository with a single command.
> 
>   I agree, I have such needs at work.  Here is how we (very informally)
> work: people push things that they believe could help other (a new
> helper function, a new module, a bug fix) in our master ASAP, but
> develop big complex feature in their repository and merge into master
> when it's ready.
> 
>   Very often we discuss some bugfix that is impeding people, or a
> most-wanted-API. Someone does the work, commits, I often want to merge
> master _directly_ into my current work-branch, because I want the
> fix/new-API/... whatever.

How about merging just that "fix/new-API/... whatever" thing and not
the whole master, which should be a complete mess by now?

The way you explained it it looks like typical centralized workflow.

^ permalink raw reply

* Re: git pull opinion
From: Alex Riesen @ 2007-11-06  7:40 UTC (permalink / raw)
  To: Aghiles; +Cc: Bill Lear, Junio C Hamano, git
In-Reply-To: <3abd05a90711052230y4d6151c6o3e7985a0c8e18161@mail.gmail.com>

Aghiles, Tue, Nov 06, 2007 07:30:23 +0100:
> > I respectfully beg to differ.  I think it is entirely reasonable, and
> > not a sign of "centralized" mindset, to want to pull changes others
> > have made into your dirty repository with a single command.
> 
> BitKeeper, for example, does a merge with a "dirty" directory.

Git does merge with dirty working directory. It just wont touch the
changed files and stop merging if the merge requires it.

^ permalink raw reply

* Re: git pull opinion
From: Aghiles @ 2007-11-06  7:45 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: git list
In-Reply-To: <176851C5-D735-4DDC-B799-A5106CD03989@lrde.epita.fr>

Hello,

> As far as I remember, a patch was proposed to change this mis-
> behavior of "git stash" (one could argue that it's a PEBCAK issue,
> but I really think this command is *way* too dangerous) but I don't
> think it's been accepted at this time.

I think that people will use this a lot with the pull command and some
accidents will happen.  I am of the opinion that the semantics of this
command must be  changed.
Additionally, having "git stash [command]" and "git stash [argument]"
mixed together seems strange. One suggestion would be:

    git stash store/add/create [stash-name]
    git stash apply [stash-name]
    git stash clear <stash-name>  (accepts wildcards but no empty args)
    ...

- Aghiles.

^ permalink raw reply

* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Jeff King @ 2007-11-06  7:51 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Michael Cohen, Gerrit Pape, Fernando J. Pereda, git,
	Junio C Hamano
In-Reply-To: <20071106072831.GA3021@steel.home>

On Tue, Nov 06, 2007 at 08:28:31AM +0100, Alex Riesen wrote:

> > In Maildir format, cur and new hold the mails. :P
> 
> So? Why *STOP* reading the mails if just one of the directories could
> not be opened? IOW, I suggest:

Because you are then trying to apply a patch series with some patches
potentially missing? Continuing only on errno == ENOENT seems prudent.

-Peff

^ permalink raw reply

* Re: Git-windows and git-svn?
From: Peter Karlsson @ 2007-11-06  8:02 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Pascal Obry, Abdelrazak Younes, Git Mailing List
In-Reply-To: <591E16CE-E303-4971-B57D-D84E883BB01D@zib.de>

Steffen Prohaska:

> Right. The experience should be good if you use binmode.

Shouldn't it be possible to fix this by sprinkling the appropriate
number of "b"s in the parameters to fopen()? Or is it more complicated
than that?

> But never use textmode. You'll not get an error right away. At first
> git seems to work. But later it reports weird errors. The experience
> is really bad. Don't do that.

I got errors almost right away when trying that (I need text mode to
interface with some other programs), so Cygwin-git is a no-go for me at
the moment. Of course, mixing msys-Git with Cygwin and ActiveState Perl
is also an interesting experience, to say the least :-)

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [ANNOUNCE] cgit v0.7
From: Peter Karlsson @ 2007-11-06  8:04 UTC (permalink / raw)
  To: Lars Hjemli; +Cc: git list
In-Reply-To: <8c5c35580711030408n658eb11fk19d554f0fa3b17@mail.gmail.com>

Lars Hjemli:

> cgit v0.7 (a fast webinterface for git) is now available at
> 
>      git://hjemli.net/pub/git/cgit

Has anyone managed to compile this for Win32? I cannot get gitweb to
work due to the Perl dependencies. When trying to compile cgit it hangs
checking the git version (or something like that, it keeps printing a
version number over and over again).

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: [RFC PATCH] Reduce the number of connects when fetching
From: Johannes Sixt @ 2007-11-06  8:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7v1wb4kuoc.fsf@gitster.siamese.dyndns.org>

Junio C Hamano schrieb:
> Daniel Barkalow <barkalow@iabervon.org> writes:
>> ... In particular, I don't know if there's a way to have the 
>> connection end up in a state where objects for more refs can be requested 
>> after some refs have been requested and the resulting objects read.
> 
> The upload-pack protocol goes "S: here are what I have, C: I
> want these, C: I have these, S: ok, continue, C: I have these,
> S: ok, continue, C: I have these, S: ok, I've heard enough, C:
> done, S: packfile is here", so after packfile generation starts
> there is nothing further the downloader can say.
> 
> Otherwise you would be able to do the tag following using the
> same connection, but that is unfortunately not a case.

How about:

S: here are what I have
C: I want these
C: want tags   <-- new
C: I have these
S: ok, continue
C: I have these
S: ok, continue
C: I have these
S: ok, these are the tags  <-- new
S: I've heard enough
C: done
S: packfile is here

The tags that the server provides are those (and only those[*]) that 
reference objects in the packfile that it's going to send.

[*] This way the client doesn't have to figure out which tags it wants; as a 
side-effect it won't accidentally fetch tags for objects that it happens to 
have in the repository, but aren't reachable from any ref (like what used to 
happen).

-- Hannes

^ permalink raw reply

* Re: Git-windows and git-svn?
From: Steffen Prohaska @ 2007-11-06  8:20 UTC (permalink / raw)
  To: Peter Karlsson; +Cc: Pascal Obry, Abdelrazak Younes, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0711060857140.8577@ds9.cixit.se>


On Nov 6, 2007, at 9:02 AM, Peter Karlsson wrote:

> Steffen Prohaska:
>
>> Right. The experience should be good if you use binmode.
>
> Shouldn't it be possible to fix this by sprinkling the appropriate
> number of "b"s in the parameters to fopen()? Or is it more complicated
> than that?

It's more complicated. I tried. But others convinced me that
it's harder than you think. The interfacing with other programs
through shell scripts is one major problem. Another difficulty
is that Cygwin's textmode is deprecated. So, the Cygwin guys
won't help. Search the list archives for more details.


>> But never use textmode. You'll not get an error right away. At first
>> git seems to work. But later it reports weird errors. The experience
>> is really bad. Don't do that.
>
> I got errors almost right away when trying that (I need text mode to
> interface with some other programs), so Cygwin-git is a no-go for  
> me at
> the moment.

Same for me. You're welcome to join the msysgit effort ;)

But do not expect too much from my side. For me the only
priority is to get the core git commands running. Everything
that is needed to support a git-only workflow has top priority.
Everything else has lowest priority. I'll refuse to put work
into the scripts interfacing with other SCMs and I'll refuse
to work on things needed to setup a git 'server', like git-shell.


> Of course, mixing msys-Git with Cygwin and ActiveState Perl
> is also an interesting experience, to say the least :-)

I can imagine that. If you already have some new insights how
to handle such a situration it would be interesting if you
could share some of them.

	Steffen

^ permalink raw reply

* Re: git pull opinion
From: Pierre Habouzit @ 2007-11-06  8:31 UTC (permalink / raw)
  To: Alex Riesen; +Cc: Bill Lear, Junio C Hamano, Aghiles, git
In-Reply-To: <20071106073841.GB3021@steel.home>

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

On Tue, Nov 06, 2007 at 07:38:41AM +0000, Alex Riesen wrote:
> Pierre Habouzit, Tue, Nov 06, 2007 01:46:01 +0100:
> > On Tue, Nov 06, 2007 at 12:36:16AM +0000, Bill Lear wrote:
> > > On Monday, November 5, 2007 at 15:33:31 (-0800) Junio C Hamano writes:
> > > > Stop thinking like "I need to integrate the changes from upstream
> > > > into my WIP to keep up to date."
> > > >
> > > > [...]
> > > >
> > > > Once you get used to that, you would not have "a dirty directory"
> > > > problem.
> > > 
> > > I respectfully beg to differ.  I think it is entirely reasonable, and
> > > not a sign of "centralized" mindset, to want to pull changes others
> > > have made into your dirty repository with a single command.
> > 
> >   I agree, I have such needs at work.  Here is how we (very informally)
> > work: people push things that they believe could help other (a new
> > helper function, a new module, a bug fix) in our master ASAP, but
> > develop big complex feature in their repository and merge into master
> > when it's ready.
> > 
> >   Very often we discuss some bugfix that is impeding people, or a
> > most-wanted-API. Someone does the work, commits, I often want to merge
> > master _directly_ into my current work-branch, because I want the
> > fix/new-API/... whatever.
> 
> How about merging just that "fix/new-API/... whatever" thing and not
> the whole master, which should be a complete mess by now?

  No master only holds simple patches (few of them, typically half a
dozen a day), or long-lived branches that are tested and ready to merge.

> The way you explained it it looks like typical centralized workflow.

  Well I disagree, it's /part/ centralized. We have a two speed devel
method, one that works the old-centralized way for quick fixes, and a
more decentralized approach for big changes. It's a rather nice and
useful middle ground for a company where all programmers are within
earshot.

-- 
·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 PATCH] Reduce the number of connects when fetching
From: Junio C Hamano @ 2007-11-06  8:34 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Daniel Barkalow, git
In-Reply-To: <47302218.3060409@viscovery.net>

Johannes Sixt <j.sixt@viscovery.net> writes:

> The tags that the server provides are those (and only those[*]) that
> reference objects in the packfile that it's going to send.
>
> [*] This way the client doesn't have to figure out which tags it
> wants; ...

Yes, but that shifts the burden to the sending side which is
always bad.  We want to make the client work as much as possible
when it is practical.

^ permalink raw reply

* Re: [ANNOUNCE] cgit v0.7
From: Lars Hjemli @ 2007-11-06  8:44 UTC (permalink / raw)
  To: Peter Karlsson; +Cc: git list
In-Reply-To: <Pine.LNX.4.64.0711060903070.8577@ds9.cixit.se>

On Nov 6, 2007 9:04 AM, Peter Karlsson <peter@softwolves.pp.se> wrote:
> Lars Hjemli:
>
> > cgit v0.7 (a fast webinterface for git) is now available at
> >
> >      git://hjemli.net/pub/git/cgit
>
> Has anyone managed to compile this for Win32?

If (win32 == cygwin): yes, just did (but it needed a makefile tweak
regarding iconv)

-- 
From: Lars Hjemli <hjemli@gmail.com>
Date: Tue, 6 Nov 2007 09:35:07 +0100
Subject: [PATCH] Makefile: link with libiconv if NEEDS_LIBICONV is defined

This seems to be needed to compile on cygwin.

Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
 Makefile |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 6abd82a..dbc34a2 100644
--- a/Makefile
+++ b/Makefile
@@ -19,6 +19,11 @@ OBJECTS = shared.o cache.o parsing.o html.o
ui-shared.o ui-repolist.o \
        ui-snapshot.o ui-blob.o ui-tag.o ui-refs.o


+ifdef NEEDS_LIBICONV
+       EXTLIBS += -liconv
+endif
+
+
 .PHONY: all git install clean distclean force-version get-git

 all: cgit git
-- 
1.5.3.4.452.g09149

^ permalink raw reply related

* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Pierre Habouzit @ 2007-11-06  8:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, Steven Grimm, git
In-Reply-To: <7vode8j7o5.fsf@gitster.siamese.dyndns.org>

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

On Tue, Nov 06, 2007 at 04:54:02AM +0000, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Mon, 5 Nov 2007, Junio C Hamano wrote:
> >
> >> Allowing people to revert or cherry pick partially by using paths 
> >> limiter is a very good idea; the whole "it comes from a commit so we 
> >> also commit" feels an utter nonsense, though.
> >
> > No.
> >
> > When "git revert <commit>" commits the result, "git revert <commit> -- 
> > <file>" should, too.
> 
> I was not questioning about that part.  "If 'git revert <some
> other form> foo' does not talk about commit, it should not
> commit" was what I was referring to.

  Well, I don't really know how closely you read #git, but I'd say that
"how do I undo my local changes in a git repository" is among the top 3
questions. There _IS_ an UI issue for that.

If git revert <commitish> -- path1 path2 path3 is going to work at some
point, I see no harm in saying that git revert HEAD -- path1 path2 path3
work. We can also in that case spit an error message:

error: this works as a courtesy but you really meant git checkout -- path/to/file

  On some other issues I'm all about educating people and learning to
them how to "think different". But here it's a pure interface problem,
and git is the sole $scm with a revert commands that doesn't reverts
local changes wrt HEAD.

  The next release of master will have tons of UI improvements (terse
output, better options parsing, more builtins hence faster commands …),
I believe it's stopping halfway not thinking about issues like this from
a newcomer point of view.

  On the pure theoretical basis I believe you're right, it's a bit
mixing apples and oranges. On the pragmatic usability side I'm quite
sure you're wrong, because everyone is used to that:

    $ hg revert --help | head -3 | tail -1
    revert files or dirs to their states as of some revision

    $ bzr help revert | head -1
    Purpose: Revert files to a previous revision.

    $ svn help revert | head -1
    revert: Restore pristine working copy file (undo most local edits).

    $ darcs help revert | head -3 | tail -1
    Revert to the recorded version (safe the first time only).

    <put your favorite non-git scm with a revert command here>

-- 
·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: git pull opinion
From: Pierre Habouzit @ 2007-11-06  8:51 UTC (permalink / raw)
  To: Benoit Sigoure; +Cc: Aghiles, git list
In-Reply-To: <176851C5-D735-4DDC-B799-A5106CD03989@lrde.epita.fr>

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

On Tue, Nov 06, 2007 at 05:29:58AM +0000, Benoit Sigoure wrote:
> On Nov 6, 2007, at 5:16 AM, Aghiles wrote:
> 
> >Hello,
> >
> >>who will run git stash clear? :)
> >
> >Yes you are right. By the way, in the context of merging into a
> >dirty tree, "git stash clear" seems to be a dangerous command:
> >there is a risk of loosing all your changes without a question
> >asked!
> >
> >I know unix is a harsh world but ...
> 
> Be *very* careful, because it's worse than that.  If you run, say, `git 
> stash clean', instead of `clear' (that's the sort of typo that quickly 
> slips through), then it will stash all your changes in a new stash named 
> "clean".  Once you realize you made a typo, you will most probably 
> correct it and run `git stash clear' but...   Oops, you just wiped your 
> changes that were in the "clean" stash.
> That happened to me and other people I know, so now I'm utterly cautious 
> when I start a command with "git stash".
> 
> As far as I remember, a patch was proposed to change this mis-behavior of 
> "git stash" (one could argue that it's a PEBCAK issue, but I really think 
> this command is *way* too dangerous) but I don't think it's been accepted 
> at this time.

  no it's a command issue. git stash <random non command name> should
_NOT_ be an alias to git stash save <random name>. Either the command
should be mandatory _or_ it should be a long option to avoid such kind
of conflicts.

  It's just a bad ui design.

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

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

^ permalink raw reply

* [PATCH amend] git-mailsplit: with maildirs not only process cur/, but also new/
From: Gerrit Pape @ 2007-11-06  8:54 UTC (permalink / raw)
  To: Alex Riesen
  Cc: Fernando J. Pereda, git, Junio C Hamano, Jakub Narebski,
	Jeff King, Alex Riesen
In-Reply-To: <20071105225258.GC4208@steel.home>

When saving patches to a maildir with e.g. mutt, the files are put into
the new/ subdirectory of the maildir, not cur/.  This makes git-am state
"Nothing to do.".  This patch lets git-mailsplit additional check new/
after reading cur/.

This was reported by Joey Hess through
 http://bugs.debian.org/447396

Signed-off-by: Gerrit Pape <pape@smarden.org>
---

On Mon, Nov 05, 2007 at 01:58:50PM +0100, Jakub Narebski wrote:
> > +        for (i = 0; i < 2; ++i) {
> Wouldn't it be better to use sizeof(sub)/sizeof(sub[0]) or it's macro
> equivalent ARRAY_SIZE(sub) instead of hardcoding 2 to avoid errors?
I made the array NULL-terminated.

On Mon, Nov 05, 2007 at 04:26:24PM -0500, Jeff King wrote:
> Isn't the subject line now wrong?
Yes, thanks.

On Mon, Nov 05, 2007 at 11:52:58PM +0100, Alex Riesen wrote:
> Why is missing "cur" (or "new", for that matter) a fatal error?
> Why is it error at all? How about just ignoring the fact?
As suggested by Jeff, I made it ignore the error on ENOENT.

 builtin-mailsplit.c |   38 ++++++++++++++++++++++----------------
 1 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index 74b0470..46b27cd 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -101,20 +101,29 @@ static int populate_maildir_list(struct path_list *list, const char *path)
 {
 	DIR *dir;
 	struct dirent *dent;
+	char name[PATH_MAX];
+	char *subs[] = { "cur", "new", NULL };
+	char **sub;
+
+	for (sub = subs; *sub; ++sub) {
+		snprintf(name, sizeof(name), "%s/%s", path, *sub);
+		if ((dir = opendir(name)) == NULL) {
+			if (errno == ENOENT)
+				continue;
+			error("cannot opendir %s (%s)", name, strerror(errno));
+			return -1;
+		}
 
-	if ((dir = opendir(path)) == NULL) {
-		error("cannot opendir %s (%s)", path, strerror(errno));
-		return -1;
-	}
+		while ((dent = readdir(dir)) != NULL) {
+			if (dent->d_name[0] == '.')
+				continue;
+			snprintf(name, sizeof(name), "%s/%s", *sub, dent->d_name);
+			path_list_insert(name, list);
+		}
 
-	while ((dent = readdir(dir)) != NULL) {
-		if (dent->d_name[0] == '.')
-			continue;
-		path_list_insert(dent->d_name, list);
+		closedir(dir);
 	}
 
-	closedir(dir);
-
 	return 0;
 }
 
@@ -122,19 +131,17 @@ static int split_maildir(const char *maildir, const char *dir,
 	int nr_prec, int skip)
 {
 	char file[PATH_MAX];
-	char curdir[PATH_MAX];
 	char name[PATH_MAX];
 	int ret = -1;
 	int i;
 	struct path_list list = {NULL, 0, 0, 1};
 
-	snprintf(curdir, sizeof(curdir), "%s/cur", maildir);
-	if (populate_maildir_list(&list, curdir) < 0)
+	if (populate_maildir_list(&list, maildir) < 0)
 		goto out;
 
 	for (i = 0; i < list.nr; i++) {
 		FILE *f;
-		snprintf(file, sizeof(file), "%s/%s", curdir, list.items[i].path);
+		snprintf(file, sizeof(file), "%s/%s", maildir, list.items[i].path);
 		f = fopen(file, "r");
 		if (!f) {
 			error("cannot open mail %s (%s)", file, strerror(errno));
@@ -152,10 +159,9 @@ static int split_maildir(const char *maildir, const char *dir,
 		fclose(f);
 	}
 
-	path_list_clear(&list, 1);
-
 	ret = skip;
 out:
+	path_list_clear(&list, 1);
 	return ret;
 }
 
-- 
1.5.3.5

^ permalink raw reply related

* Re: git pull opinion
From: Andreas Ericsson @ 2007-11-06  8:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Bill Lear, Junio C Hamano, Aghiles, git
In-Reply-To: <Pine.LNX.4.64.0711060115130.4362@racer.site>

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 6 Nov 2007, Andreas Ericsson wrote:
> 
>> Bill Lear wrote:
>>> On Monday, November 5, 2007 at 15:33:31 (-0800) Junio C Hamano writes:
>>>> Aghiles <aghilesk@gmail.com> writes:
>>>>
>>>>> Is there an "easier" way to pull into a dirty directory ? I am
>>>>> asking this to make sure I understand the problem and not
>>>>> because I find it annoying to type those 4 commands to perform
>>>>> a pull (although some of my colleagues do find that annoying :).
>>>> You need to switch your mindset from centralized SVN workflow.
>>>>
>>>> The beauty of distributedness is that it redefines the meaning
>>>> of "to commit".  In distributed systems, the act of committing
>>>> is purely checkpointing and it is not associated with publishing
>>>> the result to others as centralized systems force you to.
>>>>
>>>> Stop thinking like "I need to integrate the changes from
>>>> upstream into my WIP to keep up to date."  You first finish what
>>>> you are currently doing, at least to the point that it is
>>>> stable, make a commit to mark that state, and then start
>>>> thinking about what other people did.  You may most likely do a
>>>> "git fetch" followed by "git rebase" to update your WIP on top
>>>> of the updated work by others.
>>>>
>>>> Once you get used to that, you would not have "a dirty
>>>> directory" problem.
>>> I respectfully beg to differ.  I think it is entirely reasonable, and
>>> not a sign of "centralized" mindset, to want to pull changes others
>>> have made into your dirty repository with a single command.
>>>
>> I find it much more convenient to just fetch them. I'd rather see
>> git-pull being given a --rebase option (which would ultimately mean
>> teaching git-merge about it) to rebase already committed changes on
>> top of the newly fetched tracking branch. It's being worked on, but
>> rather slowly.
> 
> git-pull learning about --rebase does not mean teaching git-merge about 
> it.  See my patch, which you (and others) failed to enthusiastically 
> embrace, which is the sole reason it is stalled.
> 

I must have missed it. Found the thread now though. Gonna try the patch in
production for a while and see how it pans out.

I'm curious about this hunk though. It seems unaffiliated with the --rebase
option as such, but was still in the patch. Would you care to clarify?

@@ -86,7 +95,6 @@ merge_head=$(sed -e '/	not-for-merge	/d' \
 
 case "$merge_head" in
 '')
-	curr_branch=$(git symbolic-ref -q HEAD)
 	case $? in
 	  0) ;;
 	  1) echo >&2 "You are not currently on a branch; you must explicitly"

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

^ permalink raw reply

* Re: [PATCH 4/4] Implement git commit and status as a builtin commands.
From: Pierre Habouzit @ 2007-11-06  9:12 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Björn Steinbrink, Kristian Høgsberg, Junio C Hamano,
	git
In-Reply-To: <Pine.LNX.4.64.0711052317170.4362@racer.site>

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

On Mon, Nov 05, 2007 at 11:18:36PM +0000, Johannes Schindelin wrote:
> Hi,
> 
> On Mon, 5 Nov 2007, Bj?rn Steinbrink wrote:
> 
> > On 2007.11.05 13:57:53 -0500, Kristian H?gsberg wrote:
> >
> > > The shell script just has
> > > 
> > > case "$all,$interactive,$also,$#" in
> > > *t,*t,*)
> > >         die "Cannot use -a, --interactive or -i at the same time." ;;
> > > 
> > > which doesn't seem to care about the value of $also.  As far as I 
> > > understand git commit, it doesn't make sense to pass any of -a, -i, -o 
> > > or --interactive at the same time so I guess I could join the checks
> > 
> > Note that there are only two commas. The asterisks catch everything and
> > $# won't be "t", so that catches anything with at least two t's.
> 
> So shouldn't it be
> 
> 	if (!!all + !!interactive + !!also > 1)

Btw, I'm starting to work slowly on the diff_opt_parse conversion to the
macro we discussed, and the need for new option parsing callbacks
arised, and I've created a:

  parse_opt_mask_{or,and,xor} commands that you declare this way:

    OPT_MASK_OR('a', "all",         &mode, "...", MASK_ALL),
    OPT_MASK_OR('i', "interactive", &mode, "...", MASK_INTERACTIVE),
    ...

And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,

    if (!!all + !!interactive ... > 1)

becomes[0]:

    if (mode & (mode - 1)) {

    }

I've not read your patch thoroughly, but if you feel such a thing would
help you, I could send a preliminar set of patches to enable this
feature for people that may need it. Though you can look at my WIP on my
git public repository[1].

For those who care, this need arised because parse_diff_opts have a
_lot_ of single bit options, and that expansing it on many many full
blown integers looked like a regression, so I took the option to have a
`flags` member with explicit masks, and those masks will be used from
the parse-option callbacks[2]


  [0] for those who don't already know it, (i & (i - 1)) == 0
      iff i is 0 or a power of 2.

  [1] the patch[3]:
      http://git.madism.org/?p=git.git;a=commitdiff;h=9d75b0a00915fa81657934f36318c1c0f5bac96b
      example of use:
      http://git.madism.org/?p=git.git;a=commitdiff;h=74aacc487579d0cbd638adf883e743caeaee0f76
      http://git.madism.org/?p=git.git;a=commitdiff;h=af15793dde94119faa1577c9eec7e839ae628011

  [2] http://git.madism.org/?p=git.git;a=commitdiff;h=86032204f1bdf5da2fee555ec917709b3e6f662c#patch10

  [3] oh boy gitweb urls are really way too long :/
-- 
·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: [PATCH 4/4] Implement git commit and status as a builtin commands.
From: Johannes Sixt @ 2007-11-06  9:18 UTC (permalink / raw)
  To: Pierre Habouzit
  Cc: Johannes Schindelin, Björn Steinbrink,
	Kristian Høgsberg, Junio C Hamano, git
In-Reply-To: <20071106091222.GE4435@artemis.corp>

Pierre Habouzit schrieb:
> Btw, I'm starting to work slowly on the diff_opt_parse conversion to the
> macro we discussed, and the need for new option parsing callbacks
> arised, and I've created a:
> 
>   parse_opt_mask_{or,and,xor} commands that you declare this way:
> 
>     OPT_MASK_OR('a', "all",         &mode, "...", MASK_ALL),
>     OPT_MASK_OR('i', "interactive", &mode, "...", MASK_INTERACTIVE),
>     ...
> 
> And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,
> 
>     if (!!all + !!interactive ... > 1)
> 
> becomes[0]:
> 
>     if (mode & (mode - 1)) {
> 
>     }

This goes too far, IMHO. That's unnecessary cleverness/microoptimization at 
the expense of readability.

-- Hannes

^ permalink raw reply

* Re: [PATCH 4/4] Implement git commit and status as a builtin commands.
From: Pierre Habouzit @ 2007-11-06  9:26 UTC (permalink / raw)
  To: Johannes Sixt
  Cc: Johannes Schindelin, Björn Steinbrink,
	Kristian Høgsberg, Junio C Hamano, git
In-Reply-To: <4730314A.9010403@viscovery.net>

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

On Tue, Nov 06, 2007 at 09:18:02AM +0000, Johannes Sixt wrote:
> Pierre Habouzit schrieb:
> >Btw, I'm starting to work slowly on the diff_opt_parse conversion to the
> >macro we discussed, and the need for new option parsing callbacks
> >arised, and I've created a:
> >  parse_opt_mask_{or,and,xor} commands that you declare this way:
> >    OPT_MASK_OR('a', "all",         &mode, "...", MASK_ALL),
> >    OPT_MASK_OR('i', "interactive", &mode, "...", MASK_INTERACTIVE),
> >    ...
> >And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,
> >    if (!!all + !!interactive ... > 1)
> >becomes[0]:
> >    if (mode & (mode - 1)) {
> >    }
> 
> This goes too far, IMHO. That's unnecessary cleverness/microoptimization 
> at the expense of readability.

The reason why I did that is not to be able to do mode & (mode - 1).
Have a look at diff.c, imagine the insane amount of intermediate
variables we would need, and now understand why I introduced that :)

I'm not sure it's useful for git-commit, I was mentioning it just in case.

Note: the fact that an OPT_BOOLEAN when you repeat it increments the
      value isn't always a good thing, and is the reason why you need
      the (quite ugly) double bangs.

-- 
·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: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Mike Hommey @ 2007-11-06  9:29 UTC (permalink / raw)
  To: Pierre Habouzit, Junio C Hamano, Johannes Schindelin,
	Steven Grimm, git
In-Reply-To: <20071106084925.GC4435@artemis.corp>

On Tue, Nov 06, 2007 at 09:49:25AM +0100, Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Nov 06, 2007 at 04:54:02AM +0000, Junio C Hamano wrote:
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > 
> > > On Mon, 5 Nov 2007, Junio C Hamano wrote:
> > >
> > >> Allowing people to revert or cherry pick partially by using paths 
> > >> limiter is a very good idea; the whole "it comes from a commit so we 
> > >> also commit" feels an utter nonsense, though.
> > >
> > > No.
> > >
> > > When "git revert <commit>" commits the result, "git revert <commit> -- 
> > > <file>" should, too.
> > 
> > I was not questioning about that part.  "If 'git revert <some
> > other form> foo' does not talk about commit, it should not
> > commit" was what I was referring to.
> 
>   Well, I don't really know how closely you read #git, but I'd say that
> "how do I undo my local changes in a git repository" is among the top 3
> questions. There _IS_ an UI issue for that.
> 
> If git revert <commitish> -- path1 path2 path3 is going to work at some
> point, I see no harm in saying that git revert HEAD -- path1 path2 path3
> work. We can also in that case spit an error message:

It seems to me git revert HEAD -- path1 path2 path3 should revert the changes
made in the commit pointed to by HEAD, not revert the changes in the working
tree or the index...

Mike

^ permalink raw reply

* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Pierre Habouzit @ 2007-11-06  9:37 UTC (permalink / raw)
  To: Mike Hommey; +Cc: Junio C Hamano, Johannes Schindelin, Steven Grimm, git
In-Reply-To: <20071106092942.GB3197@glandium.org>

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

On Tue, Nov 06, 2007 at 09:29:42AM +0000, Mike Hommey wrote:
> On Tue, Nov 06, 2007 at 09:49:25AM +0100, Pierre Habouzit <madcoder@debian.org> wrote:
> > On Tue, Nov 06, 2007 at 04:54:02AM +0000, Junio C Hamano wrote:
> > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> > > 
> > > > On Mon, 5 Nov 2007, Junio C Hamano wrote:
> > > >
> > > >> Allowing people to revert or cherry pick partially by using paths 
> > > >> limiter is a very good idea; the whole "it comes from a commit so we 
> > > >> also commit" feels an utter nonsense, though.
> > > >
> > > > No.
> > > >
> > > > When "git revert <commit>" commits the result, "git revert <commit> -- 
> > > > <file>" should, too.
> > > 
> > > I was not questioning about that part.  "If 'git revert <some
> > > other form> foo' does not talk about commit, it should not
> > > commit" was what I was referring to.
> > 
> >   Well, I don't really know how closely you read #git, but I'd say that
> > "how do I undo my local changes in a git repository" is among the top 3
> > questions. There _IS_ an UI issue for that.
> > 
> > If git revert <commitish> -- path1 path2 path3 is going to work at some
> > point, I see no harm in saying that git revert HEAD -- path1 path2 path3
> > work. We can also in that case spit an error message:
> 
> It seems to me git revert HEAD -- path1 path2 path3 should revert the changes
> made in the commit pointed to by HEAD, not revert the changes in the working
> tree or the index...

  Yes, sorry, the `HEAD` in my sentence was spurious.

-- 
·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: [PATCH 4/4] Implement git commit and status as a builtin commands.
From: Andreas Ericsson @ 2007-11-06  9:47 UTC (permalink / raw)
  To: Pierre Habouzit, Johannes Sixt, Johannes Schindelin
In-Reply-To: <20071106092648.GG4435@artemis.corp>

Pierre Habouzit wrote:
> On Tue, Nov 06, 2007 at 09:18:02AM +0000, Johannes Sixt wrote:
>> Pierre Habouzit schrieb:
>>> Btw, I'm starting to work slowly on the diff_opt_parse conversion to the
>>> macro we discussed, and the need for new option parsing callbacks
>>> arised, and I've created a:
>>>  parse_opt_mask_{or,and,xor} commands that you declare this way:
>>>    OPT_MASK_OR('a', "all",         &mode, "...", MASK_ALL),
>>>    OPT_MASK_OR('i', "interactive", &mode, "...", MASK_INTERACTIVE),
>>>    ...
>>> And if you chose MASK_ALL/INTERACTIVE/.. to be single bits,
>>>    if (!!all + !!interactive ... > 1)
>>> becomes[0]:
>>>    if (mode & (mode - 1)) {
>>>    }
>> This goes too far, IMHO. That's unnecessary cleverness/microoptimization 
>> at the expense of readability.
> 
> The reason why I did that is not to be able to do mode & (mode - 1).

With a macro along the lines of

  #define IS_MULTI_FLAGGED(x) (x & (x - 1))

at least this particular construct becomes a lot more readable.

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

^ permalink raw reply

* Re: [Patch] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Francesco Pretto @ 2007-11-06 10:04 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8x5cmern.fsf@gitster.siamese.dyndns.org>

Junio C Hamano ha scritto:
>>  ------------------------------------------------
>> -$ git clone foo.com:/pub/repo.git/ my-project
>> +$ git clone foo.com:/pub/scm/repo.git/ my-project
>>  $ cd my-project
>>  ------------------------------------------------
> 
> This part seems an unnecessary change.
> 

Ironically, that's the same configuration of git.kernel.org. And I think is better
to put immediately the project in a appropriate directory than to move it later.

> Don't assume the "admin privilege" part, as you do not have to.
> 

Admin privilege SHALL be assumed, as this is a first time configuration, and the best
we can do is to assume it's done on a default *nix installation. Moreover, it's
what HEAD documentation is already doing when it suggests to give users ssh access.

Let's suppose the user "user1" create its own repository on a remote machine.
Now, he wants to selectively give write access on its repository to "user2".
There's 2 cases:
    1) "user2" have a local/ssh account on the machine. In this case, "user1" want
       to be sure only "user2" can write to the repository, so he can ask the admin
       to put "user1" and "user2" in the same group "projectx" or ask him to enable
       ACLs, still turned off in the majority of *nix systems.
    2) "user2" haven't a local/ssh account. Here:
	- a local/ssh account should be given to "user2", returning to 1)
        - mod_dav module has to be enabled for public http dirs of "user1"
        - git daemon has to be started and enabled to write on the repository.

The last 3 tasks all require admin privilege on default linux/bsd/macosx
installations. However, a little distinction can be made. I'll see.

>
> needs, and there is no reason members of projects A and B should
> be in the same group 'git'

I agree!

> while having members of project C in
> group 'hg' only because A and B happen to use git.

I agree!

> belong to 'src' group, or (2) make three groups, one for each
> project.
> 

It's exactly the point of:

+It's recommended, but not necessary, to create a specific group of commiters
+for every project/repository. With root credentials launch:
+
+------------------------------------------------
+$ groupadd $group
+------------------------------------------------

What you have understood here?

> Also with the "create new --shared repository for the project in
> a group's directory that has mode 2755" approach, I do not think
> there is any need to muck with umask either.
> 

umask requirement is referred to previous version of git. It's still referred as actual
in HEAD documentation. If it's ok for you, we could just cut away that reference.

Conclusion: i can try to amend my patch to be even more clearer. What i am saying
is that official documentation, commands manuals/syntaxes should be easy enough to the
first time user to set up git repositories without looking up the web for
"git tutorial"/"git installation"/"git umask 002", etc. (and consider that even an
expert sysadmin is a first time user, when he install and set up git the first time).
Or was better a "Documentation S**KS!" bug report?

^ permalink raw reply

* Re: [PATCH] Implement selectable group ownership in git-init
From: Wincent Colaiuta @ 2007-11-06 10:31 UTC (permalink / raw)
  To: Francesco Pretto; +Cc: Junio C Hamano, git
In-Reply-To: <472F98DE.2010406@gmail.com>

El 5/11/2007, a las 23:27, Francesco Pretto escribió:

> Wincent Colaiuta ha scritto:
>> I think this proposal adds unnecessary clutter to the codebase for
>> something that can easily be achieved (and *should*) using chown,  
>> chgrp,
>> or "sudo -u" etc.
>
> While still not convinced about that "unnecessary", i had to admit  
> the risk
> of breaking something with my addition was too high. What about a  
> compromise?
> I have improved the documentation about shared repositories. A patch  
> coming
> shortly.

Sounds like an excellent idea.

Cheers,
Wincent

^ 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