* Re: metastore
From: Josh England @ 2007-09-17 19:46 UTC (permalink / raw)
To: david
Cc: martin f krafft, git, Daniel Barkalow, Junio C Hamano,
Johannes Schindelin, Thomas Harning Jr., Francis Moreau,
Nicolas Vilz, David Härdeman
In-Reply-To: <Pine.LNX.4.64.0709171011160.1558@asgard.lang.hm>
I'd like to point out the following two posts, as I think they are
relevant to this thread:
[PATCH] example hook script to save/restore file permissions/ownership
http://marc.info/?l=git&m=118953004817642&w=2
[PATCH] post_merge hook, related documentation, and tests
http://marc.info/?l=git&m=118953004730496&w=2
The hook script above runs in a pre-commit hook to write out file
metadata to a file in the repository. It can then be run from the
post-merge hook (patch above) to restore permissions. Running it from a
post-checkout hook may be more appropriate, but post-merge seems to work
well for my purposes. The script handles merge conflicts and (in my
testing) does the right thing. I'm using it now to track metadata for
not just /etc, but an entire linux image.
It will handle merge conflicts by recognizing that the metadata file had
a conflict, and will direct the user to resolve the conflict and reset
working dir perms before allowing a commit.
-JE
^ permalink raw reply
* Re: RFC: German translation vocabulary
From: Christian Stimming @ 2007-09-17 19:45 UTC (permalink / raw)
To: Florian Weimer; +Cc: git
In-Reply-To: <87y7f6fjjl.fsf@mid.deneb.enyo.de>
Thanks for the replies so far. I'll summarize the proposals below, and I'll
take the freedom to reply to them directly. (Note: Please CC: me on replies
as I'm not subscribed to this list - too much traffic for me.)
One thing to keep in mind (one reader proposed to use "commit"
and "repository" in untranslated form): This translation is really about
translating the program, and the intended audience are those people who are
*not* familiar with the English git terminology. Hence, I really really want
to try hard to avoid untranslated terminology here. German really has enough
words to choose from (just like every other language), so that it should be
possible to avoid using untranslated English words just because we couldn't
come up with German ones that fir.
> repository - Projektarchiv
Simon Richter thinks just "Archiv" would be fine as well in most places.
Florian Weimar says "Projekt" or "Archiv" should suffice. David Kastrup
agrees to this.
I think "Archiv" sounds quite unspecific, because it can be another room, or
another harddisk, or another directory. Whereas a git repository is
particular to this single git project. Also, when reading through the
TortoiseSVN docs, "Projektarchiv" worked quite nicely. I'd still stick to
this.
> revision - Version
Florian Weimar proposes "Versionsangabe". He thinks "revision" is most used as
a short form of "revision specifier".
I think in sentences like "let's switch the working copy from revision xyz to
revision abc" the word "Version" would work much better than any longer form.
I'd stick to this, especially since this proposal came here from the mailing
list already :-)
> staging area - Bereitstellung
Simon Richter remarks this German word is being used a translation
for "deployment", i.e. making binaries available to end users (however, this
is probably specific to some particular development environment, isn't it?).
He thinks "Vorbereitung" would be better here. Florian Weimar
proposes "Index".
I think the word should have a connotation of "another place which is
separated from the working copy". The military term "Bereitstellung" IMHO
gives this rather nicely. I haven't seen that term in the ambiguous meaning
Simon pointed out; is this a problem? As for "Index": As mentioned above it
should be possible to find a German word here.
> branch [noun] - Zweig
No comments to this one; it seems to be just fine.
> branch [verb] - verzweigen
Florian Weimar mentiones "abzweigen", if it's used transitive.
In itself "abzweigen" is a nice word, but "verzweigen" gives more of the
(desired) connotation of a tree's branches (uh oh! Linas will beat me for
this! Of course this isn't a tree, it's a graph!) and hence for consistency I
would stick to "verzweigen".
> working copy, working tree - Arbeitskopie
No comments to this one (or did I miss anyone); it seems to be just fine.
> [commit] message - Meldung (Nachricht?; Source Safe: Kommentar)
David Soria first preferred "Kommentar". David Kastrup
proposes "Beschreibung", or later instead "Zusammenfassung", which then David
Soria thinks is the best so far.
I think "Zusammenfassung" would rather describe what the diffstat is about, as
this summarizes the actual commit. As we're naming "the short text that
describes what this is about", I think actually "Beschreibung" is probably
best so far.
> msgid "checkout [noun]"
> msgstr "Auscheck? Ausspielung? Abruf? (Source Safe: Auscheckvorgang)"
>
> msgid "checkout [verb]"
> msgstr "auschecken? ausspielen? abrufen? (Source Safe: auschecken)"
Simon Richter proposed the long translation "Erstellung einer Arbeitskopie"
(gets less awkward when you make proper sentences from it). Florian Weimar
asks how's a checkout different from a working copy? But he wouldn't
translate "repository" and "commit", at least if they are used as nouns.
I agree with Simon Richter here, just as I've already explained in my initial
email: The noun should probably simply be the working copy, "Arbeitskopie",
and the verb should be something with "Arbeitskopie erstellen". However, we
have strings like "Checkout this branch...", and those need yet another word.
Maybe "Arbeitskopie umstellen"? I'm still unsure.
> msgid "commit [noun]"
> msgstr "Übertragung (Sendung?, Übergabe?, Einspielung?, Ablagevorgang?)"
Alexander Wuerstlein proposed "Vorgang" (think governmental German). Florian
Weimar proposes "Sammlung" and "sammeln", to which David Kastrup replied it
doesn't fit because "sammeln" is what you do _before_ committing. In addition
David Kastrup proposes Buchung, Einbuchung, Verbuchung, Registrierung.
Alexander Wuerstlein proposes "Transaktion", to which David replied he
thinks "Transaktion" is anything with a permanent effect, so he finds this
term too unspecific: it would equally well cover resetting, tagging, and a
number of other things. (Also, it wouldn't work as a verb.)
I think we should try to replace this (the noun!) with "revision" and hence
translate it as "Version". However, this needs to be checked in actual
strings in the program.
> msgid "commit [verb]"
> msgstr "übertragen (TortoiseSVN: übertragen; Source Safe: einchecken;
> senden?, übergeben?, einspielen?, einpflegen?, ablegen?)"
David Soria prefers "Einspielung" as he think it reflects better, that the
commit is locally. Simon Richter proposes "einspielen"? Problem is that this
cannot be properly turned into a noun.
This verb appears on the one-word button "commit", which is obviously the most
important button in git-gui. I think both "einspielen" and "übertragen" would
work in that context, but David Kastrup's proposals of "buchen"
or "verbuchen" and the others of "einpflegen", "ablegen" might also work. Yet
more proposals, or other hints which one of these would work best?
Thanks for all the suggestions. This should be thought about for a few more
days, and then I'll prepare an updated German glossary file to be committed
to the repository.
Regards,
Christian
^ permalink raw reply
* Re: [PATCH 7/8] git-gc --auto: restructure the way "repack" command line is built.
From: Junio C Hamano @ 2007-09-17 19:53 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709171040070.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 17 Sep 2007, Junio C Hamano wrote:
>
>> @@ -154,8 +161,6 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
>> * Auto-gc should be least intrusive as possible.
>> */
>> prune = 0;
>> - for (i = 0; i < ARRAY_SIZE(argv_repack_auto); i++)
>> - argv_repack[i] = argv_repack_auto[i];
>> if (!need_to_gc())
>> return 0;
>> }
>
> This subtly changes behaviour: --auto ran also garbage collection for
> reflogs and rerere.
Does it change any behaviour? It "ran" meaning "it used to run
them always"? I do not think so. We always exited here if
there is no gc needed for object store.
I however think a behaviour change might be needed around here.
"gc --auto" is about being lightweight and no impact in the
semantics from the point fo view of the repository user. As
such, I suspect we may not want to run gc on reflogs nor rerere.
Running pack-refs is supposed to be "no impact in the semantics"
operation so I think it is Ok, but even that would affect how
the ancient fetch over http implementations would interact with
this repository.
But skipping these would make automated "behind the scene" gc
much less useful. I dunno.
^ permalink raw reply
* Re: [PATCH 8/8] git-gc --auto: run "repack -A -d -l" as necessary.
From: Junio C Hamano @ 2007-09-17 19:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709171044170.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> diff --git a/builtin-gc.c b/builtin-gc.c
>> index 34ce35b..a82f6be 100644
>> --- a/builtin-gc.c
>> +++ b/builtin-gc.c
>> @@ -78,6 +83,9 @@ static int too_many_loose_objects(void)
>> int num_loose = 0;
>> int needed = 0;
>>
>> + if (gc_auto_threshold <= 0)
>> + return 0;
>> +
>
> Heh, patch 6/8 explicitely moved this check out of the function.
Back in 6/8 there was only one toggle to disable auto gc
and it made sense to have it there. Now the two switches act
together, so that you can say "Don't count my loose objects, but
do check the number of packs" if you wanted to.
>> + if (!p->pack_local)
>> + continue;
>> + suffix = p->pack_name + strlen(p->pack_name) - 5;
>
> I suspect that you need something like
>
> int len;
> len = strlen(p->pack_name);
> if (len < 5)
> continue;
>
> before this.
While your additional check would not hurt, I actually think it
is the other way around; the code is already overly cautious.
If it is linked to packed_git list, the file should have already
been checked for having the suffix ".pack".
>> + /*
>> + * If there are too many loose objects, but not too many
>> + * packs, we run "repack -d -l". If there are too many packs,
>> + * we run "repack -A -d -l". Otherwise we tell the caller
>> + * there is no need.
>> + */
>> argv_repack[ac++] = "repack";
>> + if (too_many_packs())
>> + argv_repack[ac++] = "-A";
>> + if (!too_many_loose_objects() && ac == 1)
>> + return 0;
>
> Why not
>
> else if (!too_many_loose_objects())
> return 0;
>
Ok.
^ permalink raw reply
* Re: [PATCH 5/8] git-gc --auto: add documentation.
From: Junio C Hamano @ 2007-09-17 19:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709171031420.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Mon, 17 Sep 2007, Junio C Hamano wrote:
>
>> + When there are approximately more than this many loose
>> + objects in the repository, `git gc --auto` that is
>> + invoked by some Porcelain commands will create a new
>> + pack and prune them.
>
> It might strike a newbie as dangerous to read "prune them" here. Maybe
>
> ... `git gc --auto` will pack the loose objects. Some Porcelain
> commands use this command, to do a light weight garbage collection
> from time to time.
> +
> *Note*: you should still run "git gc" from time to time, since the
> "--auto" option is designed to be fast, whereas "git gc" _without_
> auto is designed to create small packs.
>
> Hmm? (I think that this is the most visible part of the --auto business,
> so I would document the "git gc" note here...)
Makes sense.
^ permalink raw reply
* Re: RFC: German translation vocabulary
From: Florian Weimer @ 2007-09-17 19:56 UTC (permalink / raw)
To: Christian Stimming; +Cc: git
In-Reply-To: <200709172145.26291.stimming@tuhh.de>
* Christian Stimming:
> This verb appears on the one-word button "commit", which is
> obviously the most important button in git-gui. I think both
> "einspielen" and "übertragen" would work in that context,
By the way, what about "eintragen"? It's got the advantage that
"Eintrag" is a nicen oun.
^ permalink raw reply
* State of Perforce importing.
From: David Brown @ 2007-09-17 19:30 UTC (permalink / raw)
To: Git
I'd like to track a lot of code living in a Perforce repository, so I've
been playing with 'git-p4.py'. Is the one in the contrib/fast-import
directory the latest version, or is there a better place.
So far, it is having a couple of problems:
- The commit comment is empty. It doesn't seem to grab the Perforce
description, and the user seems to be <a@b>.
- Every revision seems to check every file out of Perforce. This means
that for the directory I want, every revision is going to take about 20
minutes.
Before I start working on it, I'd like to make sure I'm working on the
latest code, though.
Thanks,
David
^ permalink raw reply
* Re: git-send-email creates duplicate Message-Id's
From: Junio C Hamano @ 2007-09-17 20:22 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Avi Kivity, linux-kernel, git
In-Reply-To: <20070917155901.GP18232@stusta.de>
Adrian Bunk <bunk@kernel.org> writes:
> The following might be a bug in git-send-email (git maintainers Cc'ed
> and KVM list removed from Cc):
>
> Patch 54 got the same Message-Id as patch 61 and patch 89 got the same
> Message-Id as patch 104.
> ...
> The emails are:
> http://marc.info/?l=linux-kernel&m=119002061330270&w=2
> http://marc.info/?l=linux-kernel&m=119002059626434&w=2
> http://marc.info/?l=linux-kernel&m=119002060011801&w=2
> http://marc.info/?l=linux-kernel&m=119002060318915&w=2
The old code generated rand(4200) for each message and appended
it to the timestamp. I do not know where the original author
got 4200 from, but I think if you send many messages within a
single second it is possible to get collisions.
I guess something like this patch is an improvement? It
generates a single prefix from timestamp and random, and appends
a number that is incremented for each message.
---
diff --git a/git-send-email.perl b/git-send-email.perl
index dd7560b..e250732 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -477,10 +477,18 @@ sub extract_valid_address {
# We'll setup a template for the message id, using the "from" address:
+my ($message_id_stamp, $message_id_serial);
sub make_message_id
{
- my $date = time;
- my $pseudo_rand = int (rand(4200));
+ my $uniq;
+ if (!defined $message_id_stamp) {
+ $message_id_stamp = sprintf("%s-%s", time, int(rand(4200)));
+ $message_id_serial = 0;
+ }
+ $message_id_serial++;
+
+ $uniq = "$message_id_stamp-$message_id_serial";
+
my $du_part;
for ($sender, $repocommitter, $repoauthor) {
$du_part = extract_valid_address(sanitize_address($_));
@@ -490,8 +498,8 @@ sub make_message_id
use Sys::Hostname qw();
$du_part = 'user@' . Sys::Hostname::hostname();
}
- my $message_id_template = "<%s-git-send-email-$du_part>";
- $message_id = sprintf $message_id_template, "$date$pseudo_rand";
+ my $message_id_template = "<%s-git-send-email-%s>";
+ $message_id = sprintf($message_id_template, $uniq, $du_part);
#print "new message id = $message_id\n"; # Was useful for debugging
}
^ permalink raw reply related
* Re: git-send-email creates duplicate Message-Id's
From: Matti Aarnio @ 2007-09-17 20:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Adrian Bunk, Avi Kivity, linux-kernel, git
In-Reply-To: <7vejgxyrde.fsf@gitster.siamese.dyndns.org>
On Mon, Sep 17, 2007 at 01:22:05PM -0700, Junio C Hamano wrote:
> Adrian Bunk <bunk@kernel.org> writes:
>
> > The following might be a bug in git-send-email (git maintainers Cc'ed
> > and KVM list removed from Cc):
> >
> > Patch 54 got the same Message-Id as patch 61 and patch 89 got the same
> > Message-Id as patch 104.
> > ...
> > The emails are:
> > http://marc.info/?l=linux-kernel&m=119002061330270&w=2
> > http://marc.info/?l=linux-kernel&m=119002059626434&w=2
> > http://marc.info/?l=linux-kernel&m=119002060011801&w=2
> > http://marc.info/?l=linux-kernel&m=119002060318915&w=2
>
> The old code generated rand(4200) for each message and appended
> it to the timestamp. I do not know where the original author
> got 4200 from, but I think if you send many messages within a
> single second it is possible to get collisions.
>
> I guess something like this patch is an improvement? It
> generates a single prefix from timestamp and random, and appends
> a number that is incremented for each message.
Much better. You may also consider a possibility of
letting your local MTA do the Message-ID generation, unless
you are tracking something with it and thus need to know the
generated values. .. but apparently git much prefers sending
email by SMTP, where the message-id must be present, or one
really should block any such emails.. (except that systems
like qmail send error messages without message-id ...)
My own recipe is:
sprintf("%d-%d-%d", time , getpid, ++localsequence)
catenate on that your favourite domain name where that recipe
is likely to be valid, and you are all set.
/Matti Aarnio -- one of <postmaster@vger.kernel.org>
^ permalink raw reply
* [PATCH] git-commit.sh: Shell script cleanup
From: David Kastrup @ 2007-09-17 20:56 UTC (permalink / raw)
To: git
This moves "shift" out of the argument processing "case". It also
replaces quite a bit of expr calls with ${parameter#word} constructs,
and uses ${parameter:+word} for avoiding conditionals where possible.
Signed-off-by: David Kastrup <dak@gnu.org>
---
Actually, this is almost the same patch as the one posted during the
1.5.3 semi-freeze. I just checked that it still applies and works,
and that nothing has been changed incompatibly in the mean time.
Some might argue that git-commit.sh could be implemented in C soon,
anyway. Well, exactly that would be an excellent reason to do this
sort of cleanup, since then this file would get moved to
contrib/examples, and of course we want the examples to look as clean
and understandable as possible, don't we?
At more than 4 times as many deletions than insertions, this is a nice
cleanup, and quite more readable.
git-commit.sh | 72 +++++++++++---------------------------------------------
1 files changed, 14 insertions(+), 58 deletions(-)
diff --git a/git-commit.sh b/git-commit.sh
index bb113e8..3e46dbb 100755
--- a/git-commit.sh
+++ b/git-commit.sh
@@ -98,101 +98,71 @@ do
no_edit=t
log_given=t$log_given
logfile="$1"
- shift
;;
-F*|-f*)
no_edit=t
log_given=t$log_given
- logfile=`expr "z$1" : 'z-[Ff]\(.*\)'`
- shift
+ logfile="${1#-[Ff]}"
;;
--F=*|--f=*|--fi=*|--fil=*|--file=*)
no_edit=t
log_given=t$log_given
- logfile=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- shift
+ logfile="${1#*=}"
;;
-a|--a|--al|--all)
all=t
- shift
;;
--au=*|--aut=*|--auth=*|--autho=*|--author=*)
- force_author=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- shift
+ force_author="${1#*=}"
;;
--au|--aut|--auth|--autho|--author)
case "$#" in 1) usage ;; esac
shift
force_author="$1"
- shift
;;
-e|--e|--ed|--edi|--edit)
edit_flag=t
- shift
;;
-i|--i|--in|--inc|--incl|--inclu|--includ|--include)
also=t
- shift
;;
--int|--inte|--inter|--intera|--interac|--interact|--interacti|\
--interactiv|--interactive)
interactive=t
- shift
;;
-o|--o|--on|--onl|--only)
only=t
- shift
;;
-m|--m|--me|--mes|--mess|--messa|--messag|--message)
case "$#" in 1) usage ;; esac
shift
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message="$1"
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-$1"
- fi
+}$1"
no_edit=t
- shift
;;
-m*)
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message=`expr "z$1" : 'z-m\(.*\)'`
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-`expr "z$1" : 'z-m\(.*\)'`"
- fi
+}${1#-m}"
no_edit=t
- shift
;;
--m=*|--me=*|--mes=*|--mess=*|--messa=*|--messag=*|--message=*)
log_given=m$log_given
- if test "$log_message" = ''
- then
- log_message=`expr "z$1" : 'z-[^=]*=\(.*\)'`
- else
- log_message="$log_message
+ log_message="${log_message:+${log_message}
-`expr "z$1" : 'zq-[^=]*=\(.*\)'`"
- fi
+}${1#*=}"
no_edit=t
- shift
;;
-n|--n|--no|--no-|--no-v|--no-ve|--no-ver|--no-veri|--no-verif|\
--no-verify)
verify=
- shift
;;
--a|--am|--ame|--amen|--amend)
amend=t
use_commit=HEAD
- shift
;;
-c)
case "$#" in 1) usage ;; esac
@@ -200,15 +170,13 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=
- shift
;;
--ree=*|--reed=*|--reedi=*|--reedit=*|--reedit-=*|--reedit-m=*|\
--reedit-me=*|--reedit-mes=*|--reedit-mess=*|--reedit-messa=*|\
--reedit-messag=*|--reedit-message=*)
log_given=t$log_given
- use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+ use_commit="${1#*=}"
no_edit=
- shift
;;
--ree|--reed|--reedi|--reedit|--reedit-|--reedit-m|--reedit-me|\
--reedit-mes|--reedit-mess|--reedit-messa|--reedit-messag|\
@@ -218,7 +186,6 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=
- shift
;;
-C)
case "$#" in 1) usage ;; esac
@@ -226,15 +193,13 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=t
- shift
;;
--reu=*|--reus=*|--reuse=*|--reuse-=*|--reuse-m=*|--reuse-me=*|\
--reuse-mes=*|--reuse-mess=*|--reuse-messa=*|--reuse-messag=*|\
--reuse-message=*)
log_given=t$log_given
- use_commit=`expr "z$1" : 'z-[^=]*=\(.*\)'`
+ use_commit="${1#*=}"
no_edit=t
- shift
;;
--reu|--reus|--reuse|--reuse-|--reuse-m|--reuse-me|--reuse-mes|\
--reuse-mess|--reuse-messa|--reuse-messag|--reuse-message)
@@ -243,32 +208,26 @@ $1"
log_given=t$log_given
use_commit="$1"
no_edit=t
- shift
;;
-s|--s|--si|--sig|--sign|--signo|--signof|--signoff)
signoff=t
- shift
;;
-t|--t|--te|--tem|--temp|--templ|--templa|--templat|--template)
case "$#" in 1) usage ;; esac
shift
templatefile="$1"
no_edit=
- shift
;;
-q|--q|--qu|--qui|--quie|--quiet)
quiet=t
- shift
;;
-v|--v|--ve|--ver|--verb|--verbo|--verbos|--verbose)
verbose=t
- shift
;;
-u|--u|--un|--unt|--untr|--untra|--untrac|--untrack|--untracke|\
--untracked|--untracked-|--untracked-f|--untracked-fi|--untracked-fil|\
--untracked-file|--untracked-files)
untracked_files=t
- shift
;;
--)
shift
@@ -281,6 +240,7 @@ $1"
break
;;
esac
+ shift
done
case "$edit_flag" in t) no_edit= ;; esac
@@ -441,12 +401,8 @@ esac
if test t = "$verify" && test -x "$GIT_DIR"/hooks/pre-commit
then
- if test "$TMP_INDEX"
- then
- GIT_INDEX_FILE="$TMP_INDEX" "$GIT_DIR"/hooks/pre-commit
- else
- GIT_INDEX_FILE="$USE_INDEX" "$GIT_DIR"/hooks/pre-commit
- fi || exit
+ GIT_INDEX_FILE="${TMP_INDEX:-${USE_INDEX}}" "$GIT_DIR"/hooks/pre-commit \
+ || exit
fi
if test "$log_message" != ''
--
1.5.3.1.96.g4568
^ permalink raw reply related
* testsuite problems
From: Miklos Vajna @ 2007-09-17 21:17 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 1290 bytes --]
hello,
i was running 'make test' on v1.5.3.1-109-gacd6917 and i hit the
following issue:
* ok 6: validate file modification time
* ok 7: git get-tar-commit-id
* ok 8: extract tar archive
* ok 9: validate filenames
* ok 10: validate file contents
* ok 11: git tar-tree with prefix
* ok 12: extract tar archive with prefix
* ok 13: validate filenames with prefix
* ok 14: validate file contents with prefix
* ok 15: create an archive with a substfile
* ok 16: extract substfile
* ok 17: validate substfile contents
* ok 18: git archive --format=zip
* ok 19: extract ZIP archive
* ok 20: validate filenames
* FAIL 21: validate file contents
diff -r a d/a
* ok 22: git archive --format=zip with prefix
* ok 23: extract ZIP archive with prefix
* ok 24: validate filenames with prefix
* FAIL 25: validate file contents with prefix
diff -r a e/prefix/a
* ok 26: git archive --list outside of a git repo
* failed 2 among 26 test(s)
make[1]: *** [t5000-tar-tree.sh] Error 1
make[1]: Leaving directory `/home/vmiklos/git/git/t'
make: *** [test] Error 2
$ tar --version
tar (GNU tar) 1.18
$ diff --version
diff (GNU diffutils) 2.8.1
maybe these versions are too new or old? if i missed any needed info,
please let me know.
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: Pierre Habouzit @ 2007-09-17 21:41 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20070917211742.GF19019@genesis.frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 2049 bytes --]
On Mon, Sep 17, 2007 at 09:17:42PM +0000, Miklos Vajna wrote:
> hello,
>
> i was running 'make test' on v1.5.3.1-109-gacd6917 and i hit the
> following issue:
>
[...]
> * FAIL 21: validate file contents
> diff -r a d/a
> * ok 22: git archive --format=zip with prefix
> * ok 23: extract ZIP archive with prefix
> * ok 24: validate filenames with prefix
> * FAIL 25: validate file contents with prefix
> diff -r a e/prefix/a
> * ok 26: git archive --list outside of a git repo
> * failed 2 among 26 test(s)
> make[1]: *** [t5000-tar-tree.sh] Error 1
> make[1]: Leaving directory `/home/vmiklos/git/git/t'
> make: *** [test] Error 2
Well, it works for me:
$ git describe && git st
v1.5.3.1-109-gacd6917
# Not currently on any branch.
nothing to commit (working directory clean)
$ cd t && ./t5000-tar-tree.sh
* ok 1: populate workdir
* ok 2: add files to repository
* ok 3: git archive
* ok 4: git tar-tree
* ok 5: git archive vs. git tar-tree
* ok 6: validate file modification time
* ok 7: git get-tar-commit-id
* ok 8: extract tar archive
* ok 9: validate filenames
* ok 10: validate file contents
* ok 11: git tar-tree with prefix
* ok 12: extract tar archive with prefix
* ok 13: validate filenames with prefix
* ok 14: validate file contents with prefix
* ok 15: create an archive with a substfile
* ok 16: extract substfile
* ok 17: validate substfile contents
* ok 18: git archive --format=zip
* ok 19: extract ZIP archive
* ok 20: validate filenames
* ok 21: validate file contents
* ok 22: git archive --format=zip with prefix
* ok 23: extract ZIP archive with prefix
* ok 24: validate filenames with prefix
* ok 25: validate file contents with prefix
* ok 26: git archive --list outside of a git repo
* passed all 26 test(s)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: René Scharfe @ 2007-09-17 21:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20070917211742.GF19019@genesis.frugalware.org>
Miklos Vajna schrieb:
> * ok 18: git archive --format=zip
> * ok 19: extract ZIP archive
> * ok 20: validate filenames
> * FAIL 21: validate file contents
> diff -r a d/a
> * ok 22: git archive --format=zip with prefix
> * ok 23: extract ZIP archive with prefix
> * ok 24: validate filenames with prefix
> * FAIL 25: validate file contents with prefix
> diff -r a e/prefix/a
> * ok 26: git archive --list outside of a git repo
> * failed 2 among 26 test(s)
> make[1]: *** [t5000-tar-tree.sh] Error 1
> make[1]: Leaving directory `/home/vmiklos/git/git/t'
> make: *** [test] Error 2
>
> $ tar --version
> tar (GNU tar) 1.18
>
> $ diff --version
> diff (GNU diffutils) 2.8.1
>
> maybe these versions are too new or old? if i missed any needed info,
> please let me know.
The failing tests check ZIP file creation, not tar file creation.
Perhaps your unzip command works a bit differently from Info-ZIP's?
Also, what is the difference between t/trash/a and t/trash/d/a after
running the test script (that's what test 21 is comparing)?
René
^ permalink raw reply
* Re: testsuite problems
From: Miklos Vajna @ 2007-09-17 22:04 UTC (permalink / raw)
To: René Scharfe; +Cc: git
In-Reply-To: <46EEF6A8.1030308@lsrfire.ath.cx>
[-- Attachment #1: Type: text/plain, Size: 821 bytes --]
On Mon, Sep 17, 2007 at 11:50:32PM +0200, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> The failing tests check ZIP file creation, not tar file creation.
> Perhaps your unzip command works a bit differently from Info-ZIP's?
$ zip -v
Copyright (c) 1990-2006 Info-ZIP - Type 'zip "-L"' for software license.
This is Zip 2.32 (June 19th 2006), by Info-ZIP.
should this version be ok?
> Also, what is the difference between t/trash/a and t/trash/d/a after
> running the test script (that's what test 21 is comparing)?
$ diff -Naur t/trash/a t/trash/d/a
diff -Naur t/trash/a/l1 t/trash/d/a/l1
--- t/trash/a/l1 2007-09-17 23:10:03.000000000 +0200
+++ t/trash/d/a/l1 2007-09-17 23:10:03.000000000 +0200
@@ -1 +1 @@
-simple textfile
+a
\ No newline at end of file
thanks,
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Johannes Schindelin @ 2007-09-17 22:34 UTC (permalink / raw)
To: gitster, git
git-am used "git apply -z --index-info" to find the original versions
of the files touched by the diff, to be able to do an inexpensive
three-way merge.
This operation makes only sense in a repository, since the index
information in the diff refers to blobs, which have to be present in
the current repository.
Therefore, teach "git apply" a mode to write out the result as an
index file to begin with, obviating the need for scripts to do it
themselves.
The primary user for --index-info is "git am", which is converted to
use --build-fake-ancestor in this patch.
Suggested by Junio.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Suggested by Junio, but the errors are all mine.
I am not quite certain, though, if there is really no porcelain
using that option. It has been around since Oct 7 2005 (!), so
there is a real chance that StGit, guilt or QGit use it. In that
case, this patch is obviously wrong.
Documentation/git-apply.txt | 11 +++++++----
builtin-apply.c | 35 ++++++++++++++++++++++-------------
git-am.sh | 6 ++----
3 files changed, 31 insertions(+), 21 deletions(-)
diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index 4c7e3a2..c1c54bf 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git-apply' [--stat] [--numstat] [--summary] [--check] [--index]
- [--apply] [--no-add] [--index-info] [-R | --reverse]
+ [--apply] [--no-add] [--build-fake-ancestor <file>] [-R | --reverse]
[--allow-binary-replacement | --binary] [--reject] [-z]
[-pNUM] [-CNUM] [--inaccurate-eof] [--cached]
[--whitespace=<nowarn|warn|error|error-all|strip>]
@@ -63,12 +63,15 @@ OPTIONS
cached data, apply the patch, and store the result in the index,
without using the working tree. This implies '--index'.
---index-info::
+--build-fake-ancestor <file>::
Newer git-diff output has embedded 'index information'
for each blob to help identify the original version that
the patch applies to. When this flag is given, and if
- the original version of the blob is available locally,
- outputs information about them to the standard output.
+ the original versions of the blobs is available locally,
+ builds a temporary index containing those blobs.
++
+When a pure mode change is encountered (which has no index information),
+the information is read from the current index instead.
-R, --reverse::
Apply the patch in reverse.
diff --git a/builtin-apply.c b/builtin-apply.c
index 938fd61..1372169 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -42,7 +42,7 @@ static int apply_in_reverse;
static int apply_with_reject;
static int apply_verbosely;
static int no_add;
-static int show_index_info;
+static const char *fake_ancestor;
static int line_termination = '\n';
static unsigned long p_context = ULONG_MAX;
static const char apply_usage[] =
@@ -2236,9 +2236,12 @@ static int get_current_sha1(const char *path, unsigned char *sha1)
return 0;
}
-static void show_index_list(struct patch *list)
+/* Build an index that contains the just the files needed for a 3way merge */
+static void build_fake_ancestor(struct patch *list, const char *filename)
{
struct patch *patch;
+ struct index_state result = { 0 };
+ int fd;
/* Once we start supporting the reverse patch, it may be
* worth showing the new sha1 prefix, but until then...
@@ -2246,11 +2249,12 @@ static void show_index_list(struct patch *list)
for (patch = list; patch; patch = patch->next) {
const unsigned char *sha1_ptr;
unsigned char sha1[20];
+ struct cache_entry *ce;
const char *name;
name = patch->old_name ? patch->old_name : patch->new_name;
if (0 < patch->is_new)
- sha1_ptr = null_sha1;
+ continue;
else if (get_sha1(patch->old_sha1_prefix, sha1))
/* git diff has no index line for mode/type changes */
if (!patch->lines_added && !patch->lines_deleted) {
@@ -2265,13 +2269,16 @@ static void show_index_list(struct patch *list)
else
sha1_ptr = sha1;
- printf("%06o %s ",patch->old_mode, sha1_to_hex(sha1_ptr));
- if (line_termination && quote_c_style(name, NULL, NULL, 0))
- quote_c_style(name, NULL, stdout, 0);
- else
- fputs(name, stdout);
- putchar(line_termination);
+ ce = make_cache_entry(patch->old_mode, sha1_ptr, name, 0, 0);
+ if (add_index_entry(&result, ce, ADD_CACHE_OK_TO_ADD))
+ die ("Could not add %s to temporary index", name);
}
+
+ fd = open(filename, O_WRONLY | O_CREAT, 0666);
+ if (fd < 0 || write_index(&result, fd) || close(fd))
+ die ("Could not write temporary index to %s", filename);
+
+ discard_index(&result);
}
static void stat_patch_list(struct patch *patch)
@@ -2791,8 +2798,8 @@ static int apply_patch(int fd, const char *filename, int inaccurate_eof)
if (apply && write_out_results(list, skipped_patch))
exit(1);
- if (show_index_info)
- show_index_list(list);
+ if (fake_ancestor)
+ build_fake_ancestor(list, fake_ancestor);
if (diffstat)
stat_patch_list(list);
@@ -2900,9 +2907,11 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
apply = 1;
continue;
}
- if (!strcmp(arg, "--index-info")) {
+ if (!strcmp(arg, "--build-fake-ancestor")) {
apply = 0;
- show_index_info = 1;
+ if (++i >= argc)
+ die ("need a filename");
+ fake_ancestor = argv[i];
continue;
}
if (!strcmp(arg, "-z")) {
diff --git a/git-am.sh b/git-am.sh
index 4db4701..8340125 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -62,10 +62,8 @@ fall_back_3way () {
mkdir "$dotest/patch-merge-tmp-dir"
# First see if the patch records the index info that we can use.
- git apply -z --index-info "$dotest/patch" \
- >"$dotest/patch-merge-index-info" &&
- GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
- git update-index -z --index-info <"$dotest/patch-merge-index-info" &&
+ git apply --build-fake-ancestor "$dotest/patch-merge-tmp-index" \
+ "$dotest/patch" &&
GIT_INDEX_FILE="$dotest/patch-merge-tmp-index" \
git write-tree >"$dotest/patch-merge-base+" ||
cannot_fallback "Repository lacks necessary blobs to fall back on 3-way merge."
--
1.5.3.1.989.g2059
^ permalink raw reply related
* Re: testsuite problems
From: Johannes Schindelin @ 2007-09-17 22:40 UTC (permalink / raw)
To: Miklos Vajna; +Cc: René Scharfe, git
In-Reply-To: <20070917220408.GG19019@genesis.frugalware.org>
Hi,
On Tue, 18 Sep 2007, Miklos Vajna wrote:
> On Mon, Sep 17, 2007 at 11:50:32PM +0200, Ren? Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> > The failing tests check ZIP file creation, not tar file creation.
> > Perhaps your unzip command works a bit differently from Info-ZIP's?
>
> $ zip -v
> Copyright (c) 1990-2006 Info-ZIP - Type 'zip "-L"' for software license.
> This is Zip 2.32 (June 19th 2006), by Info-ZIP.
>
> should this version be ok?
>
> > Also, what is the difference between t/trash/a and t/trash/d/a after
> > running the test script (that's what test 21 is comparing)?
>
> $ diff -Naur t/trash/a t/trash/d/a
> diff -Naur t/trash/a/l1 t/trash/d/a/l1
> --- t/trash/a/l1 2007-09-17 23:10:03.000000000 +0200
> +++ t/trash/d/a/l1 2007-09-17 23:10:03.000000000 +0200
> @@ -1 +1 @@
> -simple textfile
> +a
> \ No newline at end of file
Ah, that's my friend GNU diff again... Try running the test with "-i -v",
and it will pass, if I am right.
I actually made a patch for GNU diff back when this hit me, only to find
out that it was fixed in CVS. Apparently, for a short period, during
which 2.8.1 was released, there was a bug that prevented GNU diff from
working properly when the output was redirected to a non-tty.
If I'm right, and it is that bug, it might be worthwhile to convert the
calls to "diff -u" in that test script with calls to "git diff" (possibly
with --no-index).
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Junio C Hamano @ 2007-09-17 22:42 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0709172330400.28586@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Suggested by Junio, but the errors are all mine.
>
> I am not quite certain, though, if there is really no porcelain
> using that option. It has been around since Oct 7 2005 (!), so
> there is a real chance that StGit, guilt or QGit use it. In that
> case, this patch is obviously wrong.
You do not have to do the deprecating/removing part if that is
the issue.
^ permalink raw reply
* Re: testsuite problems
From: Miklos Vajna @ 2007-09-17 22:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: René Scharfe, git
In-Reply-To: <Pine.LNX.4.64.0709172337470.28586@racer.site>
[-- Attachment #1: Type: text/plain, Size: 461 bytes --]
On Mon, Sep 17, 2007 at 11:40:27PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Ah, that's my friend GNU diff again... Try running the test with "-i -v",
> and it will pass, if I am right.
* expecting success: diff -r a d/a
diff -r a/l1 d/a/l1
1c1
< simple textfile
---
> a
\ No newline at end of file
* FAIL 21: validate file contents
diff -r a d/a
it still fails, so i don't think this is that issue
- VMiklos
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: testsuite problems
From: Johannes Schindelin @ 2007-09-17 22:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: René Scharfe, git
In-Reply-To: <20070917224828.GI19019@genesis.frugalware.org>
Hi,
On Tue, 18 Sep 2007, Miklos Vajna wrote:
> On Mon, Sep 17, 2007 at 11:40:27PM +0100, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Ah, that's my friend GNU diff again... Try running the test with "-i -v",
> > and it will pass, if I am right.
>
> * expecting success: diff -r a d/a
> diff -r a/l1 d/a/l1
> 1c1
> < simple textfile
> ---
> > a
> \ No newline at end of file
> * FAIL 21: validate file contents
> diff -r a d/a
>
> it still fails, so i don't think this is that issue
Oops. I blame it on tiredness...
Sorry for the noise,
Dscho
^ permalink raw reply
* List of bugs and suggestions
From: Yann Dirson @ 2007-09-17 22:54 UTC (permalink / raw)
To: GIT list
Here are a list of issues I wrote down while using git and friends at
work, but did not take time to report of fix myself before. They are
relative to git 1.5.3.
BUGS
gitk:
- when selecting again a "local changes" pseudo-patch, gitk re-reads
the diff, and the results can be confusing if HEAD changed since last
full reload (eg. git-commit or stg/guilt push/pop). Should detect the
HEAD change and propose a reload.
- clicking in diff-contents or file-list panes does not move focus out
of text fields (must click eg. between fields instead). I remember
routinely click in diff-contents to focus out of the search field.
- if a file that is modified and copied in the same commit, clicking
on the original in file-list pane jumps to the "copy" entry. This can
easily cause overlooking of changes to the original file.
stash:
- "git stash show junk" acts like "git stash show"
- changes stashed in the index are unstashed only in workdir. If
that's intended, a note in the "stash apply" doc would be useful; but
then I am left puzzled as to the usefulness of storing the index state
in the stash branch: it could surely be applied by a variant of "stash
apply", but such a command does not seem to exist yet ?
misc git:
- git has problems with cascaded alternates that use relative paths.
"git clone -s A B && git clone -s B C" puts absolute paths in
info/alternates - there is probably a good reason, but it will
obviously hurt when moving the clone. Further more,
Documentation/repository-layout.txt says scary things about
using absolute paths there. But if one manually sets up
alternates in a set of repositories (easily reproduced by
hand-editing the alternates files in the B clone
described above), one ends with commits from A not being seen,
and the following error message on each command run with C,
even when no object lookup from A is required:
| $ git fsck
| error: .git/objects/../../../B/.git/objects: ignoring relative alternate object store ../../../A/.git/objects
- repack -a -l (after transitionning a repo to alternates) does not
detect that old packs including objects now in alternate.
I had 2 branches in a single repo and changed the setup to 2
repos (initialize by "cp -al" and removing unwanted refs from
each repo), A finally referencing B as remote and alternate.
"git repack -a -l" in A seems to correctly only include only
local objects in the pack, but all the old packs containing
objects now in B are kept.
SUGGESTIONS
git-diff*:
- could allow to declare word separators for --color-words
- could print hunk number in header (a la filterdiff --annotate)
gitk:
- on-demand loading of additional revs (eg. ancestors from reflogs)
- non-printable keystrokes could be made available from within text
fields (F5, PgUp, etc)
- large commits would be made easier to navigate with some additionnal
highlighting/coloring. Eg:
- coloring of add/remove (and copy/move ?) in patch file list
- move selection in patch file list according to currently selected
search match when there is one, or according to the file(s) whose
diff currently appears
- highlight search matches in patch file list
- highlight search matches, and the file selected from the patch file
list, in/beside the patch display slider
- file list could have its own focus history
- for large commits, it takes time to get the full diff. Showing the
progress (nfiles processed / nfiles in commit) would be nice
- a "goto prev file" binding to reverse 'f' key (or quickly find in
which file a search match occured) would be great
- a "scroll diff view to next hunk" key binding ('h' ?)
- search backwards, and case-insensitively
- a list of known heads to jump to would be useful to quickly navigate
in multiple-head display mode.
- selecting a file in file list xould put filename in paste buffer,
like what's done for commit ids in history pane.
- "lock selected diff" toggle, to avoid losing a particular diff by
error (esp. useful when getting an expensive diff relatively to
selected commit)
stgit:
- derive a stack-log from patch logs
The ordering information provided by patchlogs and starck
reflog could be used to present a history of what happenned in
a stack (eg. when coming back from holiday wondering what one
was doing before leaving).
stgit contrib scripts (mostly reminders to myself ;):
- stg-whatchanged does not identify conflicts caused by "stg pick --fold"
- stg-fold-files-from cannot fold binary files (filterdiff limitation)
=> the following only works for filenames with no special char
(would need --zero):
git show --binary $(stg id occ53) -- $(stg files --bare occ53|grep '^doc/') | stg fold
=> the following does not work either:
git show --binary $(stg id occ53) -- $(git-ls-tree --name-only $(stg id occ53) doc/)
- stg-fold-files-from using non-git-aware filterdiff causes git-apply to
mistake a "add content to an empty file" hunk for a "create file" hunk
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Kristian Høgsberg @ 2007-09-17 22:58 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061741370.28586@racer.site>
On Thu, 2007-09-06 at 17:59 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > contrib/examples/git-commit.sh | 665 +++++++++++++++++++++++++++++++++++
> > git-commit.sh | 665 -----------------------------------
>
> You might want to use "git format-patch -M" next time ;-)
>
> > @@ -357,7 +358,6 @@ BUILTIN_OBJS = \
> > builtin-rev-parse.o \
> > builtin-revert.o \
> > builtin-rm.o \
> > - builtin-runstatus.o \
>
> Better keep it; some people's scripts could depend on it.
Seriously? Why don't we remove it and see if somebody yells? It's more
of an implementation detail than most other git commands; if you need
status output in your script why wouldn't you just run git status?
> > +struct option {
> > + enum option_type type;
> > + const char *long_name;
> > + char short_name;
> > + void *value;
> > +};
> > +
> > +static int scan_options(const char ***argv, struct option *options)
> > +{
>
> I would not (no longer, anyway) be opposed to replacing the option parsing
> in git with getopt(); I hear that it is small enough to keep a copy in
> compat/getopt.c.
>
> But let's go forward with builtin-commit; getopt() can come later.
I don't know. I think it's a situation much like the string library
discussion. It's a small enough dependency (70 lines!) that there's no
gain in depending on an external implementation, and we can tailor it to
gits needs as we extend the use within git. And we can call it gitopt!
> > +static char *
> > +prepare_index(const char **files, const char *prefix)
> > +{
> > + int fd;
> > + struct tree *tree;
> > + struct lock_file *next_index_lock;
> > +
> > + fd = hold_locked_index(&lock_file, 1);
> > + if (read_cache() < 0)
> > + die("index file corrupt");
> > +
> > + if (all) {
> > + add_files_to_cache(fd, files, NULL);
> > + return lock_file.filename;
> > + } else if (also) {
> > + add_files_to_cache(fd, files, prefix);
> > + return lock_file.filename;
> > + }
> > +
> > + if (interactive)
> > + interactive_add();
> > +
> > + if (*files == NULL) {
> > + /* Commit index as-is. */
> > + rollback_lock_file(&lock_file);
> > + return get_index_file();
> > + }
> > +
> > + /*
> > + * FIXME: Warn on unknown files. Shell script does
> > + *
> > + * commit_only=`git-ls-files --error-unmatch -- "$@"`
> > + */
> > +
> > + /*
> > + * FIXME: shell script does
> > + *
> > + * git-read-tree --index-output="$TMP_INDEX" -i -m HEAD
> > + *
> > + * which warns about unmerged files in the index.
> > + */
> > +
> > + /* update the user index file */
> > + add_files_to_cache(fd, files, prefix);
>
> I suspect this, or ...
>
> > +
> > + if (!initial_commit) {
> > + tree = parse_tree_indirect(head_sha1);
> > + if (!tree)
> > + die("failed to unpack HEAD tree object");
> > + if (read_tree(tree, 0, NULL))
> > + die("failed to read HEAD tree object");
> > + }
> > +
> > + /* Uh oh, abusing lock_file to create a garbage collected file */
> > + next_index_lock = xmalloc(sizeof(*next_index_lock));
> > + fd = hold_lock_file_for_update(next_index_lock,
> > + git_path("next-index-%d", getpid()), 1);
> > + add_files_to_cache(fd, files, prefix);
>
> ... this, but not both.
No, this needs both. The first add_files_to_cache() call updates the
user index (.git/index) by adding the given files, then we build a
temporary index from a tree and add the files to that index.
>
> > +/* Find out if the message starting at position 'start' in the strbuf
> > + * contains only whitespace and Signed-off-by lines. */
> > +static int message_is_empty(struct strbuf *sb, int start)
> > +{
> > + static const char signed_off_by[] = "Signed-off-by: ";
>
> I think you already defined that globally earlier.
Ah, yes, fixed.
> In the function message_is_empty() you write:
>
> > + /* See if the template is just a prefix of the message. */
> > + strbuf_init(&tmpl);
> > + if (template_file && strbuf_read_path(&tmpl, template_file) > 0) {
> > + stripspace(&tmpl, 1);
> > + if (start + tmpl.len <= sb->len &&
> > + memcmp(tmpl.buf, sb->buf + start, tmpl.len) == 0)
> > + start += tmpl.len;
>
> Could we not bail out here, if there is no match? In that case, the
> message is clearly not empty...
The contents could be just sign-off-by's.
> > + /* Check if the rest is just whitespace and Signed-of-by's. */
> > + for (i = start; i < sb->len; i++) {
> > + nl = memchr(sb->buf + i, '\n', sb->len - i);
> > + if (nl)
> > + eol = nl - sb->buf;
> > + else
> > + eol = sb->len;
>
> Why not just "if (isspace(sb->buf[i]) || sb->buf[i] == '\n') continue;"?
> This would also catch the cases where people indent their S-O-Bs.
>
> > +
> > + if (strlen(signed_off_by) <= eol - i &&
> > + !prefixcmp(sb->buf + i, signed_off_by)) {
> > + i = eol;
> > + continue;
> > + }
> > + while (i < eol)
> > + if (!isspace(sb->buf[i++]))
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
>
> I did not review the rest of the code closely yet...
I'm sending an updated version against Pierre's strbuf changes now.
It's a smaller patch set, so hopefully we can get it in soon.
Kristian
^ permalink raw reply
* Re: [PATCH 6/9] Rewrite launch_editor, create_tag and stripspace to use strbufs.
From: Kristian Høgsberg @ 2007-09-17 22:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061732480.28586@racer.site>
On Thu, 2007-09-06 at 17:38 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > diff --git a/strbuf.c b/strbuf.c
> > index fcfc05e..ed2afea 100644
> > --- a/strbuf.c
> > +++ b/strbuf.c
> > @@ -73,43 +74,15 @@ void strbuf_printf(struct strbuf *sb, const char *fmt, ...)
> > {
> > char buffer[2048];
> > va_list args;
> > - int len, size = 2 * sizeof buffer;
> > + int len;
> >
> > va_start(args, fmt);
> > len = vsnprintf(buffer, sizeof(buffer), fmt, args);
> > va_end(args);
> >
> > - if (len > sizeof(buffer)) {
> > - /*
> > - * Didn't fit in the buffer, but this vsnprintf at
> > - * least gives us the required length back. Grow the
> > - * buffer acccordingly and try again.
> > - */
> > - strbuf_grow(sb, len);
> > - va_start(args, fmt);
> > - len = vsnprintf(sb->buf + sb->len,
> > - sb->alloc - sb->len, fmt, args);
> > - va_end(args);
> > - } else if (len >= 0) {
> > - /*
> > - * The initial vsnprintf fit in the temp buffer, just
> > - * copy it to the strbuf.
> > - */
> > - strbuf_add(sb, buffer, len);
> > - } else {
> > - /*
> > - * This vnsprintf sucks and just returns -1 when the
> > - * buffer is too small. Keep doubling the size until
> > - * it fits.
> > - */
> > - while (len < 0) {
> > - strbuf_grow(sb, size);
> > - va_start(args, fmt);
> > - len = vsnprintf(sb->buf + sb->len,
> > - sb->alloc - sb->len, fmt, args);
> > - va_end(args);
> > - size *= 2;
> > - }
> > - }
> > + if (len > sizeof(buffer) || len < 0)
> > + die("out of buffer space\n");
> > +
> > + strbuf_add(sb, buffer, len);
> > }
>
> Really?
>
> (If you find the time, it would be really nice to rebase that patch series
> on top of Pierre's strbuf work...)
Argh, this was a screwup when I edited the patch series. The next
series is based on Pierres changes.
Kristian
^ permalink raw reply
* Re: [PATCH] apply: get rid of --index-info in favor of --build-fake-ancestor
From: Johannes Schindelin @ 2007-09-17 23:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7imozzgm.fsf@gitster.siamese.dyndns.org>
Hi,
On Mon, 17 Sep 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Suggested by Junio, but the errors are all mine.
> >
> > I am not quite certain, though, if there is really no porcelain
> > using that option. It has been around since Oct 7 2005 (!), so
> > there is a real chance that StGit, guilt or QGit use it. In that
> > case, this patch is obviously wrong.
>
> You do not have to do the deprecating/removing part if that is
> the issue.
I'd rather avoid keeping this code, if nobody uses it anyway...
So I cloned StGit and guilt (the two porcelains that I feel are most used,
apart from what is in git.git, and I'm too lazy to find that mail
analysing the recent Git survey). Neither of them has any --index-info in
their complete history.
So I'm more confident now that there is no need to keep --index-info.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/9] Introduce entry point for launching add--interactive.
From: Kristian Høgsberg @ 2007-09-17 23:13 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0709061729270.28586@racer.site>
On Thu, 2007-09-06 at 17:31 +0100, Johannes Schindelin wrote:
> Hi,
>
> On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
>
> > diff --git a/builtin-add.c b/builtin-add.c
> > index 3dd4ded..e79e8f7 100644
> > --- a/builtin-add.c
> > +++ b/builtin-add.c
> > @@ -153,6 +154,13 @@ static int git_add_config(const char *var, const char *value)
> > return git_default_config(var, value);
> > }
> >
> > +int interactive_add(void)
> > +{
> > + const char *argv[2] = { "add--interactive", NULL };
> > +
> > + return run_command_v_opt(argv, RUN_GIT_CMD);
> > +}
>
> I'd rather have this in builtin-commit.c, since it is quite funny if
> builtin-add.c has code to fork() and exec() itself (eventually, that
> is) ;-)
Huh... it ends up in the same binary, and interactive_add() sounds like
it should live in builtin-add.c rather than builtin-commit.c. Either
way, I don't care too much, but can we fix it up later?
> > diff --git a/commit.h b/commit.h
> > index 467872e..64e1d4b 100644
> > --- a/commit.h
> > +++ b/commit.h
> > @@ -122,4 +122,13 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,
> > int depth, int shallow_flag, int not_shallow_flag);
> >
> > int in_merge_bases(struct commit *, struct commit **, int);
> > +
> > +extern const unsigned char *
> > +create_commit(const unsigned char *tree_sha1,
> > + unsigned char parent_sha1[][20], int parents,
> > + const char *author_info, const char *committer_info,
> > + const char *message, int length);
> > +
> > +extern int interactive_add(void);
> > +
>
> Just a guess: you did not want create_commit() to creep in here, right?
Yeah, that was another oversight, fixed in the next series.
Kristian
^ permalink raw reply
* Re: [PATCH 9/9] Implement git commit as a builtin command.
From: Johannes Schindelin @ 2007-09-17 23:16 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <1190069881.10112.10.camel@hinata.boston.redhat.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1800 bytes --]
Hi,
On Mon, 17 Sep 2007, Kristian H?gsberg wrote:
> On Thu, 2007-09-06 at 17:59 +0100, Johannes Schindelin wrote:
> >
> > On Wed, 5 Sep 2007, Kristian Høgsberg wrote:
> >
> > > - builtin-runstatus.o \
> >
> > Better keep it; some people's scripts could depend on it.
>
> Seriously? Why don't we remove it and see if somebody yells? It's more
> of an implementation detail than most other git commands; if you need
> status output in your script why wouldn't you just run git status?
git status is deemed porcelain.
Yes, we recently converted a few things to use "git log", which is
porcelain, too, and I was not happy...
But then, you're right, we could just go and break peoples' scripts, if
they indeed did not use "git diff" directly.
> > > +struct option {
> > > + enum option_type type;
> > > + const char *long_name;
> > > + char short_name;
> > > + void *value;
> > > +};
> > > +
> > > +static int scan_options(const char ***argv, struct option *options)
> > > +{
> >
> > I would not (no longer, anyway) be opposed to replacing the option parsing
> > in git with getopt(); I hear that it is small enough to keep a copy in
> > compat/getopt.c.
> >
> > But let's go forward with builtin-commit; getopt() can come later.
>
> I don't know. I think it's a situation much like the string library
> discussion. It's a small enough dependency (70 lines!) that there's no
> gain in depending on an external implementation, and we can tailor it to
> gits needs as we extend the use within git. And we can call it gitopt!
Hm. I liked the semantics of getopt better, but what the heck.
> I'm sending an updated version against Pierre's strbuf changes now. It's
> a smaller patch set, so hopefully we can get it in soon.
Yes, that would be good.
Thanks,
Dscho
^ 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