* Re: [PATCH] convert filter: supply path to external driver
From: Pete Wyckoff @ 2010-12-20 14:41 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Junio C Hamano
In-Reply-To: <4D0F0DFB.20502@viscovery.net>
j.sixt@viscovery.net wrote on Mon, 20 Dec 2010 09:04 +0100:
> Am 12/19/2010 22:29, schrieb Pete Wyckoff:
> > Filtering to support keyword expansion may need the name of
> > the file being filtered. In particular, to support p4 keywords
> > like
> >
> > $File: //depot/product/dir/script.sh $
> >
> > the smudge filter needs to know the name of the file it is
> > smudging.
> >
> > Add a "%s" conversion specifier to the gitattribute for filter.
> > It will be expanded with the path name to the file when invoking
> > the external filter command.
>
> What happens if there are any shell special characters in the path name
> (or spaces, for that matter). Does this shell-escape the substituted path
> name anywhere in the call chain?
Good catch---it doesn't. I'll see if running everything through
sq_quote_buf will help.
Incidentally there appears to be no way to quote spaces in
filenames listed in .gitattributes, although fnmatch wildcards
can be used to work around that.
-- Pete
^ permalink raw reply
* push from git gui goes to wrong branch
From: dexen deVries @ 2010-12-20 14:09 UTC (permalink / raw)
To: git
Hello,
there's a behavior in git gui (v1.7.3.2 and previous) that I consider bugous.
Failed to find any bugreporting system on git's website so I'm reporting it
here; please correct me if that's wrong.
I have the following snippet in my .git/config:
[branch "article"]
remote = baron
merge = refs/heads/dexen-article
However, the `Push' button from git gui sends local branch `article' to remote
branch `article', rather than to the indicated `dexen-article'.
For reference, with the CLI `git push', the local branch `article' is pushed
to remote branch `dexen-article' as expected.
Regards,
--
dexen deVries
``You want to make your way in the CS field? Simple. Calculate rough time of
amnesia (hell, 10 years is plenty, probably 10 months is plenty), go to the
dusty archives, dig out something fun, and go for it. It's worked for many
people, and it can work for you.'' - Ron Minnich
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: decorate a bit more remotes
From: Drew Northup @ 2010-12-20 12:04 UTC (permalink / raw)
To: Jakub Narebski; +Cc: Sylvain Rabot, git
In-Reply-To: <m3tyjglxi0.fsf@localhost.localdomain>
On Thu, 2010-12-16 at 14:23 -0800, Jakub Narebski wrote:
> Sylvain Rabot <sylvain@abstraction.fr> writes:
>
> > Signed-off-by: Sylvain Rabot <sylvain@abstraction.fr>
> > ---
> > gitweb/gitweb.perl | 8 ++++----
> > 1 files changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index db18d06..9398475 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -5127,13 +5127,13 @@ sub git_remote_block {
> >
> > if (defined $fetch) {
> > if ($fetch eq $push) {
> > - $urls_table .= format_repo_url("URL", $fetch);
> > + $urls_table .= format_repo_url("<strong>URL:</strong>", $fetch);
>
> I would really prefer to use CSS for that...
Agreed; <span class="repo_url">URL:</span> is far more appropriate in my
opinion.
--
-Drew Northup N1XIM
AKA RvnPhnx on OPN
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: [PATCH] trace.c: mark file-local function static
From: Drew Northup @ 2010-12-20 12:00 UTC (permalink / raw)
To: Thiago Farina; +Cc: Vasyl', git
In-Reply-To: <AANLkTimBtpOx_GBzC=g4V6jW2aiF7Hg8uALWt2NQFFZG@mail.gmail.com>
On Thu, 2010-12-16 at 21:43 -0200, Thiago Farina wrote:
> On Thu, Dec 16, 2010 at 8:38 PM, Vasyl' <vvavrychuk@gmail.com> wrote:
> > Signed-off-by: Vasyl' Vavrychuk <vvavrychuk@gmail.com>
> > ---
> > trace.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/trace.c b/trace.c
> > index 1e560cb..62586fa 100644
> > --- a/trace.c
> > +++ b/trace.c
> > @@ -25,7 +25,7 @@
> > #include "cache.h"
> > #include "quote.h"
> >
> > -void do_nothing(size_t unused)
> > +static void do_nothing(size_t unused)
> > {
> > }
> >
> If it means something, this looks sane to me.
>
> Acked-by: Thiago Farina <tfransosi@gmail.com>
It may be sane, but why should we trust that it is without a commit
message?
--
-Drew Northup N1XIM
AKA RvnPhnx on OPN
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59
^ permalink raw reply
* Re: 'show' pretty %B without a diff
From: Peter Vereshagin @ 2010-12-20 11:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmxo0ddbm.fsf@alter.siamese.dyndns.org>
Hey Mr(s) Junio show some good to me!
2010/12/20 01:05:17 -0800 Junio C Hamano <gitster@pobox.com> => To Peter Vereshagin :
JCH> > I'd like to output a comment in my script.
JCH> > Thus, I try: 'git show --format="%B" commitId'
JCH> > There is always diff in the end of the output. No matter what format I specify, even when it is empty.
JCH> > How can I avoid diff output in the 'git show'?
JCH>
JCH> The best answer to "avoid" it would be not to use "git show"; after all
JCH> the command is about showing the change it introduces when it is used on a
JCH> commit.
JCH>
JCH> Especially if you are doing a script, you probably should be using
JCH> "cat-file commit" anyway, no?
cat-file doesn't seem to support formatting option?
Also, it outputs 4 more unwanted fields than just the %B... used with the -p is
the only what looks like the what I asked. Is it appropriate in a general case
to skip the everything on that output till the first empty line? cause I'm not
sure those are always 4 and not-empty lines.
Thanks anyway, it's just better than nothing. Although I wish the -p for
cat-file to behave like the simlar-minded --pretty of the 'show'. (=
Or the Git.pm just to have a functionality to get the %B of the commit.
73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627)
--
http://vereshagin.org
^ permalink raw reply
* Re: 'show' pretty %B without a diff
From: Junio C Hamano @ 2010-12-20 9:05 UTC (permalink / raw)
To: Peter Vereshagin; +Cc: git
In-Reply-To: <20101220073842.GC10354@external.screwed.box>
Peter Vereshagin <peter@vereshagin.org> writes:
> Hey Mr(s) git show some good to me!
>
> I'd like to output a comment in my script.
> Thus, I try: 'git show --format="%B" commitId'
> There is always diff in the end of the output. No matter what format I specify, even when it is empty.
> How can I avoid diff output in the 'git show'?
The best answer to "avoid" it would be not to use "git show"; after all
the command is about showing the change it introduces when it is used on a
commit.
Especially if you are doing a script, you probably should be using
"cat-file commit" anyway, no?
^ permalink raw reply
* [wish] git svn create-ignore should support resuming just like git svn fetch does
From: Geoffrey De Smet @ 2010-12-20 9:01 UTC (permalink / raw)
To: git
Hi,
git svn create-ignore doesn't support resuming.
Our OSS project is really big and my connection fails every few 1000 files.
As a result, it's impossible to complete the git svn create-ignore
command and we had to manually recreate them (and guess where they are).
More info here:
http://stackoverflow.com/questions/4484643/migration-from-svn-go-git-how-do-i-create-gitignore-files-for-a-big-repository
--
With kind regards,
Geoffrey De Smet
^ permalink raw reply
* Re: [PATCH] convert filter: supply path to external driver
From: Junio C Hamano @ 2010-12-20 8:52 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Pete Wyckoff, git
In-Reply-To: <4D0F0DFB.20502@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> What happens if there are any shell special characters in the path name
> (or spaces, for that matter). Does this shell-escape the substituted path
> name anywhere in the call chain?
Good eyes. Thanks.
You would need something like %'s (that is "'" modifier applied to 's'
placeholder in strbuf_expand() to cause the expansion sq'ed), and then the
caller must write something like
clean = git-p4-filter --clean '%s'
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Junio C Hamano @ 2010-12-20 8:42 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Kirill Smelkov, git, Axel Bonnet,
Clément Poulain, Diane Gasselin
In-Reply-To: <20101220044214.GA5942@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Sun, Dec 19, 2010 at 06:26:55PM -0800, Junio C Hamano wrote:
> ...
>> FILE_VALID() is about "does that side have a blob there, or is this
>> create/delete diff?", so the caller should be handling this properly as
>> you said, but your fill_textconv() already prepares for the case where the
>> caller for some reason calls this function with "no blob on this side" and
>> returns an empty string (see the precontext of your patch).
>>
>> I think it is fine to be defensive to prepare for such a case, but then
>> dying like this patch does is inconsistent. Perhaps we should move the
>> new check higher and remove the *outbuf = "" while at it?
>
> I'm not sure returning the empty string for a textconv is the right
> solution....
>
> So I stand by my thought that it should die(). But I don't think there
> _are_ any such bugs currently, so it probably doesn't matter much either
> way. I can live with "return 0", or even just leaving it alone.
I must have phrased it badly. I am actually Ok either way (i.e. make this
function prepare for a future when we start passing the missing side to
the function, and have a special case for "if (!DIFF_FILE_VALID)" and
returning something like an empty string, or make this function refuse to
be fed the missing side by dying in "if (!DIFF_FILE_VALID)". I was only
pointing out that the result of applying your patch does one in one case
and another in the other case, which is inconsistent. And we do not know
in advance what is the reasonable fallback value for the missing side, so
we do not now "something like an empty string" is a reasonable thing to do
yet. Hence "move the new check higher and remove ..." was my suggestion,
which would look like the attached, which would be consistent with your
message I am replying to. IOW, I think we are on the same page.
diff.c | 15 +++++++++++----
1 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 5422c43..a04ab2f 100644
--- a/diff.c
+++ b/diff.c
@@ -4401,11 +4401,18 @@ size_t fill_textconv(struct userdiff_driver *driver,
{
size_t size;
+ /*
+ * !DIFF_FILE_VALID(df) means this is a missing side of the
+ * diff (preimage of creation, or postimage of deletion diff).
+ * The caller should not try to textconv such a filespec, as
+ * there is no such blob to begin with!
+ */
+
+ if (!DIFF_FILE_VALID(df))
+ die("Feeding missing side to fill_textconv?: '%s'",
+ df->path);
+
if (!driver || !driver->textconv) {
- if (!DIFF_FILE_VALID(df)) {
- *outbuf = "";
- return 0;
- }
if (diff_populate_filespec(df, 0))
die("unable to read files to diff");
*outbuf = df->data;
^ permalink raw reply related
* Re: [PATCH] convert filter: supply path to external driver
From: Johannes Sixt @ 2010-12-20 8:04 UTC (permalink / raw)
To: Pete Wyckoff; +Cc: git, Junio C Hamano
In-Reply-To: <20101219212925.GA7393@arf.padd.com>
Am 12/19/2010 22:29, schrieb Pete Wyckoff:
> Filtering to support keyword expansion may need the name of
> the file being filtered. In particular, to support p4 keywords
> like
>
> $File: //depot/product/dir/script.sh $
>
> the smudge filter needs to know the name of the file it is
> smudging.
>
> Add a "%s" conversion specifier to the gitattribute for filter.
> It will be expanded with the path name to the file when invoking
> the external filter command.
What happens if there are any shell special characters in the path name
(or spaces, for that matter). Does this shell-escape the substituted path
name anywhere in the call chain?
-- Hannes
^ permalink raw reply
* 'show' pretty %B without a diff
From: Peter Vereshagin @ 2010-12-20 7:38 UTC (permalink / raw)
To: git
Hey Mr(s) git show some good to me!
I'd like to output a comment in my script.
Thus, I try: 'git show --format="%B" commitId'
There is always diff in the end of the output. No matter what format I specify, even when it is empty.
How can I avoid diff output in the 'git show'?
Thank you.
73! Peter pgp: A0E26627 (4A42 6841 2871 5EA7 52AB 12F8 0CE1 4AAC A0E2 6627)
--
http://vereshagin.org
^ permalink raw reply
* Re: Commiting automatically (2)
From: Enrico Weigelt @ 2010-12-20 7:33 UTC (permalink / raw)
To: git
In-Reply-To: <loom.20101220T062209-24@post.gmane.org>
* Maaartin <grajcar1@seznam.cz> wrote:
> Yes, I wonder why it wasn't already implemented. I do something like
> make all; git snapshot; send_the_executable_to_the_customer
> which is IMHO needed quite often.
Perhaps it's wise to just use a separate repository on the same
repository. Maybe make it more convenient using some little
shell functions. I'm also using that for backup purposes, where
the repo lies outside the to-be-backed-up tree.
cu
--
----------------------------------------------------------------------
Enrico Weigelt, metux IT service -- http://www.metux.de/
phone: +49 36207 519931 email: weigelt@metux.de
mobile: +49 151 27565287 icq: 210169427 skype: nekrad666
----------------------------------------------------------------------
Embedded-Linux / Portierung / Opensource-QM / Verteilte Systeme
----------------------------------------------------------------------
^ permalink raw reply
* Re: [PATCH] convert filter: supply path to external driver
From: david @ 2010-12-20 5:52 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Pete Wyckoff, git
In-Reply-To: <20101220022409.GA4625@sigill.intra.peff.net>
On Sun, 19 Dec 2010, Jeff King wrote:
> On Sun, Dec 19, 2010 at 01:59:50PM -0800, Junio C Hamano wrote:
>
>> Modulo one worry. Don't we have, or don't we at least plant to allow us
>> to have, a facility to cache expensive blob conversion result, similar to
>> the textconv caching? How would this change interact with two blobs that
>> live in different paths?
>
> Yeah, it has been talked about, but I don't think anyone is working on
> it (I don't personally use clean/smudge at all, so it is not something I
> have thought _that_ much about).
>
> This does definitely complicate matters, as the filtering is no longer a
> pure mapping of sha1->sha1. However, I think in practice we could do
> just fine by using a multi-level lookup. I.e., mapping a sha1 to be
> filtered into a tree. Each tree entry would represent the remaining
> cache parameters. In this case, the only other parameter we have is the
> path given to the filter (but it could easily be extended to include
> other parameters, if they existed, in this or other caching cases).
>
> We only get a high-performance lookup for the first part of the
> multi-level (i.e., the sha1), but that's OK if we assume the number of
> second-level items is going to be small. Which I think is the case here
> (a sha1 will tend to be found only under one or a few names).
>
> An alternative would be to combine all parts of the filter under a
> single lookup key. E.g., calculate and store under sha1(sha1(blob) +
> filename)). But that means the notes keys are not actual object sha1s,
> which throws off pruning.
>
> Anyway, that's just my quick thinking on the subject. I don't see any
> reason to restrict a feature just because we might want to cache it in
> the future. At the very worst, we could always cache filters which do
> not use %s, and make only %s users pay the penalty.
or you could cache the stats of the filtered file, including any changes
that are made.
David Lang
^ permalink raw reply
* Re: Commiting automatically (2)
From: Maaartin @ 2010-12-20 5:46 UTC (permalink / raw)
To: git
In-Reply-To: <7vaak1ftin.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster <at> pobox.com> writes:
> Maaartin <grajcar1 <at> seznam.cz> writes:
>
> > However, when I use my git-autocom script, those files get marked as
deleted.
> > This is quite strange, especially because of them still existing. I'd
strongly
> > prefer git-autocom to behave just like git commit (i.e., tracking the
files).
> >
> > The relevant part of my script follows:
> >
> > export GIT_INDEX_FILE=.git/autocom.tmp
> > git add -A &&
>
> If you really want "just like commit", then it would be more like "make a
> commit object out of the current index, and put that somewhere outside the
> current branch", and will not involve any "git add", no?
You're right, I was using the wrong term, what I wanted was to take a SNAPSHOT
of the current working dir (this is called "commit" in csv/svn but not in git,
I know).
> A useful goal would be "as if I said 'git add -u && git commit' from the
> current state" (alternatively, you could say s/-u/-A/).
Yes, I wonder why it wasn't already implemented. I do something like
make all; git snapshot; send_the_executable_to_the_customer
which is IMHO needed quite often.
> If this autocom.tmp starts out empty, "add" will of course honor what you
> wrote in .gitignore hence would not add ignored files. You may have '*.o'
> in the ignore mechanism to exclude usual build products. Until you
> somehow tell git that you care about a vendor-supplied binary blob file
> "binblob1.o" even though it has a name for usual ignored ones, you don't
> want to get it tracked, and once you have done so with "git add -f", you
> do want to get it tracked from that point. But your script cannot be
> clever enough to selectively say "add -f" for such a file.
>
> The "from the current state" part of the sentence of your goal (clarified
> by the second paragraph above) fundamentally means you need to start from
> your real index, so "cp -p .git/index $TMP_INDEX" is both appropriate and
> inevitable for your script.
Now it's clear, thank you for the explanation.
^ permalink raw reply
* Re: Commiting automatically (2)
From: Maaartin @ 2010-12-20 5:12 UTC (permalink / raw)
To: git
In-Reply-To: <20101219183619.GB11955@burratino>
Jonathan Nieder <jrnieder <at> gmail.com> writes:
> Hmm, the "git add" manual suggests it is the other way around:
>
> -A, --all
> Like -u, but match <filepattern> against files in the working
> tree in addition to the index. That means that it will find new
> files as well as staging modified content and removing files
> that are no longer in the working tree.
>
> So I would expect "git add -A" to do the same thing as "git add -u",
> plus handling added files.
>
> Maaartin, could you give an example showing where add -A goes wrong?
I can't, since I was wrong. These commits have two parents (I'm not sure if this
is a good idea), and that's why I saw no changes in the log. Actually, "git add -
A" does everything I need, and with "/bin/cp .git/index $GIT_INDEX_FILE"
everything seems to work. Sorry for the noise.
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Jeff King @ 2010-12-20 4:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kirill Smelkov, git, Axel Bonnet, Cl??ment Poulain,
Diane Gasselin
In-Reply-To: <7vd3oxdv3h.fsf@alter.siamese.dyndns.org>
On Sun, Dec 19, 2010 at 06:41:22PM -0800, Junio C Hamano wrote:
> > I'm not that familiar with git internals involved, so here is updated
> > patch with added paragraph about "df->sha1_valid=0 means files from
> > worktree with unknown sha1", and appropriate excerpt from Jeff's post.
> > That's the most reasonable I could come up with.
> [...]
> Here is how I would describe it.
>
> commit 87bb04bb760659dd33d7a173333329cd900620a9
> Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> Date: Sat Dec 18 17:54:12 2010 +0300
>
> fill_textconv(): Don't get/put cache if sha1 is not valid
>
> When blaming files in the working tree, the filespec is marked with
> !sha1_valid, as we have not given the contents an object name yet. The
> function to cache textconv results (keyed on the object name), however,
> didn't check this condition, and ended up on storing the cached result
> under a random object name.
>
> Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
FWIW, I think that is a good description.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Jeff King @ 2010-12-20 4:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Kirill Smelkov, git, Axel Bonnet, Clément Poulain,
Diane Gasselin
In-Reply-To: <7vr5dddvrk.fsf@alter.siamese.dyndns.org>
On Sun, Dec 19, 2010 at 06:26:55PM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > PS It is a little disturbing that in fill_textconv, we handle
> > case (1), !DIFF_FILE_VALID for the non-textconv case, but not so for the
> > textconv case. I think we are OK, as get_textconv will never load a
> > textconv driver for a !DIFF_FILE_VALID filespec, so we always follow the
> > non-textconv codepath in that case. But I am tempted to do this just to
> > be more defensive:
>
> FILE_VALID() is about "does that side have a blob there, or is this
> create/delete diff?", so the caller should be handling this properly as
> you said, but your fill_textconv() already prepares for the case where the
> caller for some reason calls this function with "no blob on this side" and
> returns an empty string (see the precontext of your patch).
>
> I think it is fine to be defensive to prepare for such a case, but then
> dying like this patch does is inconsistent. Perhaps we should move the
> new check higher and remove the *outbuf = "" while at it?
I'm not sure returning the empty string for a textconv is the right
solution. I am inclined to say that trying to textconv a
!DIFF_FILE_VALID is simply an error. More on that in a second.
If we were to do anything else, I would think it would be to feed "" to
the textconv filter, in case it wanted to do something magical for the
create/delete case. For example, imagine a textconv filter which turned
a string of bytes like "foo" into a length plus set of converted bytes,
like "3: FOO". You would want the /dev/null case to turn into "0: ".
Now, this is obviously a ridiculous toy case. I have no idea if anyone
would want to do something like that. So far most people have been happy
with /dev/null never being textconv'd, and always looking like the empty
string. Moreover, even if somebody did want this behavior, 99% of the
other filters _wouldn't_ want the behavior. Because programs like
odt2txt or exiftool that people _do_ use for textconv filters do not
want to be fed /dev/null; they will signal an error.
So that is my "if we wanted it to do something useful, this is what it
would do" case, and I don't see any real-world evidence that anybody
wants that. Now on to being defensive.
What we are defending against is a caller marking something as
to-be-textconv'd, even though it is !DIFF_FILE_VALID. But what did the
caller want? One sensible behavior is what I described above. Or maybe
they did just want the empty string. Or more likely, it is simply a bug
in the diff code. Since we haven't defined the semantics, I would much
rather loudly scream "BUG!" than paper over it by returning what we
guess they would have wanted (which may generate subtly wrong results).
And then we can think about that use case and decide what the semantics,
if any, should be.
So I stand by my thought that it should die(). But I don't think there
_are_ any such bugs currently, so it probably doesn't matter much either
way. I can live with "return 0", or even just leaving it alone.
-Peff
^ permalink raw reply
* cvsimport still not working with cvsnt
From: Guy Rouillier @ 2010-12-20 4:05 UTC (permalink / raw)
To: git
I'm going to try sending this blind, as the mailing list has sent me the
promised authorization key after 24 hrs.
I'm brand new to git. We'll be moving over from CVS, so I imported a
small part of our CVS repository to start learning git. We use the
CVSNT server, and git-cvsimport was failing with "I HATE YOU". I
finally found the problems, both of which were reported in 2008 here:
http://kerneltrap.org/mailarchive/git/2008/3/13/1157364
However, these changes do not appear in the version 1.7.2.2 that Gentoo
supplies. I checked the 1.7.3-rc2 source and the changes are not in
there either.
I do see one possible issue with the supplied modifications. At work,
we upgraded from CVS to CVSNT. So, my home directory has both .cvspass
(from the original CVS) and .cvs/cvspass (after the conversion to
CVSNT.) Sloppy housekeeping on my part, I admit, but probably not
uncommon. The supplied patch would pick up the original CVS file and
would fail. (BTW, this is true only of the git-cvsimport.perl script
itself; cvsps must shell out to the installed CVS client (in my case,
cvsnt), because when I invoked that manually, it worked.)
So, I would advise checking to see if both files exist, and if so exit
with an error. Unless cvsimport wants to get real fancy and shell out
to the installed cvs client to try to figure out what is installed,
there is no way to tell which cvspass file is actively being used. I
don't recommend trying to figure this out, as the user's intent is unclear.
--
Guy Rouillier
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Junio C Hamano @ 2010-12-20 2:41 UTC (permalink / raw)
To: Kirill Smelkov
Cc: Junio C Hamano, Jeff King, git, Axel Bonnet, Cl??ment Poulain,
Diane Gasselin
In-Reply-To: <20101219121059.GA10985@landau.phys.spbu.ru>
Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:
> On Sat, Dec 18, 2010 at 07:23:29PM -0800, Junio C Hamano wrote:
>> Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:
>>
>> > Thanks for your ACK and for the explanation.
>> >
>> > My last patches to git were blame related so semi-intuitively I knew
>> > that invalid sha1 are coming from files in worktree. Your description
>> > makes things much more clear and I'd put it into patch log as well.
>> > What is the best practice for this? For me to re-roll, or for Junio to
>> > merge texts?
>>
>> Re-rolling to explain changes in your own words is preferred; thanks.
>
> I see, thanks.
>
> I'm not that familiar with git internals involved, so here is updated
> patch with added paragraph about "df->sha1_valid=0 means files from
> worktree with unknown sha1", and appropriate excerpt from Jeff's post.
> That's the most reasonable I could come up with.
>
> Thanks,
> Kirill
>
> P.S. please don't forget to pick patch 1 which is unchanged.
Here is how I would describe it.
commit 87bb04bb760659dd33d7a173333329cd900620a9
Author: Kirill Smelkov <kirr@landau.phys.spbu.ru>
Date: Sat Dec 18 17:54:12 2010 +0300
fill_textconv(): Don't get/put cache if sha1 is not valid
When blaming files in the working tree, the filespec is marked with
!sha1_valid, as we have not given the contents an object name yet. The
function to cache textconv results (keyed on the object name), however,
didn't check this condition, and ended up on storing the cached result
under a random object name.
Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Junio C Hamano @ 2010-12-20 2:32 UTC (permalink / raw)
To: Kirill Smelkov
Cc: git, Axel Bonnet, Clément Poulain, Diane Gasselin, Jeff King
In-Reply-To: <14308c2dd50037246e319649944d308b9f32fc39.1292681111.git.kirr@landau.phys.spbu.ru>
Kirill Smelkov <kirr@landau.phys.spbu.ru> writes:
> It turned out, under blame there are requests to fill_textconv() with
> sha1=0000000000000000000000000000000000000000 and sha1_valid=0.
The code shouldn't even look at sha1[] if sha1_valid is false, as we do
not know the hash value for the blob (reading from the working tree).
Thanks.
^ permalink raw reply
* Re: [PATCH 2/2] fill_textconv(): Don't get/put cache if sha1 is not valid
From: Junio C Hamano @ 2010-12-20 2:26 UTC (permalink / raw)
To: Jeff King
Cc: Kirill Smelkov, git, Axel Bonnet, Clément Poulain,
Diane Gasselin
In-Reply-To: <20101218161337.GB18643@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> PS It is a little disturbing that in fill_textconv, we handle
> case (1), !DIFF_FILE_VALID for the non-textconv case, but not so for the
> textconv case. I think we are OK, as get_textconv will never load a
> textconv driver for a !DIFF_FILE_VALID filespec, so we always follow the
> non-textconv codepath in that case. But I am tempted to do this just to
> be more defensive:
FILE_VALID() is about "does that side have a blob there, or is this
create/delete diff?", so the caller should be handling this properly as
you said, but your fill_textconv() already prepares for the case where the
caller for some reason calls this function with "no blob on this side" and
returns an empty string (see the precontext of your patch).
I think it is fine to be defensive to prepare for such a case, but then
dying like this patch does is inconsistent. Perhaps we should move the
new check higher and remove the *outbuf = "" while at it?
> diff --git a/diff.c b/diff.c
> index b0ee213..5320849 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4404,22 +4404,25 @@ size_t fill_textconv(struct userdiff_driver *driver,
> if (!driver || !driver->textconv) {
> if (!DIFF_FILE_VALID(df)) {
> *outbuf = "";
> return 0;
> }
> if (diff_populate_filespec(df, 0))
> die("unable to read files to diff");
> *outbuf = df->data;
> return df->size;
> }
>
> + if (!DIFF_FILE_VALID(df))
> + die("BUG: attempt to textconv an invalid filespec");
> +
^ permalink raw reply
* Re: [PATCH] convert filter: supply path to external driver
From: Jeff King @ 2010-12-20 2:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Pete Wyckoff, git
In-Reply-To: <7vzks1e84p.fsf@alter.siamese.dyndns.org>
On Sun, Dec 19, 2010 at 01:59:50PM -0800, Junio C Hamano wrote:
> Modulo one worry. Don't we have, or don't we at least plant to allow us
> to have, a facility to cache expensive blob conversion result, similar to
> the textconv caching? How would this change interact with two blobs that
> live in different paths?
Yeah, it has been talked about, but I don't think anyone is working on
it (I don't personally use clean/smudge at all, so it is not something I
have thought _that_ much about).
This does definitely complicate matters, as the filtering is no longer a
pure mapping of sha1->sha1. However, I think in practice we could do
just fine by using a multi-level lookup. I.e., mapping a sha1 to be
filtered into a tree. Each tree entry would represent the remaining
cache parameters. In this case, the only other parameter we have is the
path given to the filter (but it could easily be extended to include
other parameters, if they existed, in this or other caching cases).
We only get a high-performance lookup for the first part of the
multi-level (i.e., the sha1), but that's OK if we assume the number of
second-level items is going to be small. Which I think is the case here
(a sha1 will tend to be found only under one or a few names).
An alternative would be to combine all parts of the filter under a
single lookup key. E.g., calculate and store under sha1(sha1(blob) +
filename)). But that means the notes keys are not actual object sha1s,
which throws off pruning.
Anyway, that's just my quick thinking on the subject. I don't see any
reason to restrict a feature just because we might want to cache it in
the future. At the very worst, we could always cache filters which do
not use %s, and make only %s users pay the penalty.
-Peff
^ permalink raw reply
* Re: Basic Git Questions
From: Thomas Rast @ 2010-12-19 23:15 UTC (permalink / raw)
To: Parag Kalra; +Cc: Konstantin Khomoutov, git
In-Reply-To: <20101219230839.GM3264@localhost.localdomain>
Konstantin Khomoutov wrote:
> > 2. Also sometime when the code is committed, I realize that I have
> > forgot to change or add a file. Is there a way we can we commit a file
> > to existing commit number with git log showing only the old git commit
> > number and not creating a new one for the last commit.
> `git commit --amend` does exactly that -- amends the last commit
> (pointed by the HEAD ref).
Let me add that you can fundamentally not edit any git object in any
way without its ID (hash, sha1, whatever you call it) changing. All
methods of "changing" commits, like amend, rebase, filter-branch
etc. always create a new object. The trick is that, especially with
amend, the branch is also changed to point to the new one, so unless
you have already merged or pushed that commit, this is as close to
"changing" as it gets.
Tv put this more eloquently as: "you can never change; you can only
rewrite and forget".
If you *have* merged or pushed the commit, you shouldn't rewrite
history. It's not that hard to fix up your local merges, but if you
have pushed the history and others have started working with it, it's
*very* annoying for them.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Basic Git Questions
From: Konstantin Khomoutov @ 2010-12-19 23:08 UTC (permalink / raw)
To: Parag Kalra; +Cc: git
In-Reply-To: <AANLkTiki2e4-YD0P9kb7g-WbLTPhAXQ22R2jCAXVtjN1@mail.gmail.com>
On Sun, Dec 19, 2010 at 02:32:41PM -0800, Parag Kalra wrote:
> 1. I generally add the files in my working directory using "git add
> .". But sometimes you want to add all the files except 1 or 2 file. Is
> there a shortcut way we can tell git to add all the files but exclude
> a particular file.
Use `git add .` to add all the files and then `git rm --cached` those
one or two you don't need to be committed. See the git-rm manual for
details.
> 2. Also sometime when the code is committed, I realize that I have
> forgot to change or add a file. Is there a way we can we commit a file
> to existing commit number with git log showing only the old git commit
> number and not creating a new one for the last commit.
`git commit --amend` does exactly that -- amends the last commit
(pointed by the HEAD ref).
If you need to modify a commit older than the last one, you have to
rebase a part of the branch starting with that commit and then edit
(amend) that commit during the rebasing process. This might have
certain caveats though, so refer to git-rebase manual.
More on rebasing can be found in various pieces of documentation and
books listed at http://git-scm.com/documentation
^ permalink raw reply
* Re: subdirectory-filter does not delete files before the directory came into existence?
From: Thomas Rast @ 2010-12-19 22:51 UTC (permalink / raw)
To: Jan Wielemaker; +Cc: git
In-Reply-To: <1292751289.21229.26.camel@ct>
Jan Wielemaker wrote:
> On Sun, 2010-12-19 at 03:23 +0100, Thomas Rast wrote:
> > Jan Wielemaker wrote:
> > > * get all tags, use comm and delete the tags not in the `contained'
> > > set above.
[...]
> > http://article.gmane.org/gmane.comp.version-control.git/91708
[...]
> Funny. That was me having problems with filtering out directories
> as well :-) I thought your patch was added using the --prune-empty
> flag. I guess you can comment on that. I can confirm that I've got
> nice and clean filtering using
No, those two are rather different. --prune-empty drops commits that
became "no-ops" in the sense that their tree is the same as their
(only) parent's. In the case of --subdirectory-filter, --prune-empty
is most likely[*] redundant since the former already enables history
simplification limited to that directory.
As you can see from "TOY PATCH", my patch wasn't really meant for
application anyway. I'm now wondering what the ramifications would
be. filter-branch only attempts to change refs that you told it to
(listed positively on the command line), so maybe deleting anything
that was not rewritten is a sensible option (not default, mind you).
[*] Read: I think it is redundant, I'm just too lazy to double-check.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ 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