* Re: [PATCH] gc: use parse_options
From: Brandon Casey @ 2007-11-05 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: James Bowes, git
In-Reply-To: <7vhck579pm.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> James Bowes <jbowes@dangerouslyinc.com> writes:
>
>> + struct option builtin_gc_options[] = {
>> + OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),
>
> I would write "unreferenced loose" instead of "unused" here...
It is not just "loose" objects here, but also unreferenced objects in packs,
since the "-a" option to repack is now only used when --prune is specified.
Without --prune, "-A" is supplied to repack instead.
So maybe the message should just be "prune unreferenced objects"
-brandon
^ permalink raw reply
* [REPLACEMENT PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-05 23:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Andreas Ericsson, David Symonds
Signed-off-by: David Symonds <dsymonds@gmail.com>
---
Documentation/git-format-patch.txt | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 92c0ab6..9d4bae2 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -9,9 +9,10 @@ git-format-patch - Prepare patches for e-mail submission
SYNOPSIS
--------
[verse]
-'git-format-patch' [-n | -N | -k] [-o <dir> | --stdout] [--thread]
+'git-format-patch' [-k] [-o <dir> | --stdout] [--thread]
[--attach[=<boundary>] | --inline[=<boundary>]]
[-s | --signoff] [<common diff options>]
+ [-n | --numbered | -N | --no-numbered]
[--start-number <n>] [--numbered-files]
[--in-reply-to=Message-Id] [--suffix=.<sfx>]
[--ignore-if-in-upstream]
--
1.5.3.1
^ permalink raw reply related
* Re: [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-05 22:56 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <472F9DC6.8020607@op5.se>
On 11/6/07, Andreas Ericsson <ae@op5.se> wrote:
> David Symonds wrote:
> > On 11/5/07, David Symonds <dsymonds@gmail.com> wrote:
> >> [-s | --signoff] [<common diff options>]
> >> - [--start-number <n>] [--numbered-files]
> >> + [-n | --numbered-files | -N | --no-numbered]
> >> + [--start-number <n>]
> >
> > Now that I look at it again, it seems the long options look quite
> > inconsistent. I think it should be either
> > --numbered-files/--no-numbered-files or --numbered/--no-numbered. My
> > preference is with the latter (for brevity), but that breaks
> > backward-compatibility.
> >
> > Would you accept a patch that changed --numbered-files to --numbered,
> > and kept the former as a synonym?
> >
>
> I thought files were always numbered, but the [PATCH m/n] wasn't. Have I
> missed something?
>
> If your --numbered-files is supposed to affect only file-numbering, I'd
> suggest *not* using --numbered, as it's ambiguous with "number-subject".
You're right. There's both --numbered ([PATCH n/m] stuff) and
--numbered-files (0001.patch instead of 0001-subject-line.patch). I'll
revise my patch to clarify this.
Dave.
^ permalink raw reply
* Re: [bug in next ?] git-fetch/git-push issue
From: Jeff King @ 2007-11-05 22:55 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Pierre Habouzit, Nicolas Pitre, Git ML
In-Reply-To: <Pine.LNX.4.64.0711051620230.7357@iabervon.org>
On Mon, Nov 05, 2007 at 04:41:37PM -0500, Daniel Barkalow wrote:
> > Nope, that's not the problem. We _only_ update any tracking refs at all
> > if ret == 0, and if we fail to push, then we are setting ret to -2.
>
> That's an odd combination of behavior: we update some of the refs, leave
> the ones that didn't work alone, report success on the ones that worked,
> but then we forget that some things worked?
I think the current behavior is more odd. We mark some errors, and then
if there were any possible pushes, we replace the marked errors with the
status of the actual push, forgetting about the previous errors. Thus
the behavior where if 'next' needs pushing, then we don't mark any errors
at all (even though we spewed an error to stderr), but if it doesn't,
then we return an error.
I don't mind being conservative with updating tracking refs; they really
are just an optimization to avoid an extra git-fetch. But the most
sensible behavior would be to mark errors for _each_ ref individually,
try to push or update tracking branches where appropriate, and then
return an error status based on all refs (whether they had an error in
prep time or at push time).
Which I guess is what you were trying to accomplish by removing the
peer_ref, though I think that doesn't distinguish between "didn't match
a remote ref" and "had an error." Perhaps we just need an error flag in
the ref struct?
> If we're going to refuse to update local tracking refs, whose state
> doesn't matter much, we should certainly refuse to update the remote refs,
> which are probably public and extremely important. If we just pushed and
I would also be fine with that: if your intended push has _any_
problems, then abort the push.
> we fetch, we should see exclusively changes that somebody else (including
> hooks remotely) did, not anything that we ourselves did.
I don't necessarily agree, just because the notion of "we" and "somebody
else" is sort of pointless in a distributed system. I consider local
tracking ref updates to be a "best guess" attempt to optimize and avoid
a fetch (but one that will always be overwritten by the _actual_
contents when you do fetch).
> I'd guess -2 is supposed to indicate that there were some errors but some
> things may have worked. If pack_objects() or receive_status() fails, we
In that case, I think the simple fix is:
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 947c42b..f773dc8 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -338,7 +338,7 @@ static int do_send_pack(int in, int out, struct remote *remote, int nr_refspec,
packet_flush(out);
if (new_refs && !args.dry_run)
- ret = pack_objects(out, remote_refs);
+ ret |= pack_objects(out, remote_refs);
close(out);
if (expect_status_report) {
and then we accept that we don't know _which_ refs shouldn't have their
tracking branches updated (and we don't update any). But at least we
don't forget that the error occured.
And the better solution is an error flag (or bitfield) in struct ref.
-Peff
^ permalink raw reply related
* Re: [PATCH] git-mailsplit: with maildirs try to process new/ if cur/ is empty
From: Alex Riesen @ 2007-11-05 22:52 UTC (permalink / raw)
To: Gerrit Pape; +Cc: Fernando J. Pereda, git, Junio C Hamano
In-Reply-To: <20071105124920.17726.qmail@746e9cce42b49f.315fe32.mid.smarden.org>
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?
^ permalink raw reply
* Re: git pull opinion
From: Alex Riesen @ 2007-11-05 22:49 UTC (permalink / raw)
To: Aghiles; +Cc: git
In-Reply-To: <3abd05a90711051352t2f6be00bsa862585abd370fb1@mail.gmail.com>
Aghiles, Mon, Nov 05, 2007 22:52:12 +0100:
> ps; if someone is interested to hear what is the general opinion
> on switching to git from svn in our company, I could elaborate.
Yes, please. And how did you manage to convince them to switch, if possible:
there are still some suckers here trying to do the same to their colleagues.
^ permalink raw reply
* Re: [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: Andreas Ericsson @ 2007-11-05 22:48 UTC (permalink / raw)
To: David Symonds; +Cc: Junio C Hamano, git
In-Reply-To: <ee77f5c20711051432w284cf22dx71192c145d25dced@mail.gmail.com>
David Symonds wrote:
> On 11/5/07, David Symonds <dsymonds@gmail.com> wrote:
>> [-s | --signoff] [<common diff options>]
>> - [--start-number <n>] [--numbered-files]
>> + [-n | --numbered-files | -N | --no-numbered]
>> + [--start-number <n>]
>
> Now that I look at it again, it seems the long options look quite
> inconsistent. I think it should be either
> --numbered-files/--no-numbered-files or --numbered/--no-numbered. My
> preference is with the latter (for brevity), but that breaks
> backward-compatibility.
>
> Would you accept a patch that changed --numbered-files to --numbered,
> and kept the former as a synonym?
>
I thought files were always numbered, but the [PATCH m/n] wasn't. Have I
missed something?
If your --numbered-files is supposed to affect only file-numbering, I'd
suggest *not* using --numbered, as it's ambiguous with "number-subject".
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* [Patch] Documentation: enhanced "git for CVS users" doc about shared repositories
From: Francesco Pretto @ 2007-11-05 22:32 UTC (permalink / raw)
To: git
More detailed instructions on how to set up shared repositories.
Added a reference to "git for CVS users" doc in git-init manual.
Signed-off-by: Francesco Pretto <ceztkoml@gmail.com>
---
Documentation/cvs-migration.txt | 72 ++++++++++++++++++++++++++++++--------
Documentation/git-init.txt | 7 ++++
2 files changed, 64 insertions(+), 15 deletions(-)
diff --git a/Documentation/cvs-migration.txt b/Documentation/cvs-migration.txt
index 3b6b494..c92ed49 100644
--- a/Documentation/cvs-migration.txt
+++ b/Documentation/cvs-migration.txt
@@ -13,12 +13,12 @@ link:tutorial.html[tutorial introduction to git] should be sufficient.
Developing against a shared repository
--------------------------------------
-Suppose a shared repository is set up in /pub/repo.git on the host
+Suppose a shared repository is set up in /pub/scm/repo.git on the host
foo.com. Then as an individual committer you can clone the shared
repository over ssh with:
------------------------------------------------
-$ git clone foo.com:/pub/repo.git/ my-project
+$ git clone foo.com:/pub/scm/repo.git/ my-project
$ cd my-project
------------------------------------------------
@@ -68,37 +68,79 @@ other than `master`.
Setting Up a Shared Repository
------------------------------
-We assume you have already created a git repository for your project,
-possibly created from scratch or from a tarball (see the
-link:tutorial.html[tutorial]), or imported from an already existing CVS
-repository (see the next section).
+We assume you have admin privilege on the remote machine. Moreover, we assume
+you have already created a git repository for your project, possibly created
+from scratch or from a tarball (see the link:tutorial.html[tutorial]),or
+imported from an already existing CVS repository (see the next section).
-Assume your existing repo is at /home/alice/myproject. Create a new "bare"
-repository (a repository without a working tree) and fetch your project into
-it:
+First, let's create a common directory for all the projects you'll want to
+track with git:
+
+-----------------------------------------------
+$ mkdir -p /pub/scm
+-----------------------------------------------
+
+It's recommended, but not necessary, to create a specific group of commiters
+for every project/repository. With root credentials launch:
+
+------------------------------------------------
+$ groupadd $group
+------------------------------------------------
+
+Assume your existing repository is at /home/alice/myproject. Create a new
+"bare" repository (a repository without a working tree) and fetch your project
+into it:
------------------------------------------------
-$ mkdir /pub/my-repo.git
+$ mkdir /pub/scm/my-repo.git
$ cd /pub/my-repo.git
$ git --bare init --shared
$ git --bare fetch /home/alice/myproject master:master
------------------------------------------------
+Now, set the group ownership of the git repository you've just created to the
+same group of the commiters:
+
+------------------------------------------------
+$ chgrp -R $group /pub/scm/my-repo.git
+------------------------------------------------
+
Next, give every team member read/write access to this repository. One
easy way to do this is to give all the team members ssh access to the
machine where the repository is hosted. If you don't want to give them a
full shell on the machine, there is a restricted shell which only allows
users to do git pushes and pulls; see gitlink:git-shell[1].
-Put all the committers in the same group, and make the repository
-writable by that group:
+First, enable it putting on the trusted shells list of the system:
+
+------------------------------------------------
+$ echo `which git-shell` >> /etc/shells
+------------------------------------------------
+
+Ensure users will not have write permission on /pub/scm. Now, let's create
+them with the following command launched with root credentials:
------------------------------------------------
-$ chgrp -R $group /pub/my-repo.git
+$ useradd -g $group -d /pub/scm -s `which git-shell` $username
------------------------------------------------
-Make sure committers have a umask of at most 027, so that the directories
-they create are writable and searchable by other group members.
+They will be enabled to push on repositories owned by the group $group.
+Later, you can give users access to other projects simply by adding them to
+other groups.
+
+[NOTE]
+================================
+With previous versions of git, it could be necessary to set umask variable of
+all commiters with values like 002 or 007. If you still need to support them,
+you can do it in the following way; assuming that all users have their home
+positionated at /pub/scm, like in the previous example, launch the command:
+
+------------------------------------------------
+$ echo "umask 022" >> /pub/scm/.profile
+------------------------------------------------
+
+At the next login, users will have their umask variable automatically set.
+================================
Importing a CVS archive
-----------------------
diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 07484a4..f5f363d 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -101,6 +101,13 @@ $ git-add . <2>
<2> add all existing file to the index
+SHARED REPOSITORIES
+-------------------
+
+Please refer to link:cvs-migration.html[git for CVS users], section "Setting Up
+a Shared Repository", for details on how to set up shared repositories.
+
+
Author
------
Written by Linus Torvalds <torvalds@osdl.org>
^ permalink raw reply related
* Re: [PATCH] Rearrange git-format-patch synopsis to improve clarity.
From: David Symonds @ 2007-11-05 22:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, David Symonds
In-Reply-To: <119421522591-git-send-email-dsymonds@gmail.com>
On 11/5/07, David Symonds <dsymonds@gmail.com> wrote:
> [-s | --signoff] [<common diff options>]
> - [--start-number <n>] [--numbered-files]
> + [-n | --numbered-files | -N | --no-numbered]
> + [--start-number <n>]
Now that I look at it again, it seems the long options look quite
inconsistent. I think it should be either
--numbered-files/--no-numbered-files or --numbered/--no-numbered. My
preference is with the latter (for brevity), but that breaks
backward-compatibility.
Would you accept a patch that changed --numbered-files to --numbered,
and kept the former as a synonym?
Dave.
^ permalink raw reply
* Re: git pull opinion
From: Jakub Narebski @ 2007-11-05 22:28 UTC (permalink / raw)
To: git
In-Reply-To: <3abd05a90711051352t2f6be00bsa862585abd370fb1@mail.gmail.com>
Aghiles wrote:
> I am not sure this is the best place to write about this. Anyway,
> we just switched a couple of repositories to git (from svn) here
> at work and one thing people find annoying is a pull into
> a dirty directory. Before the "stash" feature it was even worse
> but now we can type:
>
> git stash
> git pull
> git stash apply
>
> But isn't that something we should be able to specify to the "pull"
> command ?
If I remember correctly there is/was some preliminary work (at most 'pu'
stages) about adding --dirty option to git-merge, git-pull and git-rebase.
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
^ permalink raw reply
* Re: [PATCH] Implement selectable group ownership in git-init
From: Francesco Pretto @ 2007-11-05 22:27 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Junio C Hamano, git
In-Reply-To: <8EF5148D-C1F0-4329-A221-82D0B7E9932C@wincent.com>
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.
Francesco
^ permalink raw reply
* Re: [PATCH] t3502: Disambiguate between file and rev by adding --
From: Alex Riesen @ 2007-11-05 22:25 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: git, Junio C Hamano
In-Reply-To: <20071104153126.GA61398@173.242.249.10.in-addr.arpa>
Brian Gernhardt, Sun, Nov 04, 2007 16:31:26 +0100:
> This test failed because git-diff didn't know if it was asking for the
> file "a" or the branch "a". Adding "--" at the end of the ambiguous
> commands allows the test to finish properly.
To be precise: this is ambiguous only on case-challenged filesystems
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Junio C Hamano @ 2007-11-05 22:21 UTC (permalink / raw)
To: Steven Grimm; +Cc: Pierre Habouzit, git
In-Reply-To: <CD2E6759-9E7E-41E6-8B58-AB6CA9604111@midwinter.com>
Steven Grimm <koreth@midwinter.com> writes:
> But that suggested command is not going to convince anyone they were
> wrong about git being hard to learn. I wonder if instead of saying, "I
> know what you meant, but I'm going to make you type a different
> command," we should make git revert just do what the user meant.
>
> There is already precedent for that kind of mixed-mode UI:
>
> git checkout my-branch
> vs.
> git checkout my/source/file.c
That's an example of mixed-mode UI, but what you are suggesting
is quite different, isn't it?
There is no other officially supported single-command-way to
checkout paths out of the index. "git checkout paths..." does
not introduce a confusion because of that. The user learns the
way git supports that concept and that's the end of the story.
The same thing can be said about "git checkout <commit>
paths...". That's _the_ way to checkout paths out of an
arbitrary commit.
In the case being discussed, we already have the concept of
checking out paths from the index, which has an officially
supported way to express.
You are proposing to give it a synonym "git revert paths...",
which superfitially sounds friendlier. But I actually think
allowing a mistaken
git revert path...
to be burned to users' fingers and brains is doing the user a
great disservice.
The next person would say "Why doesn't 'git revert HEAD path...'
work?", and you would add the synonym to do 'git checkout HEAD
path...'. Up to that point it is sort-of Ok (but not quite).
You already have "git checkout" that let's you do so, but you
introduced new concepts that are "revert paths to the index" and
"revert paths to the last commit".
Which may make you feel good, but you just introduced a narrower
synonym the user needs to learn, than a more established and
wider concept that we already have: "checkout paths out of X",
where X are either the index or an arbitrary commit.
The reason I think the narrower synonym is bad and will lead to
more user confusion is because after that point you will have
a few issues.
Another newcomer would say "I like the fact that 'git revert
HEAD path...' works but why doesn't 'git revert HEAD~12 path...'
work?".
- You may further allow "git revert <arbitrary-commit>
path...". But what does that _mean_? "revert the path to
the twelfth commit"? You may implement that _anyway_.
Then, the user would say "eh, why do you have both 'git
checkout path...' and 'git revert path...' that seem to do
the same thing? There's no difference? Why Why Why, git is
so hard to learn".
- You may instead not to do so, and explain that the "arbitrary
commit" form is not supported and tell the user to use "git
checkout <commit> paths...".
The user will say: "but you earlier told me to use revert --
you could have taught me to use checkout from the beginning
and saved me from great confusion instead".
Giving the same concept two different names is bad unless there
is a compelling reason to do so. Labelling an initially
narrower subset of an existing concept with a different name,
and having to extended that 'new concept' ending up with the
same as the existing concept is even worse.
^ permalink raw reply
* Re: [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options
From: Pierre Habouzit @ 2007-11-05 22:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vd4uonz30.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1189 bytes --]
On Mon, Nov 05, 2007 at 09:48:19PM +0000, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > After kicking this around a bit more on IRC, we had another idea. Instead
> > of introducing OPT_RECURSE(), do something like OPT__QUIET(), only this
> > time in diff.h:
> >
> > #define OPT__DIFF(opt) \
> > OPT_BOOLEAN('p', NULL, &opt.format_patch, "show a patch"), \
> > ...
> >
> > Pierre said this feels a bit "80s", so I'd like to hear other people's
> > opinions.
> >
> > Hmm?
>
> As I am from "80s" ;-)
Hey grand'pa, how are your rheumatisms today :-P
> I like the simpler "macro" one much better. There aren't many things
> that can go wrong in the approach.
Okay, I suppose we will end up using the big macros then. Though I had a
look at diff_opt_parse and setup_revisions, the move isn't for tomorrow.
The current code is heavy (at best), and has some logic that parseopt
is not yet ready to deal with (mainly the --not flags).
--
·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] Make git-clean a builtin
From: Carlos Rica @ 2007-11-05 22:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn Bohrer, git
In-Reply-To: <7vejf4pf7r.fsf@gitster.siamese.dyndns.org>
2007/11/5, Junio C Hamano <gitster@pobox.com>:
> Shawn Bohrer <shawn.bohrer@gmail.com> writes:
>
> > +static int show_only = 0;
> > +static int remove_directories = 0;
> > +static int quiet = 0;
> > +static int ignored = 0;
> > +static int ignored_only = 0;
>
> Please do not explicitly initialize static variables to zero.
Is it really needed to declare those variables outside of a function
in this case? This scheme makes difficult reusing the code from other
builtins, rewriting it for libification, calling it many times, or
even understand if they were declared that way with a purpose or not.
I just don't know why they are that way in this case, is there a
reason for it?
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: David Kastrup @ 2007-11-05 22:06 UTC (permalink / raw)
To: Steven Grimm; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <20071105215433.GA12827@inspiron>
Alejandro Martinez Ruiz <alex@flawedcode.org> writes:
> On Mon 05 Nov 2007, 11:28, Steven Grimm wrote:
>>
>> But that suggested command is not going to convince anyone they were wrong
>> about git being hard to learn. I wonder if instead of saying, "I know what
>> you meant, but I'm going to make you type a different command," we should
>> make git revert just do what the user meant.
>
> I think that would just add to confusion. "revert" applies to full
> changesets, not single files, plus it creates a new commit, which is
> probably not what the user wants. Most of them just want to revert some
> local changes to some random files, so teach them what they need, if
> anything.
>
>> There is already precedent for that kind of mixed-mode UI:
>>
>> git checkout my-branch
>> vs.
>> git checkout my/source/file.c
>
> This is a different case: you're basically performing the same
> operation, with the second line applying just to a subset of files.
Huh? The first one moves HEAD. The second one doesn't.
--
David Kastrup, Kriemhildstr. 15, 44793 Bochum
^ permalink raw reply
* Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.
From: Alejandro Martinez Ruiz @ 2007-11-05 21:54 UTC (permalink / raw)
To: Steven Grimm; +Cc: Pierre Habouzit, Junio C Hamano, git
In-Reply-To: <CD2E6759-9E7E-41E6-8B58-AB6CA9604111@midwinter.com>
On Mon 05 Nov 2007, 11:28, Steven Grimm wrote:
>
> But that suggested command is not going to convince anyone they were wrong
> about git being hard to learn. I wonder if instead of saying, "I know what
> you meant, but I'm going to make you type a different command," we should
> make git revert just do what the user meant.
I think that would just add to confusion. "revert" applies to full
changesets, not single files, plus it creates a new commit, which is
probably not what the user wants. Most of them just want to revert some
local changes to some random files, so teach them what they need, if
anything.
> There is already precedent for that kind of mixed-mode UI:
>
> git checkout my-branch
> vs.
> git checkout my/source/file.c
This is a different case: you're basically performing the same
operation, with the second line applying just to a subset of files.
- Alex
^ permalink raw reply
* git pull opinion
From: Aghiles @ 2007-11-05 21:52 UTC (permalink / raw)
To: git
Hello,
I am not sure this is the best place to write about this. Anyway,
we just switched a couple of repositories to git (from svn) here
at work and one thing people find annoying is a pull into
a dirty directory. Before the "stash" feature it was even worse
but now we can type:
git stash
git pull
git stash apply
But isn't that something we should be able to specify to the "pull"
command ? Additionally and if I am not mistakn, those commands will
create "dangling" commits and blobs. So one has to execute:
git prune
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 :).
For now, I am recommanding to my colleagues to commit very often
(even unfinished changes), pull, and then rebase the commits into
a more meaningful commit before pushing. Which seems to be a good
practice anyway,
Thank you for git,
- Aghiles.
ps; if someone is interested to hear what is the general opinion
on switching to git from svn in our company, I could elaborate.
^ permalink raw reply
* Re: [PATCH] Use parseopts in builtin-push
From: Junio C Hamano @ 2007-11-05 21:50 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0711042235350.7357@iabervon.org>
Thanks; will take this one.
The finalized version for builtin-fetch is appreciated.
^ permalink raw reply
* Re: [PATCH 2/3] parseopt: introduce OPT_RECURSE to specify shared options
From: Junio C Hamano @ 2007-11-05 21:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Pierre Habouzit
In-Reply-To: <Pine.LNX.4.64.0711051340490.4362@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> After kicking this around a bit more on IRC, we had another idea. Instead
> of introducing OPT_RECURSE(), do something like OPT__QUIET(), only this
> time in diff.h:
>
> #define OPT__DIFF(opt) \
> OPT_BOOLEAN('p', NULL, &opt.format_patch, "show a patch"), \
> ...
>
> Pierre said this feels a bit "80s", so I'd like to hear other people's
> opinions.
>
> Hmm?
As I am from "80s" ;-) I like the simpler "macro" one much
better. There aren't many things that can go wrong in the
approach.
^ permalink raw reply
* Re: [bug in next ?] git-fetch/git-push issue
From: Daniel Barkalow @ 2007-11-05 21:41 UTC (permalink / raw)
To: Jeff King; +Cc: Pierre Habouzit, Nicolas Pitre, Git ML
In-Reply-To: <20071105210711.GA9176@sigill.intra.peff.net>
On Mon, 5 Nov 2007, Jeff King wrote:
> On Mon, Nov 05, 2007 at 01:17:14PM -0500, Daniel Barkalow wrote:
>
> > I think this is the bit that's wrong. I blame Jeff, in 334f4831. :)
> >
> > The issue is that, in the previous version, we'd hit a continue on the
> > not-an-ancestor message and not reach the update_tracking_ref() section
> > for that ref. In 334f4831, all of the updating is after the loop, and it
> > doesn't filter out the refs that didn't actually get pushed.
>
> Nope, that's not the problem. We _only_ update any tracking refs at all
> if ret == 0, and if we fail to push, then we are setting ret to -2.
That's an odd combination of behavior: we update some of the refs, leave
the ones that didn't work alone, report success on the ones that worked,
but then we forget that some things worked?
If we're going to refuse to update local tracking refs, whose state
doesn't matter much, we should certainly refuse to update the remote refs,
which are probably public and extremely important. If we just pushed and
we fetch, we should see exclusively changes that somebody else (including
hooks remotely) did, not anything that we ourselves did.
> Hrm. Oh wait, it looks like we then totally write over the current value
> of 'ret' when we do pack_objects. Oops.
>
> I'm unclear how to fix this, as I'm not really sure what ret is
> _supposed_ to be communicating. What does the '-2' mean, as compared to
> a '-4'? Should we be doing a 'ret += pack_objects(out, remote_refs)' or
> some other bit-masking magic?
I'd guess -2 is supposed to indicate that there were some errors but some
things may have worked. If pack_objects() or receive_status() fails, we
shouldn't update anything locally (because it won't have been accepted
remotely); otherwise, we should make local updates of every remote efect
we had, even if we end up returning non-zero to indicate that we didn't do
everything asked.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* [PATCH] t5530-upload-pack-error: Check more carefully for failures.
From: Johannes Sixt @ 2007-11-05 21:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <7v3avkqwyz.fsf@gitster.siamese.dyndns.org>
Previously, the test only triggered a failure in upload-pack's rev-list
subprocess. Here we also test for a failure of pack-objects, and make sure
that the right process failed.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
On Monday 05 November 2007 21:05, Junio C Hamano wrote:
> > The test case checks for failures in rev-list (a missing
> > object). Any hints how to trigger a failure in pack-objects
> > that does not also trigger in rev-list would be welcome.
>
> How about removing a blob from the test repository to corrupt
> it? rev-list --objects I think would happily list the blob
> because it sees its name in its containing tree without checking
> its existence.
That does it. This goes on top of my previous patch.
-- Hannes
t/t5530-upload-pack-error.sh | 27 +++++++++++++++++++++++----
1 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh
index 9923ba0..70d4f86 100755
--- a/t/t5530-upload-pack-error.sh
+++ b/t/t5530-upload-pack-error.sh
@@ -15,7 +15,7 @@ test_expect_success 'setup and corrupt repository' '
test_tick &&
echo changed >file &&
git commit -a -m changed &&
- rm -f .git/objects/f7/3f3093ff865c514c6c51f867e35f693487d0d3
+ rm -f .git/objects/5e/a2ed416fbd4a4cbe227b75fe255dd7fa6bd4d6
'
@@ -24,14 +24,33 @@ test_expect_failure 'fsck fails' '
git fsck
'
-test_expect_failure 'upload pack fails due to error in rev-list' '
+test_expect_success 'upload-pack fails due to error in pack-objects' '
- echo "0032want $(git rev-parse HEAD)
+ ! echo "0032want $(git rev-parse HEAD)
00000009done
-0000" | git-upload-pack . > /dev/null
+0000" | git-upload-pack . > /dev/null 2> output.err &&
+ grep "pack-objects died" output.err
+'
+
+test_expect_success 'corrupt repo differently' '
+
+ git hash-object -w file &&
+ rm -f .git/objects/be/c63e37d08c454ad3a60cde90b70f3f7d077852
'
+test_expect_failure 'fsck fails' '
+
+ git fsck
+'
+test_expect_success 'upload-pack fails due to error in rev-list' '
+
+ ! echo "0032want $(git rev-parse HEAD)
+00000009done
+0000" | git-upload-pack . > /dev/null 2> output.err &&
+ grep "waitpid (async) failed" output.err
+'
+
test_expect_success 'create empty repository' '
mkdir foo &&
--
1.5.3.4.315.g2ce38
^ permalink raw reply related
* Re: [PATCH] git-commit.sh: Fix usage checks regarding paths given when they do not make sense
From: Junio C Hamano @ 2007-11-05 21:39 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: paolo.bonzini, krh, git
In-Reply-To: <1194291393-1067-1-git-send-email-B.Steinbrink@gmx.de>
Björn Steinbrink <B.Steinbrink@gmx.de> writes:
> The checks that looked for paths given to git-commit in addition to
> --all or --interactive expected only 3 values, while the case statement
> actually provides 4, so the check was never triggered.
>
> The bug was introduced in 6cbf07efc5702351897dee4742525c9b9f7828ac when
> the case statement was extended to handle --interactive.
Gaah, and thanks.
We really should have "negative" tests to catch this kind of
breakage in our testsuite. People when inventing new features
are eager to write tests to show off the new stuff works, but
not many people are careful enough to add tests to demonstrate
the commands properly catch user errors such as borked command
line options.
^ permalink raw reply
* Re: [PATCH 3/2] Enhance --early-output format
From: Linus Torvalds @ 2007-11-05 21:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Paul Mackerras, Marco Costalba, Git Mailing List
In-Reply-To: <alpine.LFD.0.999.0711051313350.15101@woody.linux-foundation.org>
On Mon, 5 Nov 2007, Linus Torvalds wrote:
>
> Here's a possible cleanup patch. It's on top of the enhanced
> --early-output format commit, and in fact fixes a stupid bug in that
> commit ("return -1" vs "return NULL"), but that bug-fix is really an
> independent thing.
.. and this extends a bit further on the notion.
It basically means that "rev->dense" can now be ignored outside of
revision.c, because we'll just set TREECHANGE automatically when
seeing a non-merge regular commit when --sparse is being used.
So it's not just a simplification, it's a performance optimization too!
Although since nobody sane would ever use --sparse, I guess nobody really
cares.
Linus
---
builtin-log.c | 8 ++------
revision.c | 9 +++++++++
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 76c84e2..d6845bc 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -88,13 +88,9 @@ static int estimate_commit_count(struct rev_info *rev, struct commit_list *list)
while (list) {
struct commit *commit = list->item;
unsigned int flags = commit->object.flags;
-
list = list->next;
- if (flags & UNINTERESTING)
- continue;
- if (!(flags & TREECHANGE) && rev->dense && single_parent(commit))
- continue;
- n++;
+ if ((flags & TREECHANGE) && !(flags & UNINTERESTING))
+ n++;
}
return n;
}
diff --git a/revision.c b/revision.c
index 7a1ecba..02e9241 100644
--- a/revision.c
+++ b/revision.c
@@ -325,6 +325,15 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
return;
}
+ /*
+ * Normal non-merge commit? If we don't want to make the
+ * history dense, we consider it always to be a change..
+ */
+ if (!revs->dense && !commit->parents->next) {
+ commit->object.flags |= TREECHANGE;
+ return;
+ }
+
pp = &commit->parents;
while ((parent = *pp) != NULL) {
struct commit *p = parent->item;
^ permalink raw reply related
* Re: [PATCH] errors: "strict subset" -> "ancestor"
From: Jeff King @ 2007-11-05 21:28 UTC (permalink / raw)
To: Wincent Colaiuta
Cc: David Symonds, Steffen Prohaska, J. Bruce Fields, Junio C Hamano,
git
In-Reply-To: <E7B0CBAA-BB97-40A0-8CFB-A6F01A047D17@wincent.com>
On Mon, Nov 05, 2007 at 02:06:35PM +0100, Wincent Colaiuta wrote:
> Kind of: it aligns "error:" with "local":
>
> error: remote 'refs/heads/master' is not an ancestor of
> local 'refs/heads/master'.
FYI, in the thread 'more terse push output', there is discussion of
eliminating this message entirely. So I wanted to point readers of this
thread in that direction.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox