Git development
 help / color / mirror / Atom feed
* Re: git 1.6.1 on AIX 5.3
From: Perry Smith @ 2009-01-28 15:03 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Mike Ralphson, git
In-Reply-To: <20090128074204.GA31951@coredump.intra.peff.net>

On Jan 28, 2009, at 1:42 AM, Jeff King wrote:

> On Tue, Jan 27, 2009 at 11:37:21PM -0800, Junio C Hamano wrote:
>
>>>> Just to be sure we are on the same page.  My directory structure  
>>>> has a
>>>> top/src/git-1.6.1 and top/build/git.1.6.1.  The src/git-1.6.1 is  
>>>> the
>>>> tar ball.  The  build/git-1.6.1 starts out empty.  I cd into it and
>>>> then do: ../../src/git-1.6.1/configure <options>  After this
>>>> completes, you can do "make".
>>>
>>> I don't see how this would work without automake support, which  
>>> git does
>>> not have.
>>
>> ... nor want to have ;-).
>
> Heh. Yes, in case there was any confusion: I don't want my statement  
> in
> any way to be construed as a suggestion to support automake.
>
> I would not be opposed to it if it somehow enhanced some users'
> experience without bothering people who didn't want to touch it (like
> the way that autoconf support is implemented). But I don't see how  
> that
> would be possible.

Thats fine.  I didn't know if this was a "known" situation or not so I  
just mentioned it.

As I mentioned, most open source code supports this style of build.  I  
have no idea
what it would take to get it to work.

^ permalink raw reply

* Honoring a checked out gitattributes file
From: Kristian Amlie @ 2009-01-28 15:25 UTC (permalink / raw)
  To: git

Hi!

We currently use msysGit in our company test farm to checkout the latest
development branch and do autotests. However, we have one problem:
Certain files require UNIX line endings, even though this is a Windows
system. For this we use .gitattributes.

However, if the .gitattributes file is also checked in to the branch, it
will not always be honored. I browsed the code a bit, and it seems to
happen whenever there is an existing .gitattributes file, but the
checkout adds new files to it. These new files will not get the correct
line endings (although I'm not sure if it happens *every* time, it could
depend on the order they are checked out).

I think this should be fairly straightforward to fix, by ensuring that
if there is a file called .gitattributes in the index of the current
directory, check it out first, before all the other files that may be
affected by it. I can produce a patch and testcase for it, but I wanted
to hear the opinions of some developers in case there is an obvious flaw
in my solution.

What do you think?

--
Kristian Amlie

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Linus Torvalds @ 2009-01-28 15:45 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <7vvdrzvpwd.fsf@gitster.siamese.dyndns.org>



On Tue, 27 Jan 2009, Junio C Hamano wrote:
> 
>  - When digging deeper into the ancestry chain of a commit that is already
>    painted as UNINTERESTING, in order to paint its parents UNINTERESTING,
>    we barfed if parse_parent() for a parent commit object failed.  We can
>    ignore such a parent commit object.

Wouldn't it be better to still mark it UNINTERESTING too?

> @@ -480,7 +483,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
>  			struct commit *p = parent->item;
>  			parent = parent->next;
>  			if (parse_commit(p) < 0)
> -				return -1;
> +				continue;
>  			p->object.flags |= UNINTERESTING;
>  			if (p->parents)
>  				mark_parents_uninteresting(p);

IOW, move that

	p->object.flags |= UNINTERESTING;

to before parse_commit(). That's assuming 'parent' is never NULL, of 
course.

Side note: parse_commit() is still going to print out the error message 
if the object is missing ("Could not read %s"). I guess that's fine, but 
if you really want to make this a "not an error at all" condition...

		Linus

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Shawn O. Pearce @ 2009-01-28 16:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <7vd4e7x5ov.fsf@gitster.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> 
> I've been toying with an idea for an alternative solution, and need
> somebody competent to bounce it around with.

Heh, then we need to wait for Nico... :-)
 
> pack-objects ends up doing eventually
> 
>     rev-list --objects $send1 $send2 $send3 ... --not $have1 $have2 ...
> 
> which lists commits and associated objects reachable from $sendN,
> excluding the ones that are reachable from $haveN.
> 
> The tentative solution Björn Steinbrink and I came up with excludes
> missing commit from $haveN to avoid rev-list machinery to barf, but it
> violates the ref-object contract as I explained to Björn in my other
> message.

Oh, OK, I now _finally_ understand what you were trying to say
by the reachability thing.  I kept scratching my head trying to
understand you, and was going to say something stupid on list;
but waited because I just didn't get what the big deal was...

Its the crash in rev-list that you were worried about.

> Checking if each commit is reachable from any of the refs is quite
> expensive, and it would especially be so if it is done once per ".have"
> and real ref we receive from the other end.

Yup.
 
> An alternative is to realize that rev-list traversal already does
> something quite similar to what is needed to prove if these ".have"s are
> reachable from refs when listing the reachable objects.  This computation
> is what it needs to do anyway, so if we teach rev-list to ignore missing
> or broken chain while traversing negative refs, we do not have to incur
> any overhead over existing code.

EXACTLY.

JGit does this.

The functional equivilant of rev-list in JGit will by default
throw an exception if any object is missing when we try to walk it.
That includes things we've painted UNINTERESTING, as it is a sure
sign of repository corruption.

However; our equivilant of pack-objects can toggle what you are
calling "ignore-missing-negative" when it starts enumeration.
Any UNINTERESTING object which is missing or failed to parse is
simply tossed aside.  Yes, the pack may be larger than necessary
like in Peff's example of:

       Q-R
      /
  D--E
      \
       A-C

If the other side has C reachable, we are pushing R, and we have
C but are missing A, we'll "over push" D-E, but its still a clean
and valid push.  Its no worse than we were before the ".have" came
about, or if C hadn't been downloaded locally at all.  (Of course
your tell-me-more extension would help fix this over-push, but lets
not get off topic.)

IMHO, this corruption of A is harmless if C isn't reachable.

It isn't really local corruption unless C was reachable by a ref.
But we don't tend to see much corruption like that, and if it did
exist, it would show up during *other* operations that access a
larger set of local refs, such as "git gc".

> I have a mild suspicion that it may even be the right thing to ignore them
> unconditionally, and it might even match the intention of Linus's original
> code.  That would make many hunks in this patch much simpler.

I don't think its right to ignore broken UNINTERESTING chains all
of the time.  Today we would see fatal errors if I asked for

  git log R ^C

and A was missing, but R and C are both local refs.  I still want
to see that fatal error.  Its a local corruption that should be
raised quickly to the user.  In fact by A missing we'd compute the
wrong result and produce D-E too, which is wrong.

IMHO, the *only* time this missing uninteresting A is safe is
during send-pack, upload-pack, or bundle creation, where you are
bringing the other side up to R by transferring any amount of data
necessary to reach that goal.  Which is why JGit enables this.
(Though at the API level we do let the caller flag if they want
the error to be fatal instead, but AFAIK nobody sets it for "fatal".)

FWIW, Linus' most recent message on this thread about hoisting the
UNINTERESTING test up sooner makes sense too.
 
> The evidences behind this suspicion are found in a handful of places in
> revision.c.  mark_blob_uninteresting() does not complain if the caller
> fails to find the blob.  mark_tree_uninteresting() does not, either.
> mark_parents_uninteresting() does not, either, and it even has a comment
> that strongly suggests the original intention was not to care about
> missing UNINTERESTING objects.

That feels wrong to me... given the "git log R ^C" example I give above.
 
-- 
Shawn.

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Shawn O. Pearce @ 2009-01-28 16:16 UTC (permalink / raw)
  To: Jeff King
  Cc: Junio C Hamano, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <20090128081745.GA2172@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:
> On Wed, Jan 28, 2009 at 12:05:33AM -0800, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
> > >
> > >        C--D
> > >       /
> > >   A--B
> > >       \
> > >        E--F
> 
> Don't the other changes have similar parallel use cases? [2/2] also deals
> with tag lookup. Wouldn't you also expect, if you had a tag "T" pointing
> to "E" in the above scenario that "git rev-list T..D" would barf? I
> really think you don't want to ignore missing negations _ever_ unless
> the caller knows that such a miss is really only about optimization and
> not correctness.

Exactly what I just said in my other message.
 
> Side note:
> 
> As you described, we expect to reach this situation from a partial
> transfer. Which means that you don't actually have a _ref_ for "T" (or
> "F"). So it is unlikely to come up in normal use (you would have to
> manually specify the sha1 of a broken portion of the graph).

True, but in the send-pack case we are discussing the remote side
has specified the SHA-1 of broken portions of the graph to us,
and we've taken that into consideration.  So we have to fix that
assumption we've made.

> But what is more important is that your repository _is_ corrupted,

Depends.  If the SHA-1 came from the remote side during send-pack,
it doesn't matter that we have a broken chain along that path,
it may have been a dumb transport fetch that was interrupted.
Our local repository isn't corrupt, it just has some extra crap
laying around that hasn't gc'd yet.

If the SHA-1 came from the user, then it depends on the context
of why the user is giving it to us.  In pretty much every case,
yes, its a corruption and we should be aborting.  :-)

Actually, the only time where it *isn't* a corruption is when its
input to "git bundle create A.bdl ... -not $SOMEBADID" as that is
the exact same thing as coming from the other side via send-pack.

> I
> think we are losing an important method by which the user finds out. Git
> is usually very good at informing you of a problem in the repo early,
> and I think unconditionally ignoring missing objects would lose that.

Yup, I agree.  But as you and Junio have already pointed out, C Git
can miss some types of corruption because the revision machinary has
some gaps.  *sigh*

I'd really like to see those gaps closed.  But I don't have a good
enough handle on the code structure of the C Git revision machinary
to do that myself in a short period of time.  I know JGit's well...
but that's only because I wrote it.  ;-)

Its now on my wish list of things I wish I had time for in C Git.
But perhaps someone who is more familiar with the revision machinary
will get to it first.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin
From: Pieter de Bie @ 2009-01-28 16:18 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, gitster, sverre
In-Reply-To: <1233154990-19745-1-git-send-email-jaysoffian@gmail.com>


On Jan 28, 2009, at 3:03 PM, Jay Soffian wrote:

> When git am is called w/o arguments, w/o a patch on stdin and the  
> user hits
> ctrl-c, it leaves behind a partially populated $dotest directory.  
> After this
> commit, it emits usage when called w/o arguments and w/o a patch on  
> stdin.

FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.

I can probably retrain my finger to use 'git am -', but I'm not sure  
if that
works (after this patch)? At least it's not mentioned in the manpage.

- Pieter

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Nicolas Pitre @ 2009-01-28 16:38 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <20090128160900.GJ1321@spearce.org>

On Wed, 28 Jan 2009, Shawn O. Pearce wrote:

> Junio C Hamano <gitster@pobox.com> wrote:
> > 
> > I've been toying with an idea for an alternative solution, and need
> > somebody competent to bounce it around with.
> 
> Heh, then we need to wait for Nico... :-)

Hmmm... ehhh... what have I done?

/me tries to walk by innocently without being noticed  ;-)


Nicolas

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and  w/o patch on stdin
From: Jay Soffian @ 2009-01-28 16:40 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: git, gitster, sverre
In-Reply-To: <780A42F8-E27C-404A-945C-38C16378EF57@ai.rug.nl>

On Wed, Jan 28, 2009 at 11:18 AM, Pieter de Bie <pdebie@ai.rug.nl> wrote:
> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.
>
> I can probably retrain my finger to use 'git am -', but I'm not sure if that
> works (after this patch)? At least it's not mentioned in the manpage.

"git am -" doesn't work (before or after this patch), but "cat | git am" will.

j.

^ permalink raw reply

* Re: Honoring a checked out gitattributes file
From: Michael J Gruber @ 2009-01-28 16:44 UTC (permalink / raw)
  To: Kristian Amlie; +Cc: git
In-Reply-To: <498078F1.20807@trolltech.com>

Kristian Amlie venit, vidit, dixit 28.01.2009 16:25:
> Hi!
> 
> We currently use msysGit in our company test farm to checkout the latest
> development branch and do autotests. However, we have one problem:
> Certain files require UNIX line endings, even though this is a Windows
> system. For this we use .gitattributes.
> 
> However, if the .gitattributes file is also checked in to the branch, it
> will not always be honored. I browsed the code a bit, and it seems to
> happen whenever there is an existing .gitattributes file, but the
> checkout adds new files to it. These new files will not get the correct
> line endings (although I'm not sure if it happens *every* time, it could
> depend on the order they are checked out).
> 
> I think this should be fairly straightforward to fix, by ensuring that
> if there is a file called .gitattributes in the index of the current
> directory, check it out first, before all the other files that may be
> affected by it. I can produce a patch and testcase for it, but I wanted
> to hear the opinions of some developers in case there is an obvious flaw
> in my solution.
> 
> What do you think?

I think there's a general time ordering problem. Say you do the
following commits:

A: add a.txt
B: add a .gitattributes file covering *.txt (say with crlf or any filter)
C: add c.txt

Now, with an empty dir, you do either

1) checkout A, B, C sequentially
2) checkout C

The contents of the checkout will be different in cases 1) and 2):
1) a.txt is checked out out as is, c.txt according to the attributes
2) with current git: probably like 1), with your suggestion: both a.txt
and c.txt filtered according to the attributes.

If you add a file and .gitattributes covering it in the same commit
there is an ordering ambiguity which can be solved (patched away)
easily, but I think the difference between 1) and 2) is still
problematic, and would need to be dealt with.

The main problem seems to be that changing a file like gitattributes can
potentially change (by changing filters) the contents which should be
stored for a different file even if the checkout of that file doesn't
change.

Cheers,
Michael

^ permalink raw reply

* [PATCH] git-p4: avoid syncing duplicate changes
From: Pete Wyckoff @ 2009-01-28 16:45 UTC (permalink / raw)
  To: git

When a particular changeset affects multiple depot paths, it
will appear multiple times in the output of "p4 changes".
Filter out the duplicates to avoid the extra empty commits that
this would otherwise create.

Signed-off-by: Pete Wyckoff <pw@padd.com>
---
 contrib/fast-import/git-p4 |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
index a85a7b2..63c8eca 100755
--- a/contrib/fast-import/git-p4
+++ b/contrib/fast-import/git-p4
@@ -444,8 +444,9 @@ def p4ChangesForPaths(depotPaths, changeRange):
 
     changes = []
     for line in output:
-        changeNum = line.split(" ")[1]
-        changes.append(int(changeNum))
+	changeNum = int(line.split(" ")[1])
+	if changeNum not in changes:
+	    changes.append(changeNum)
 
     changes.sort()
     return changes
-- 
1.6.0.6

^ permalink raw reply related

* Vim's gitconfig filetype (and folding)
From: Ted Pavlic @ 2009-01-28 16:56 UTC (permalink / raw)
  To: git, vim_use

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

I don't know who is responsible for maintaining the git-related files 
distributed with Vim, but it would be nice if folding was enable for the 
gitconfig filetype.

I've attached a sample set of folding rules that works well for me... 
Right now I have the attached file in ~/.vim/ftplugin/gitconfig.vim.

I'll paste those rules here (but beware of word wrap):

:set 
foldexpr=getline(v:lnum)=~'^[;#]\\s*\\(ex\\\|vi\\\|vim\\):'?\">1\":((getline(v:lnum)=~\"^#\\\\=[\")?\">1\":\"1\")
:set fdm=expr

--Ted


-- 
Ted Pavlic <ted@tedpavlic.com>

   Please visit my ALS association page:
         http://web.alsa.org/goto/tedpavlic
   My family appreciates your support in the fight to defeat ALS.

--~--~---------~--~----~------------~-------~--~----~
You received this message from the "vim_use" maillist.
For more information, visit http://www.vim.org/maillist.php
-~----------~----~----~----~------~----~------~--~---


[-- Attachment #2: gitconfig.vim --]
[-- Type: text/plain, Size: 133 bytes --]

:set foldexpr=getline(v:lnum)=~'^[;#]\\s*\\(ex\\\|vi\\\|vim\\):'?\">1\":((getline(v:lnum)=~\"^#\\\\=[\")?\">1\":\"1\")
:set fdm=expr

^ permalink raw reply

* Re: Honoring a checked out gitattributes file
From: Kristian Amlie @ 2009-01-28 17:25 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: git
In-Reply-To: <49808B53.6040907@drmicha.warpmail.net>

Michael J Gruber wrote:
> I think there's a general time ordering problem. Say you do the
> following commits:
> 
> A: add a.txt
> B: add a .gitattributes file covering *.txt (say with crlf or any filter)
> C: add c.txt
> 
> Now, with an empty dir, you do either
> 
> 1) checkout A, B, C sequentially
> 2) checkout C
> 
> The contents of the checkout will be different in cases 1) and 2):
> 1) a.txt is checked out out as is, c.txt according to the attributes
> 2) with current git: probably like 1), with your suggestion: both a.txt
> and c.txt filtered according to the attributes.
> 
> If you add a file and .gitattributes covering it in the same commit
> there is an ordering ambiguity which can be solved (patched away)
> easily, but I think the difference between 1) and 2) is still
> problematic, and would need to be dealt with.

I agree.

> 
> The main problem seems to be that changing a file like gitattributes can
> potentially change (by changing filters) the contents which should be
> stored for a different file even if the checkout of that file doesn't
> change.

Yes, that is a problem. Ideally, the crlf attribute would be tied to the
file entry itself rather than a separate file (so changing the attribute
would mean a change to the file), but I guess we are stuck with what we
have.

I still think that case 2) is the most common, and fixing it has the
appealing property that if the repository line endings are broken for
some reason (because of case 1 or something else), then recloning or
checking out from scratch is guaranteed to bring the working tree into
the "correct" state.

Since fixing both cases is a pretty big task and fixing only case 2 is
small, I propose that we go ahead with that.

--
Kristian Amlie

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin
From: Jeff King @ 2009-01-28 17:51 UTC (permalink / raw)
  To: Pieter de Bie; +Cc: Jay Soffian, git, gitster, sverre
In-Reply-To: <780A42F8-E27C-404A-945C-38C16378EF57@ai.rug.nl>

On Wed, Jan 28, 2009 at 04:18:30PM +0000, Pieter de Bie wrote:

> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.

I have to admit, I usually am opposed to this sort of terminal DWIMmery
for exactly that reason. But I don't personally ever cut and paste into
git-am, so I was trying not to raise a fuss. ;)

-Peff

^ permalink raw reply

* Re: Honoring a checked out gitattributes file
From: Jeff King @ 2009-01-28 17:55 UTC (permalink / raw)
  To: Kristian Amlie; +Cc: git
In-Reply-To: <498078F1.20807@trolltech.com>

On Wed, Jan 28, 2009 at 04:25:37PM +0100, Kristian Amlie wrote:

> However, if the .gitattributes file is also checked in to the branch, it
> will not always be honored. I browsed the code a bit, and it seems to
> happen whenever there is an existing .gitattributes file, but the
> checkout adds new files to it. These new files will not get the correct
> line endings (although I'm not sure if it happens *every* time, it could
> depend on the order they are checked out).

This is a known limitation of gitattributes. There has been some
discussion in the past on how it should work, but I don't recall the
specifics; try searching the list archive. I think it is really just
waiting for somebody to step up and write some patches.

As a workaround, you might be able to do something like:

  branch=master
  git show $branch:.gitattributes >.git/info/attributes
  git checkout $branch

which is very hacky, but might work depending on your setup. Notably it
will overwrite any actual use you were making of .git/info/attributes,
and it will not respect any .gitattributes files in subdirectories.

-Peff

^ permalink raw reply

* Re: [PATCH v2 1/2] git-am: emit usage when called w/o arguments and  w/o patch on stdin
From: Junio C Hamano @ 2009-01-28 18:10 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, sverre
In-Reply-To: <76718490901280628y3761b41dhd2e544093e01e209@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> On Tue, Jan 27, 2009 at 11:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Jay Soffian <jaysoffian@gmail.com> writes:
>>
>>> +     test $# = 0 && test -t 0 && usage
>>
>> Sorry to be dense.  Why isn't your patch the above single liner?
>>
>
> "Also ensure that $dotest is cleaned up if user manages to interupt mailsplit
> while it is processing input, but not if mailsplit exits non-zero due to
> error."

My point was why "Also" needs to be in the same commit.  Aren't they
separate issues?

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Jeff King @ 2009-01-28 18:11 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <20090128160900.GJ1321@spearce.org>

On Wed, Jan 28, 2009 at 08:09:00AM -0800, Shawn O. Pearce wrote:

> I don't think its right to ignore broken UNINTERESTING chains all
> of the time.  Today we would see fatal errors if I asked for
> 
>   git log R ^C
> 
> and A was missing, but R and C are both local refs.  I still want
> to see that fatal error.  Its a local corruption that should be
> raised quickly to the user.  In fact by A missing we'd compute the
> wrong result and produce D-E too, which is wrong.

I think you wrote this before reading the other part of the thread where
we see that many of these checks are not in C git. But to be clear, even
without Junio's patches the exact case I mentioned is not currently
reported as an error (i.e., will produce incorrect results). I tested
with:

-- >8 --
commit() {
    echo $1 >$1 && git add $1 && git commit -m $1 && git tag $1
}

mkdir repo && cd repo && git init
commit A
commit B
commit C
commit D
git checkout -b other B
commit E
commit F

rm -f .git/objects/`git rev-parse E | sed 's,^..,&/,'`
git log F..D
-- 8< --

which shows A-B-C-D.

-Peff

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and  w/o patch on stdin
From: Junio C Hamano @ 2009-01-28 18:15 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Pieter de Bie, git, sverre
In-Reply-To: <76718490901280840g7d36aa96u3d0f98a709ac9fde@mail.gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> On Wed, Jan 28, 2009 at 11:18 AM, Pieter de Bie <pdebie@ai.rug.nl> wrote:
>> FWIW, I sometimes like to run 'git am', paste in a patch and hit ctrl-d.
>>
>> I can probably retrain my finger to use 'git am -', but I'm not sure if that
>> works (after this patch)? At least it's not mentioned in the manpage.
>
> "git am -" doesn't work (before or after this patch), but "cat | git am" will.

Another approach we've taken in other places to avoid the "Huh?" that
triggered this thread is to do something like:

        if there is no argument
        then
		if reading from tty
	        then
	        	echo >&2 "Reading from terminal, waiting for input..."
		fi
                process stdin	
	else
        	for arg
                do
                	process $arg
		done
	fi

Unfortunately, this will invalidate your "check -t 0 and error out" patch,
but some people may find it easier to work with and more friendly.

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Jeff King @ 2009-01-28 18:16 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Junio C Hamano, Linus Torvalds, PJ Hyett, Johannes Schindelin,
	git
In-Reply-To: <20090128161652.GK1321@spearce.org>

On Wed, Jan 28, 2009 at 08:16:53AM -0800, Shawn O. Pearce wrote:

> > But what is more important is that your repository _is_ corrupted,
> 
> Depends.  If the SHA-1 came from the remote side during send-pack,

Sorry, there is a critical typo in there. I meant to say "_if_ your
repository is corrupted" (and I have no idea how I ended up not only
omitting a word, but emphasizing the wrong one, so I can only suspect
the typo was in my brain and not my fingers).

So basically I agree with everything you said.

> Yup, I agree.  But as you and Junio have already pointed out, C Git
> can miss some types of corruption because the revision machinary has
> some gaps.  *sigh*
> 
> I'd really like to see those gaps closed.  But I don't have a good
> enough handle on the code structure of the C Git revision machinary
> to do that myself in a short period of time.  I know JGit's well...
> but that's only because I wrote it.  ;-)

I would like to see them closed, too. But keep in mind that this may
actually be a harder form of corruption to achieve than something like
just flipping some bits. Once the objects are in a packfile, you are
unlikely to lose a single or a small number of objects. But like with
any type of corruption, it is infrequent enough that it is hard to say
which is more common or "worse".

-Peff

^ permalink raw reply

* Re: (beginner) git rm
From: Zabre @ 2009-01-28 18:23 UTC (permalink / raw)
  To: git
In-Reply-To: <49804D41.3010801@dbservice.com>



Tomas Carnecky wrote:
> 
> Oops, sorry. git checkout HEAD -- d.txt
> You have to tell which version of d.txt you want. In your case the 
> version in HEAD.
> 

Thank you for this precision, it makes me understand this command better.
(Sorry for my late answer I've been unable to check my computer for a few
hours)
-- 
View this message in context: http://n2.nabble.com/%28beginner%29-git-rm-tp2231416p2233892.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: (beginner) git rm
From: Zabre @ 2009-01-28 18:25 UTC (permalink / raw)
  To: git
In-Reply-To: <20090128130333.GA11160@mit.edu>



Theodore Tso wrote:
> 
> I use this command enough that I have this defined as an alias in my
> ~/.gitconfig file.  Try running this command:
> 
> 	git config --global alias.revert-file "checkout HEAD --"
> 
> Now you will be able to do this:
> 
> 	git revert-file d.txt
> 
> This is also useful when I've edited d.txt, and decided that I didn't
> go about it the right away, and so I want to revert my edits.
> 

Very good, I'll try that. (Plus it shows me how to customize git, thanks!)
(Sorry for my late answer I've been unable to check my computer for a few
hours)
-- 
View this message in context: http://n2.nabble.com/%28beginner%29-git-rm-tp2231416p2233899.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and  w/o patch on stdin
From: Jay Soffian @ 2009-01-28 18:26 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pieter de Bie, git, sverre
In-Reply-To: <7v3af3thyj.fsf@gitster.siamese.dyndns.org>

On Wed, Jan 28, 2009 at 1:15 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Another approach we've taken in other places to avoid the "Huh?" that
> triggered this thread is to do something like:
>
>        if there is no argument
>        then
>                if reading from tty
>                then
>                        echo >&2 "Reading from terminal, waiting for input..."
>                fi
>                process stdin
>        else
>                for arg
>                do
>                        process $arg
>                done
>        fi
>
> Unfortunately, this will invalidate your "check -t 0 and error out" patch,
> but some people may find it easier to work with and more friendly.

Well perhaps we should just deal w/ctrl-c only and ignore the terminal
check altogether.

j.

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28 18:26 UTC (permalink / raw)
  To: Shawn O. Pearce
  Cc: Jeff King, Linus Torvalds, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <20090128161652.GK1321@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Actually, the only time where it *isn't* a corruption is when its
> input to "git bundle create A.bdl ... -not $SOMEBADID" as that is
> the exact same thing as coming from the other side via send-pack.

And notice that it is about a nagative ref.

Another case you may use an object ID that may or may not be good and it
is not a corruption is when a Porcelain has an object ID obtained from
somewhere, and wants to know if it is safe to use the object.  After
determining that the object itself exists (e.g. via "cat-file -t"),
you run

	rev-list --objects $THAT_UNKNOWN_ID --not --all

to see if it is reachable from some of your own refs, or at least it is
connected to them without gaps.  If it errors out while traversing, you
know it is bad; if it doesn't, you know you can merge one of the commits
reachable from your refs with it and put the result in your ref without
violating the ref-objects contract.

Notice that in this case, it is about a positive ref, and revision
machinery is set to notice the breakage.

So in that sense, the existing semantics is internally consistent.  The
rules (I am not making up a new rule here, but just spelling out) are:

 (1) You cannot just pick a random object that happens to exist in your
     repository, traverse to the objects it refers to and expect
     everything exists;

 (2) If an object is reachable from any of your refs, however, you can
     expect everything reachable from that object exists.  Otherwise you
     have a corrupt repository [*1*]).

 (3) Your object store may have garbage objects that are not reachable
     from any of your refs and it is normal.

 (4) You can use random objects that may not be well connected as negative
     revs to limit the range of revs (and optionally objects reachable
     from them) listed by object traversal.  If they are well connected,
     they will affect the outcome, but it is not an error if they are
     leftover cruft that is not connected to the positive ones you start
     your listing traversal at.


[Footnote]

*1* You don't have to bring up grafts and shallow.  People who know about
them know they are ways to hide or deliberately introduce this type of
corruption while keeping the system (mostly) working.

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and  w/o patch on stdin
From: Sverre Rabbelier @ 2009-01-28 18:33 UTC (permalink / raw)
  To: Jay Soffian; +Cc: Junio C Hamano, Pieter de Bie, git
In-Reply-To: <76718490901281026i45c26cb0r978f795a50dede19@mail.gmail.com>

On Wed, Jan 28, 2009 at 19:26, Jay Soffian <jaysoffian@gmail.com> wrote:
> Well perhaps we should just deal w/ctrl-c only and ignore the terminal
> check altogether.

That would suit my needs; as long as doing "git am" and then ^C does
not change my worktree and .git directory. Doing a stray "git am
--abort" after aborting does not obliterate my worktree as it does now
(not nice!), even so, it will change where my branch is currently at!

-- 
Cheers,

Sverre Rabbelier

^ permalink raw reply

* Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin
From: Junio C Hamano @ 2009-01-28 18:52 UTC (permalink / raw)
  To: Jay Soffian; +Cc: git, sverre
In-Reply-To: <1233154990-19745-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

> When git am is called w/o arguments, w/o a patch on stdin and the user hits
> ctrl-c, it leaves behind a partially populated $dotest directory. After this
> commit, it emits usage when called w/o arguments and w/o a patch on stdin.
>
> Noticed by Sverre Rabbelier
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> Change from v2: make Junio happy by no longer removing $dotest if git-am is
> interupted while mailsplit is running.

Sorry, I think I was misleading.  I did not mean that "Also" change was a
bad thing to do.  It just did not logically belong to this "is it coming
from the tty?" issue.

While you are looking at "am", I have a complaint about a very rough and
dangerous edge around a relatively new "usability" feature.

A scenario goes like this.

 (1) You have a patch, try to apply it to one existing topic branch

	$ git checkout myTopic
	$ git am some-patch

 (2) You see it does not apply, you realize that the patch is more suited
     for another branch.

	$ git checkout anotherTopic
        $ git am

 (3) It still does not apply, you examine the patch, and find flaws in its
     logic and decide not to apply it at all anywhere.

	$ git am --abort

After this, the tip of anotherTopic is updated to where the tip of myTopic
was!

Being able to remove the state directory $GIT_DIR/rebase-apply from the
command line is very useful when you want to get a clean slate to run
another git-am session, and often it is handy to abort the application of
the whole series when you do so (the latter is done by resetting the
original branch back to the original commit).  But being able to switch
branches after starting application of patches from one mailbox is also
useful in practice, so at least the latter should not be done blindly when
the user asks for --abort.  We definitely need some safety here.

Perhaps....

 (1) If --abort is asked for and you are on a branch different from where
     you started your "git am" session from, error out without doing
     anything.

 (2) Add --clean that does what the current --abort does, except for the
     rewinding of the HEAD.

The second one may be useful regardless of the "rough and dangerous edge"
issue above.  The early parts of the series may be useful and you may want
to keep the result from their application, while discarding the remainder
of the series.

Hmm?

^ permalink raw reply

* Re: Bad objects error since upgrading GitHub servers to 1.6.1
From: Junio C Hamano @ 2009-01-28 19:00 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Shawn O. Pearce, PJ Hyett, Johannes Schindelin, git
In-Reply-To: <alpine.LFD.2.00.0901280738430.3123@localhost.localdomain>

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Tue, 27 Jan 2009, Junio C Hamano wrote:
>> 
>>  - When digging deeper into the ancestry chain of a commit that is already
>>    painted as UNINTERESTING, in order to paint its parents UNINTERESTING,
>>    we barfed if parse_parent() for a parent commit object failed.  We can
>>    ignore such a parent commit object.
>
> Wouldn't it be better to still mark it UNINTERESTING too?
>
>> @@ -480,7 +483,7 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,
>>  			struct commit *p = parent->item;
>>  			parent = parent->next;
>>  			if (parse_commit(p) < 0)
>> -				return -1;
>> +				continue;
>>  			p->object.flags |= UNINTERESTING;
>>  			if (p->parents)
>>  				mark_parents_uninteresting(p);
>
> IOW, move that
>
> 	p->object.flags |= UNINTERESTING;
>
> to before parse_commit(). That's assuming 'parent' is never NULL, of 
> course.

Ok, makes sense.  Will do.

^ 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