* 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
* Re: [PATCH] Makefile: Make 'configure --with-expat=path' actually work
From: Junio C Hamano @ 2009-01-28 19:13 UTC (permalink / raw)
To: Serge van den Boom; +Cc: git
In-Reply-To: <alpine.BSF.2.00.0901251938120.97940@toad.stack.nl>
Serge van den Boom <svdb@stack.nl> writes:
> The prefix specified with the --with-expat option of configure was not
> actually used.
I see configure.ac already has support for autodetection but I realized it
only after running "git grep EXPATDIR". "Even though the configure script
knows how to autodetect presence of the expat library and set EXPATDIR to
the prefix of the location it was found, the Makefile ignored it and only
honoured NO_EXPAT" might have been a better way to describe the breakage
the patch fixes.
If you look at the Makefile, you will notice a sequence of comments like
this:
# Define NO_CURL if you do not have libcurl installed. git-http-pull and
# git-http-push are not built, and you cannot use http:// and https://
# transports.
#
# Define CURLDIR=/foo/bar if your curl header and library files are in
# /foo/bar/include and /foo/bar/lib directories.
#
Please add one for EXPATDIR, just after "Define NO_EXPAT if ...". People
who do not run ./configure but add their own customizations in config.mak
should benefit from your patch as well.
You might want to add a logic to drop NO_EXPAT when EXPATDIR is specified
to the Makefile as well, but I didn't check.
Please do *not* send a patch in 'text/plain; format="flowed"' content-type.
You will get a whitespace mangled patch that I have to fix up by hand.
Thanks.
> Signed-off-by: Serge van den Boom <svdb@stack.nl>
> ---
> diff --git a/Makefile b/Makefile
> index b4d9cb4..e7218cb 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -849,7 +849,12 @@ else
> endif
> endif
> ifndef NO_EXPAT
> - EXPAT_LIBEXPAT = -lexpat
> + ifdef EXPATDIR
> + BASIC_CFLAGS += -I$(EXPATDIR)/include
> + EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
> + else
> + EXPAT_LIBEXPAT = -lexpat
> + endif
> endif
> endif
^ permalink raw reply
* Re: [JGIT PATCH] Add getTaggerIdent, getShortMessage, getFullMessage to RevTag
From: Robin Rosenberg @ 2009-01-28 20:10 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git
In-Reply-To: <1233117316-3291-1-git-send-email-spearce@spearce.org>
onsdag 28 januari 2009 05:35:16 skrev Shawn O. Pearce:
> These methods make the RevTag API more like the RevCommit API, such
> that it is more consistent for applications to access the "fields"
> of a tag object in the same way that they access the fields of any
> commit object.
>
> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
> ---
>
> Actually, this has also gone through Gerrit2. If you want to see
> what the same patch looks like (just for fun):
The reviewer is biased :) Where are the unit tests?
I'm kind of surprised you did not optimized skipping past the object header,
like the parents are skipped when parsing commits.
-- robin
^ permalink raw reply
* Re: (beginner) git rm
From: Björn Steinbrink @ 2009-01-28 20:17 UTC (permalink / raw)
To: Zabre; +Cc: git
In-Reply-To: <1233166992184-2233892.post@n2.nabble.com>
On 2009.01.28 10:23:12 -0800, Zabre wrote:
> 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)
The "git checkout -- d.txt" is also a valid command, but that restores
the file from the index.
git checkout -- paths
==> Copy "paths" from the index to the working tree
git checkout <tree-ish> -- paths
==> Copy "paths" from the tree-ish to the index and working tree
So, for "rm d.txt", a plain "git checkout -- d.txt" would also do the
trick, as d.txt is still in the index. But your "git rm d.txt" also
removed the file from the index, and thus that checkout does nothing.
But "git checkout HEAD -- d.txt" works, as it gets the file from HEAD
and puts it into the index and working tree.
Björn
^ permalink raw reply
* ??? Re: [PATCH/RFC v1 1/6] symlinks.c: small cleanup and optimisation
From: Junio C Hamano @ 2009-01-28 20:36 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: git
In-Reply-To: <1233004637-15112-2-git-send-email-barvik@broadpark.no>
Kjetil Barvik <barvik@broadpark.no> writes:
> Simplify the if-else test in longest_match_lstat_cache() such that we
> only have one simple if test. Instead of testing for 'i == cache.len'
> or 'i == len', we transform this to a common test for 'i == max_len'.
>
> And to further optimise we use 'i >= max_len' instead of 'i ==
> max_len', the reason is that it is now the exact opposite of one part
> inside the while-loop termination expression 'i < max_len && name[i]
> == cache.path[i]', and then the compiler can hopefully/probably reuse
> a test-result from it.
This is *dense*, and made me wonder if is this really worth doing.
> + /*
> + * Is the cached path string a substring of 'name', is 'name'
> + * a substring of the cached path string, or is 'name' and the
> + * cached path string the exact same string?
> + */
> + if (i >= max_len && ((i < len && name[i] == '/') ||
> + (i < cache.len && cache.path[i] == '/') ||
> + (len == cache.len))) {
As you described in your commit log message, what this really wants to
check is (i == max_len). By saying (i >= max_len) you are losing
readability, optimizing for compilers (that do not notice this) and
pessimizing for human readers (like me, who had to spend a few dozens of
seconds to realize what you are doing, to speculate why you might have
thought this would be a good idea, and writing this paragraph). I do not
know if it is a good trade-off.
> @@ -91,7 +92,7 @@ static int lstat_cache(int len, const char *name,
> match_len = last_slash =
> longest_match_lstat_cache(len, name, &previous_slash);
> match_flags = cache.flags & track_flags & (FL_NOENT|FL_SYMLINK);
> - if (match_flags && match_len == cache.len)
> + if (match_flags && match_len >= cache.len)
> return match_flags;
> /*
> * If we now have match_len > 0, we would know that
Likewise.
> @@ -102,7 +103,7 @@ static int lstat_cache(int len, const char *name,
> * we can return immediately.
> */
> match_flags = track_flags & FL_DIR;
> - if (match_flags && len == match_len)
> + if (match_flags && match_len >= len)
> return match_flags;
> }
>
Likewise.
> @@ -184,7 +185,7 @@ void invalidate_lstat_cache(int len, const char *name)
> int match_len, previous_slash;
>
> match_len = longest_match_lstat_cache(len, name, &previous_slash);
> - if (len == match_len) {
> + if (match_len >= len) {
> if ((cache.track_flags & FL_DIR) && previous_slash > 0) {
> cache.path[previous_slash] = '\0';
> cache.len = previous_slash;
Likewise.
People who would want to fix potential bugs in this code 3 months down the
road may not have a ready access to your commit log message (they may be
looking at an extract from a release tarball, scratching their heads
wondering why you are not checking for equality). Perhaps the inline
function can have comments that says something like "when comparing the
return value from this function to see if it is equal to cache.len or len,
checking if the returned value is >= might help your compiler optimize it
better" to help the readers, but still, I do not know if it is a good
trade-off.
> @@ -153,7 +154,7 @@ static int lstat_cache(int len, const char *name,
> cache.path[last_slash] = '\0';
> cache.len = last_slash;
> cache.flags = save_flags;
> - } else if (track_flags & FL_DIR &&
> + } else if ((track_flags & FL_DIR) &&
> last_slash_dir > 0 && last_slash_dir <= PATH_MAX) {
> /*
> * We have a separate test for the directory case,
Good.
^ permalink raw reply
* Re: [PATCH 2/3] Make has_commit non-static
From: Junio C Hamano @ 2009-01-28 20:36 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jake Goulding, git
In-Reply-To: <alpine.DEB.1.00.0901261637300.25749@intel-tinevez-2-302>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 26 Jan 2009, Jake Goulding wrote:
>
>> Moving has_commit from branch to a common location in preparation for
>> using it in tag. Renaming it to commit_has_any_in_commit_list to be more
>> unique.
>
> I feel like bike-shedding for a change, and I'd also like to prove that
> not all Germans like long names:
>
> is_ancestor_of_any()
I'll queue this after munging it to "is_descendant_of()".
Thanks.
^ permalink raw reply
* Re: [PATCH/RFC v1 2/6] remove some memcpy() and strchr() calls inside create_directories()
From: Junio C Hamano @ 2009-01-28 20:36 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: git
In-Reply-To: <1233004637-15112-3-git-send-email-barvik@broadpark.no>
Kjetil Barvik <barvik@broadpark.no> writes:
> OK, maybe I instead should have tried to merge the function
> create_directories() with the safe_create_leading_directories() and
> *_const() functions? What do pepople think?
Strictly speaking, the safe_create_leading_* functions are meant to work
on paths inside $GIT_DIR and are not meant for paths inside the work tree,
which is this function is about. Their semantics are different with
respect to adjust_shared_perm().
Which means that you would either have two variants (one for work tree
paths, and another for paths inside $GIT_DIR), or a unified function that
has an argument to specify whether to run adjust_shared_perm().
HOWEVER.
That is only "strictly speaking".
A non-bare repository that is shared feels like an oximoron, but perhaps
there is a valid "shared work area" workflow that may benefit from such a
setup.
I see existing (mis)uses of the safe_create_leading_* in builtin-apply.c,
builtin-clone.c (the one that creates the work_tree, the other one is Ok),
merge-recursive.c (both call sites) that are used to touch the work tree,
but all places that create regular files in the work tree do not run
adjust_shared_perm() but simply honor the user's umask.
If we _were_ to support a "shared work area" workflow, having a unified
"create leading directory" function that always calls adjust_shared_perm()
may make sense (note that adjust_shared_perm() is a no-op in a non-shared
repository). We then need to also call adjust_shared_perm() for codepaths
that create regular files as well, though (e.g. write_entry() in entry.c,
but there are many others).
> diff --git a/entry.c b/entry.c
> index 05aa58d..c2404ea 100644
> --- a/entry.c
> +++ b/entry.c
> @@ -2,15 +2,19 @@
> #include "blob.h"
> #include "dir.h"
>
> -static void create_directories(const char *path, const struct checkout *state)
> +static void
> +create_directories(int path_len, const char *path, const struct checkout *state)
Please do not split the line like this.
The existing sources are not laid out to allow "grep ^funcname(", nor are
we going to reformat all the files to support such a use case.
When we pass <string, length> pair to functions, I think we pass them in
the order I just wrote in all the other functions.
The micro-optimization itself makes sense to me, though.
^ permalink raw reply
* Re: [PATCH/RFC v1 5/6] combine-diff.c: remove a call to fstat() inside show_patch_diff()
From: Junio C Hamano @ 2009-01-28 20:37 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: git
In-Reply-To: <1233004637-15112-6-git-send-email-barvik@broadpark.no>
Kjetil Barvik <barvik@broadpark.no> writes:
> Currently inside show_patch_diff() we have and fstat() call after an
> ok lstat() call. Since we before the call to fstat() have already
> test for the link case with S_ISLNK() the fstat() can be removed.
Good eyes. Thanks.
^ permalink raw reply
* Re: (beginner) git rm
From: Zabre @ 2009-01-28 20:42 UTC (permalink / raw)
To: git
In-Reply-To: <20090128201727.GD7503@atjola.homenet>
Björn Steinbrink wrote:
>
> The "git checkout -- d.txt" is also a valid command, but that restores
> the file from the index.
>
> git checkout -- paths
> ==> Copy "paths" from the index to the working tree
>
> git checkout <tree-ish> -- paths
> ==> Copy "paths" from the tree-ish to the index and working tree
>
> So, for "rm d.txt", a plain "git checkout -- d.txt" would also do the
> trick, as d.txt is still in the index. But your "git rm d.txt" also
> removed the file from the index, and thus that checkout does nothing.
> But "git checkout HEAD -- d.txt" works, as it gets the file from HEAD
> and puts it into the index and working tree.
>
This is enlightening, thank you very much!
(I knew I would love git more and more)
Oh just one (probably stupid) thing : <tree-ish> does represent a directory
being the root of a tree of folders (which has been added to the index),
does it?
This is the way I understand it at the moment. It must be a convention I
don't know just yet. (I need to investigate on this)
--
View this message in context: http://n2.nabble.com/%28beginner%29-git-rm-tp2231416p2234796.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* [PATCH v2] Makefile: Make 'configure --with-expat=path' actually work
From: Serge van den Boom @ 2009-01-28 20:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vocxrqm57.fsf@gitster.siamese.dyndns.org>
While the configure script sets the EXPATDIR environment variable to
whatever value was passed to its option --with-expat as the prefix of
the location of the expat library and headers, the Makefile ignored it.
This patch fixes this bug.
Signed-off-by: Serge van den Boom <svdb@stack.nl>
---
On Wed, 28 Jan 2009, Junio C Hamano wrote:
> Serge van den Boom <svdb@stack.nl> writes:
> > The prefix specified with the --with-expat option of configure was not
> > actually used.
>
> I see configure.ac already has support for autodetection but I realized it
> only after running "git grep EXPATDIR". "Even though the configure script
> knows how to autodetect presence of the expat library and set EXPATDIR to
> the prefix of the location it was found, the Makefile ignored it and only
> honoured NO_EXPAT" might have been a better way to describe the breakage
> the patch fixes.
That's not entirely right, unless I'm missing something. The configure script
does not try to detect expat itself, though it passes on the argument
to --with-expat to the Makefile, via the EXPATDIR environment variable.
> If you look at the Makefile, you will notice a sequence of comments like
> this:
>
> # Define NO_CURL if you do not have libcurl installed. git-http-pull and
> # git-http-push are not built, and you cannot use http:// and https://
> # transports.
> #
> # Define CURLDIR=/foo/bar if your curl header and library files are in
> # /foo/bar/include and /foo/bar/lib directories.
> #
>
> Please add one for EXPATDIR, just after "Define NO_EXPAT if ...". People
> who do not run ./configure but add their own customizations in config.mak
> should benefit from your patch as well.
Ok.
diff --git a/Makefile b/Makefile
index 9d451cf..a7310f2 100644
--- a/Makefile
+++ b/Makefile
@@ -23,6 +23,9 @@ all::
# Define NO_EXPAT if you do not have expat installed. git-http-push is
# not built, and you cannot push using http:// and https:// transports.
#
+# Define EXPATDIR=/foo/bar if your expat header and library files are in
+# /foo/bar/include and /foo/bar/lib directories.
+#
# Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
#
# Define NO_D_TYPE_IN_DIRENT if your platform defines DT_UNKNOWN but lacks
@@ -850,7 +853,12 @@ else
endif
endif
ifndef NO_EXPAT
- EXPAT_LIBEXPAT = -lexpat
+ ifdef EXPATDIR
+ BASIC_CFLAGS += -I$(EXPATDIR)/include
+ EXPAT_LIBEXPAT = -L$(EXPATDIR)/$(lib) $(CC_LD_DYNPATH)$(EXPATDIR)/$(lib) -lexpat
+ else
+ EXPAT_LIBEXPAT = -lexpat
+ endif
endif
endif
^ permalink raw reply related
* Re: [PATCHv2 0/6] gitweb: feed metadata enhancements
From: Junio C Hamano @ 2009-01-28 20:58 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski
In-Reply-To: <1232970616-21167-1-git-send-email-giuseppe.bilotta@gmail.com>
This is independent from your other topic on PATH_INFO, right? I am aware
that other one is being polished between you and Jakub, but is anobody
doing anything on this one, or is it already polished enough?
^ permalink raw reply
* Re: (beginner) git rm
From: Zabre @ 2009-01-28 21:05 UTC (permalink / raw)
To: git
In-Reply-To: <1233175322729-2234796.post@n2.nabble.com>
Zabre wrote:
>
> Oh just one (probably stupid) thing : <tree-ish> does represent a
> directory being the root of a tree of folders (which has been added to the
> index), does it?
> This is the way I understand it at the moment. It must be a convention I
> don't know just yet. (I need to investigate on this)
>
Ok I found in the doc (in the "The Object Database" section) what trees are
about, seems a little bit more obscure, so I guess I'm not ready for this
right now. (I'm a beginner trying to use git for a simple regular workflow,
managing code and other documents)
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#the-object-database
I guess <tree-ish> refers to such an object, and you use it in order to
revert it from the Object Database.
I'll keep this idea in mind for when I'll try to restore data from tricky
last-chance "stores", there is a section about such things in the doc :
http://www.kernel.org/pub/software/scm/git/docs/user-manual.html#fixing-mistakes
I think that in a normal everyday use, the small index modification I can do
using the previous advices can do the job (or am I wrong?)
--
View this message in context: http://n2.nabble.com/%28beginner%29-git-rm-tp2231416p2234918.html
Sent from the git mailing list archive at Nabble.com.
^ permalink raw reply
* Re: [PATCH] Add --ff-only flag to git-merge
From: Junio C Hamano @ 2009-01-28 21:12 UTC (permalink / raw)
To: Yuval Kogman; +Cc: git
In-Reply-To: <1233147238-30082-1-git-send-email-nothingmuch@woobling.org>
Yuval Kogman <nothingmuch@woobling.org> writes:
> This patch adds an --ff-only flag to git-merge. When specified git-merge
> will exit with an error whenver a merge will not be a simple fast
> forward, similar to the default behavior of git push.
> ---
Lacks a sign-off.
> @@ -167,6 +168,8 @@ static struct option builtin_merge_options[] = {
> "perform a commit if the merge succeeds (default)"),
> OPT_BOOLEAN(0, "ff", &allow_fast_forward,
> "allow fast forward (default)"),
> + OPT_BOOLEAN(0, "ff-only", &only_fast_forward,
> + "allow only fast forward"),
> OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
> "merge strategy to use", option_parse_strategy),
> OPT_CALLBACK('m', "message", &merge_msg, "message",
After parse_options() returns, there are a few existing tests to reject
nonsensical combination of options. I think there are some combinations
you would want to reject (for example, "--no-ff --ff-only" is an obvious
such nonsensical combination, but there may be others, such as giving a
strategy explicitly).
> @@ -1012,6 +1015,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
> finish(o->sha1, msg.buf);
> drop_save();
> return 0;
> + } else if ( only_fast_forward ) {
> + printf("Merge is non fast forward, aborting.\n");
> + return 1;
> } else if (!remoteheads->next && common->next)
> ;
This is wrong for at least three reasons:
* Style of "if (expression) {", somebody else already pointed out.
* Only this case exits with 1, while others die() that exits with 128.
* The placement of this misses the case where a merge of two unrelated
histories is attempted. You would need to also have a check at "No
common ancestors found. We need a real merge." part. The octopus
codepath could also end up with a fast forward or up-to-date.
^ permalink raw reply
* Re: (beginner) git rm
From: Junio C Hamano @ 2009-01-28 21:29 UTC (permalink / raw)
To: Zabre; +Cc: git
In-Reply-To: <1233175322729-2234796.post@n2.nabble.com>
Zabre <427@free.fr> writes:
> Björn Steinbrink wrote:
>>
>> The "git checkout -- d.txt" is also a valid command, but that restores
>> the file from the index.
>>
>> git checkout -- paths
>> ==> Copy "paths" from the index to the working tree
>>
>> git checkout <tree-ish> -- paths
>> ==> Copy "paths" from the tree-ish to the index and working tree
>>
>> So, for "rm d.txt", a plain "git checkout -- d.txt" would also do the
>> trick, as d.txt is still in the index. But your "git rm d.txt" also
>> removed the file from the index, and thus that checkout does nothing.
>> But "git checkout HEAD -- d.txt" works, as it gets the file from HEAD
>> and puts it into the index and working tree.
>
> This is enlightening, thank you very much!
> (I knew I would love git more and more)
>
> Oh just one (probably stupid) thing : <tree-ish> does represent a directory
> being the root of a tree of folders (which has been added to the index),
> does it?
Yeah, it typically is a commit object.
Björn said "Copy", but the operation really is like checking out a book
from a library and "checkout" is a good word for it. "I do not like what
I have in my work tree, and I'd like to replace it with a fresh one taken
out of the index (or, out of that commit)".
^ 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