Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, Oct 28, 2013 at 3:46 AM, Johan Herland wrote:
> On Sun, Oct 27, 2013 at 8:04 PM, Christian Couder
> wrote:
>>
>> If "git commit" processes these arguments and puts the result in the
>> commit message file that is passed to the
>> prepare-commit-msg hook, then this hook can still get them from the
>> file and process them however it wants.
>>
>> And in most cases the processing could be the same as what is done by
>> the commit-msg hook when the user changes the "Fixes: xxx" and
>> "Stuffed-by: yyy" lines in the editor.
>>
>> So it would probably be easier for people customizing the
>> prepare-commit-msg and commit-msg if "git commit" processes the
>> arguments instead of just passing them to the prepare-commit-msg hook.
>>
>> And it will be better for people who don't set up any *commit-msg hook.
>> Even if there is no commit template, "-f Acked-by:Peff" and "-f
>> Fixes:security-bug" could still work.
>> I suspect most users don't setup any hook or commit template.
>
> Hmm. I'm not sure what you argue about which part of the system should
> perform which function. Let's examine the above options in more
> detail. Roughly, the flow of events look like this
>
> git commit -f ack:Peff -f fix:security-bug
> |
> v
> builtin/commit.c (i.e. inside "git commit")
> |
> v
> prepare-commit-msg hook
> |
> v
> commit message template:
> Fixes: security-bug
> Acked-by: Peff
Here it could already be:
Fixes: 1234beef56 (Commit message summmary)
Acked-by: Jeff King
Because builtin/commit.c hook could already have expanded everything.
> |
> v
> user edits commit message (may or may not change Fixes/Acked-by lines)
> |
> v
> commit-msg hook
> |
> v
> commit message:
> Fixes: 1234beef56 (Commit message summmary)
> Acked-by: Jeff King
>
> (The above is even a bit simplified, but I believe it's sufficient for
> the current discussion.) So, there are several expansions happening
> between the initial "git commit" and the final commit message. They
> are:
>
> 1. "fix" -> "Fixes: "
> 2. "security-bug" -> "1234beef56 (Commit message summmary)"
> 3. "ack" -> "Acked-by: "
> 4. "Peff" -> "Jeff King "
>
> First, I think we both agree that expansions #2 and #4 MUST be done by
> the commit-msg hook. The reason for this is two-fold: (a) the
> expansion must be done (at least) after the user has edited the commit
> message (since the values entered by the user might require the same
> expansion), and (b) how (and whether) to perform the expansion is a
> project-specific policy question, and not something that Git can
> dictate.
I don't agree. Git doesn't need to dictate anything to be able to do
these expansions.
Git only needs some hints to do these expansions properly and it could
just look at the commit template, or the config, to get those hints.
For example, if there is a "Acked-by:" line in the commit template,
then Git might decide that "ack" means "Acked-by", and then that "-by"
means that "Peff" should be related to an author, and then that it is
probably "Jeff King ".
> Obviously, common functionality can be made available in the
> default hook shipped by Git, but it's up to each project to enable
> and/or customize this.
>
> Second, there is #1 and #3, the expansion of "ack" -> "Acked-by:" and
> "fix" -> "Fixes:". Is this expansion performed by the
> prepare-commit-msg hook, or directly inside builtin/commit.c?
>
> If you are arguing for the latter (and I'm not sure that you are), we
> would need to add a dictionary to "git commit" that maps shorthand
> field names ("ack") to the RFC822 -style equivalent ("Acked-by: ").
Yes, I am arguing that builtin/commit.c, or better a plumbing command
launched by builtin/commit.c, should do it.
And I don't think there is an absolute need for a dictionary.
There could be such a dictionary in the config (as Junio proposed).
But if there isn't, the plumbing command launched by builtin/commit.c
could look at the commit template to decide that "ack" means
"Acked-by:".
> I would instead argue for the former, i.e. simply forwarding "ack" and
> "fix" as-is to the prepare-commit-msg hook, and let it deal with the
> appropriate expansion. The main reason for this is that if a project
> wants to add another shorthand expansion (e.g. "bug" ->
> "Related-Bugzilla-Id: "), they can do so without hacking
> builtin/commit.c.
I agree that there should be no need to hack builtin/commit.c.
For example if "Bugzilla-Id:" is in the commit template, the plumbing
command launched by builtin/commit.c would decide that "bug" means
"Bugzilla-Id:" without any hack.
Of course this suppose that there is no other "Bugtracker:" or "Bug:"
in the commit template.
But even in this case it would mean that users have to use "-f
bugz:XXX" (or "--trailer bugz=XXX") instead of just "-f bug:XXX".
>> Supporting project specific conventions/rules would still be possible
>> by processing lines in the commit message fil
Limiting disk usage
Hi, What can be done to limit amount of space occupied on clone and checkout? I know about shallow clone and sparse checkout, anything else? Preferably I would split repository into multiple repositories but I worry that working with multiple directories would be much more troublesome. But maybe I am wrong? I don't have much experience with submodules, repo tool (what else?) Thanks -- Piotr Krukowiecki -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
KANN ICH DEPONIEREN 18.000.000,00 GBP IN IHREM KONTO ?
Guten Tag ,
Bitte nehmen Sie meine aufrichtigen Entschuldigungen an, wenn meine E-mail Ihre
persönliche Ethik nicht trifft.
Für ich weiß, dass dies wie ein vollständiges Eindringen zu Ihrer Ruhe scheinen
kann, aber zurzeit ,dies ist meine Option für Kommunikation zu Ihnen.
Dies könnte fremd oder wahrscheinlich unwahr scheinen,wegen der Hoehe von
Ausschuss E-mail,die wir täglich empfangen, aber ich glaube, dass dies noch der
echteste Weg ist, einen wahren Charakter zu kontaktieren.Ich heisse Frau
Jacqueline Benett-Baggs Murchie {Eine Vereinigten Staaten von amerikanischer
Frau},eine Witwe zu Spätem Herrn Benett-Baggs Murchie {Mein Ehemann hat mit
einem öl refinary Lukoil in Russland für 31 Jahre gearbeitet} Vor seinem
tragischen Tod in Japan 11. März. 2011.
Mein Ehemann war auf einer Geschäftsreise nach Japan Wenn der hässliche
Vorfall, der ihn von mir weggenommen hat, gestreikt hat. Hier ist eine
Verbindung für Fotos der Katastrophe
{http://www.boston.com/bigpicture/2011/03/massive_earthquake_hits_japan.html}Ich
bin 74years alt und leide an lange Zeit Krebs von der Speiseröhre. Ich bin
jetzt im Krankenhaus {NCI Hospital PT 13717, Jalan BBN 2/1, Putra Nilai, Negeri
Sembilan, 71800], Malaysia.
Von allen Anzeigen verschlechtert sich meine Bedingung wirklich und es ist
ziemlich offensichtlich, dass ich mehr als zwei Monate {gemäß medizinischen
Berichten von einem Arzt}nicht leben werde.Dies ist, weil die Krebsphase zu
einer sehr schlechten Phase erreichen hat.Mein später Ehemann war sehr
wohlhabend und reich und nach seinem Tod,ich habe alle sein Geschäft und
Reichtum geerbt.
Mein Ehemann hat die Summe von £18.000.000,00 GBP mit einer Bank in Schottland
deponiert, Mein Ehemann und ich haben geplant, dieses Geld Zu Benutzen, Und
Öffnungs-höhe Eine Gewebefirma Vor Dem Japan Tsunami Krise. Mein Ehemann hat
mich der Nutznießer des Treuhandvermögens genannt, weil wir keine Kinder gehabt
haben.
Da ich nichts habe, für mehr zu leben.Der Arzt hat mir geraten,dass ich für
mehr als zwei Monate nicht leben kann,so habe ich mich jetzt entschieden, Teil
von diesem Reichtum zu teilen,zur Entwicklung von dem wenigen privilegierten
Leute in Europa beizutragen,da dies die Wunsch von meinem Ehemann Herrn
Benett-Baggs Murchie bevor seinem Tod ist.
Ich habe Behandlung für meinen ösophagealen Krebs erlebt. Ich habe, da meine
Fähigkeit verloren hat, zu reden, und meine Ärzte haben mir erzählt, dass ich
nur ein paar Wochen habe, zu leben.Des meines Ehemanns Familienwunsch mich
Verstorbene Um diesen Reichtum zu erlangen. Ich kann mit der Qual nicht leben,
von dieser riesigen Verantwortung zu welchem des meines späten Ehemanns
Familienmitglieds anzuvertrauen Weil sie Ungläubigen sind. Ich will diesen
Fonds, zu WOHLTÄTIGKEITEN ORGANISATIONEN verteilt zu werden. Bitte, Ich will
Sie, mir Vertreter zu helfen, als der Nutznießer, die Fonds von der Bank zu
sammeln und verteilen Sie zu Wohltätigkeit.
Ich bete ehrlich,dass dieses Geld,wenn es zu Ihnen überweist ist,sollen Sie
versichern,dass es für den gesagten Zweck benutzt werden muss.Weil ich darauf
gekommen bin,dass jene Reichtumerwerbung ohne Christus zu erfahren, ist
Eitelkeit auf Eitelkeit.Für Ihre Hilfe,habe ich 30% von diesem gesamten Geld
{£18.000.000,00}für Sie gelegt, auf Grund Ihrer persönlichen Bemühung, die Sie
für Ihren persönlichen Gebrauch sofort das Geld auf Sie überweist ist,
abziehenwerden.Zuletzt,ich bete und hoffe, dass wenn das Geld schließlich auf
Sie überweisen ist, werden Sie es umsichtig gemäß meinem Willen und Gott
benutzen {der 70% vom Geld zum wenigen privilegierten in Europa zu spenden}.
Ihres in Christus.
Frau Jacqueline Benett-Baggs Murchie
E-MAIL : jacquelinemurc...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
KANN ICH DEPONIEREN 18.000.000,00 GBP IN IHREM KONTO ?
Guten Tag ,
Bitte nehmen Sie meine aufrichtigen Entschuldigungen an, wenn meine E-mail Ihre
persönliche Ethik nicht trifft.
Für ich weiß, dass dies wie ein vollständiges Eindringen zu Ihrer Ruhe scheinen
kann, aber zurzeit ,dies ist meine Option für Kommunikation zu Ihnen.
Dies könnte fremd oder wahrscheinlich unwahr scheinen,wegen der Hoehe von
Ausschuss E-mail,die wir täglich empfangen, aber ich glaube, dass dies noch der
echteste Weg ist, einen wahren Charakter zu kontaktieren.Ich heisse Frau
Jacqueline Benett-Baggs Murchie {Eine Vereinigten Staaten von amerikanischer
Frau},eine Witwe zu Spätem Herrn Benett-Baggs Murchie {Mein Ehemann hat mit
einem öl refinary Lukoil in Russland für 31 Jahre gearbeitet} Vor seinem
tragischen Tod in Japan 11. März. 2011.
Mein Ehemann war auf einer Geschäftsreise nach Japan Wenn der hässliche
Vorfall, der ihn von mir weggenommen hat, gestreikt hat. Hier ist eine
Verbindung für Fotos der Katastrophe
{http://www.boston.com/bigpicture/2011/03/massive_earthquake_hits_japan.html}Ich
bin 74years alt und leide an lange Zeit Krebs von der Speiseröhre. Ich bin
jetzt im Krankenhaus {NCI Hospital PT 13717, Jalan BBN 2/1, Putra Nilai, Negeri
Sembilan, 71800], Malaysia.
Von allen Anzeigen verschlechtert sich meine Bedingung wirklich und es ist
ziemlich offensichtlich, dass ich mehr als zwei Monate {gemäß medizinischen
Berichten von einem Arzt}nicht leben werde.Dies ist, weil die Krebsphase zu
einer sehr schlechten Phase erreichen hat.Mein später Ehemann war sehr
wohlhabend und reich und nach seinem Tod,ich habe alle sein Geschäft und
Reichtum geerbt.
Mein Ehemann hat die Summe von £18.000.000,00 GBP mit einer Bank in Schottland
deponiert, Mein Ehemann und ich haben geplant, dieses Geld Zu Benutzen, Und
Öffnungs-höhe Eine Gewebefirma Vor Dem Japan Tsunami Krise. Mein Ehemann hat
mich der Nutznießer des Treuhandvermögens genannt, weil wir keine Kinder gehabt
haben.
Da ich nichts habe, für mehr zu leben.Der Arzt hat mir geraten,dass ich für
mehr als zwei Monate nicht leben kann,so habe ich mich jetzt entschieden, Teil
von diesem Reichtum zu teilen,zur Entwicklung von dem wenigen privilegierten
Leute in Europa beizutragen,da dies die Wunsch von meinem Ehemann Herrn
Benett-Baggs Murchie bevor seinem Tod ist.
Ich habe Behandlung für meinen ösophagealen Krebs erlebt. Ich habe, da meine
Fähigkeit verloren hat, zu reden, und meine Ärzte haben mir erzählt, dass ich
nur ein paar Wochen habe, zu leben.Des meines Ehemanns Familienwunsch mich
Verstorbene Um diesen Reichtum zu erlangen. Ich kann mit der Qual nicht leben,
von dieser riesigen Verantwortung zu welchem des meines späten Ehemanns
Familienmitglieds anzuvertrauen Weil sie Ungläubigen sind. Ich will diesen
Fonds, zu WOHLTÄTIGKEITEN ORGANISATIONEN verteilt zu werden. Bitte, Ich will
Sie, mir Vertreter zu helfen, als der Nutznießer, die Fonds von der Bank zu
sammeln und verteilen Sie zu Wohltätigkeit.
Ich bete ehrlich,dass dieses Geld,wenn es zu Ihnen überweist ist,sollen Sie
versichern,dass es für den gesagten Zweck benutzt werden muss.Weil ich darauf
gekommen bin,dass jene Reichtumerwerbung ohne Christus zu erfahren, ist
Eitelkeit auf Eitelkeit.Für Ihre Hilfe,habe ich 30% von diesem gesamten Geld
{£18.000.000,00}für Sie gelegt, auf Grund Ihrer persönlichen Bemühung, die Sie
für Ihren persönlichen Gebrauch sofort das Geld auf Sie überweist ist,
abziehenwerden.Zuletzt,ich bete und hoffe, dass wenn das Geld schließlich auf
Sie überweisen ist, werden Sie es umsichtig gemäß meinem Willen und Gott
benutzen {der 70% vom Geld zum wenigen privilegierten in Europa zu spenden}.
Ihres in Christus.
Frau Jacqueline Benett-Baggs Murchie
E-MAIL : jacquelinemurc...@gmail.com
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, Oct 28, 2013 at 10:08 AM, Junio C Hamano wrote:
>
> Thinking aloud further, what I had in mind was along the lines of
> the following.
>
> * The most generic external interface would be spelled as
>
> --trailer [=]
>
>where can be things like "signoff", "closes", "acked-by",
>"change-id", "fixes", etc.; they can be taken from an unbounded
>set. The historical "--signoff" can become a short-hand for
>"--trailer signoff". More than one "--trailer" option can be
>given on a single command line.
Ok, and maybe the could also be the full trailer like "Signed-off-by".
> * The token is used to look into the configuration, e.g.,
>
>[commitTrailer "signoff"]
> style = append-norepeat
> trailer = Signed-off-by
> command = echo "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"'
>
>[commitTrailer "change-id"]
> style = append-only-if-missing
> trailer = Change-Id
> command = 'git hash-object -t commit --stdin <$GIT_PROTO_COMMIT'
>
>[commitTrailer "fixes"]
> style = overwrite
> trailer = Fixes
> command = 'git log -1 --oneline --format="%h (%s)" --abbrev-commit=14
> $ARG'
>
>where
>
>- "commitTrailer..style" defines the interaction with
> existing trailer of the same kind (e.g. S-o-b: accumulates by
> appending, but we try not to repeat the same sign-off twice
> which would show you forwarding your own message you are the
> last person in the Sign-off chain; Fixes: if there is already
> one will remove the old one and replaces; etc.);
>
>- "commitTrailer..trailer" defines the trailer label at
> the beginning of the trailer line;
>
>- "commitTrailer..command" gives the command to run to
> obtain the payload after the "trailer" label. A handful
> obvious and useful variables are exported for the command to
> use, and is exported as $ARG, if present.
>
> With the most generic syntax, with the above commitTrailer.fixes.*
> configuration, I would imagine that you can say something like:
>
> git commit --trailer fixes="v2.6.12^{/^i386: tweak frobnitz}"
>
> to say that the first commit you find traversing the history of
> v2.6.12 whose title is "i386: tweak frobnitz" was faulty, and you
> are creating a commit that corrects its mistake.
>
> Giving some default configuration to often used trailer types
> (e.g. configuration for "--trailer signoff") and promoting some
> commonly used ones into a separate built-in option (e.g. an option
> "--signoff" that does not have to say "--trailer signoff") are
> entirely separate issues, and only time can nudge us into evaluating
> individual types of trailers.
Ok, and maybe, if there is no configuration for a trailer token, we
could look at the commit template.
Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] remote-curl: fix large pushes with GSSAPI
Due to an interaction between the way libcurl handles GSSAPI authentication over
HTTP and the way git uses libcurl, large pushes (those over http.postBuffer
bytes) would fail due to an authentication failure requiring a rewind of the
curl buffer. Such a rewind was not possible because the data did not fit into
the entire buffer.
Enable the use of the Expect: 100-continue header for large requests where the
server offers GSSAPI authentication to avoid this issue, since the request would
otherwise fail. This allows git to get the authentication data right before
sending the pack contents. Existing cases where pushes would succeed, including
small requests using GSSAPI, still disable the use of 100 Continue, as it causes
problems for some remote HTTP implementations (servers and proxies).
Signed-off-by: brian m. carlson
---
remote-curl.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/remote-curl.c b/remote-curl.c
index c9b891a..2c276de 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -449,6 +449,7 @@ static int post_rpc(struct rpc_state *rpc)
char *gzip_body = NULL;
size_t gzip_size = 0;
int err, large_request = 0;
+ int needs_100_continue = 0;
/* Try to load the entire request, if we can fit it into the
* allocated buffer space we can use HTTP/1.0 and avoid the
@@ -472,6 +473,8 @@ static int post_rpc(struct rpc_state *rpc)
}
if (large_request) {
+ long authtype = 0;
+
do {
err = probe_rpc(rpc);
if (err == HTTP_REAUTH)
@@ -479,11 +482,19 @@ static int post_rpc(struct rpc_state *rpc)
} while (err == HTTP_REAUTH);
if (err != HTTP_OK)
return -1;
+
+#if LIBCURL_VERSION_NUM >= 0x070a08
+ slot = get_active_slot();
+ curl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL,
&authtype);
+ if (authtype & CURLAUTH_GSSNEGOTIATE)
+ needs_100_continue = 1;
+#endif
}
headers = curl_slist_append(headers, rpc->hdr_content_type);
headers = curl_slist_append(headers, rpc->hdr_accept);
- headers = curl_slist_append(headers, "Expect:");
+ headers = curl_slist_append(headers, needs_100_continue ?
+ "Expect: 100-continue" : "Expect:");
retry:
slot = get_active_slot();
--
1.8.4.1.635.g6a5
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] remote-curl: fix large pushes with GSSAPI
On Sat, Oct 26, 2013 at 10:34:42PM +, brian m. carlson wrote: > Enable the use of the Expect: 100-continue header for large requests where the > server offers GSSAPI authentication to avoid this issue, since the request > would > otherwise fail. This allows git to get the authentication data right before > sending the pack contents. Existing cases where pushes would succeed, > including > small requests using GSSAPI, still disable the use of 100 Continue, as it > causes > problems for some remote HTTP implementations (servers and proxies). This iteration looks very reasonable to me. One minor nit: > + slot = get_active_slot(); > + curl_easy_getinfo(slot->curl, CURLINFO_HTTPAUTH_AVAIL, > &authtype); > + if (authtype & CURLAUTH_GSSNEGOTIATE) > + needs_100_continue = 1; According to curl_easy_getinfo(3), CURLINFO_HTTPAUTH_AVAIL was introduced in 7.10.8 (and it looks like CURLAUTH_GSSNEGOTIATE came earlier in 7.10.6). We should probably surround it with #if LIBCURL_VERSION_NUM >= 0x070a08 -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, Oct 28, 2013 at 12:29:32PM +0100, Johan Herland wrote: > > A hook-based solution could do this. But a built-in "all-purpose" > > handler like "footer.Fixes.arg=commit", which was intended to be > > reusable, wouldn't be able to do such footer-specific extra work without > > having to create new special cases in git each time. > > Which begs the question (posed to all, not specifically to you): Why > would we want solve this issue in config instead of in hooks? The > hooks will always be more flexible and less dependent on making > changes in git.git. (...a suitably flexible hook could even use the > config options discussed above as input...) In both cases, we need the > user to actively enable the functionality (either installing hooks, or > setting up config), and in both cases we could bundle Git with > defaults that solve the common cases, so that is not a useful > differentiator between the two approaches. I would even venture to > ask: If we end up solving this problem in config and not in hooks, > then why do we bother having hooks in the first place? One thing that is much nicer with config vs hooks is that you can manage config for all of your repositories by tweaking ~/.gitconfig (and that is where I would expect this type of config to go). Managing hooks globally means having each repo symlink to a central hook area, and having the forethought to set up the symlink farm and use init.templatedir before cloning any repos. We could probably make this friendlier by reading from ~/.githooks and defining some semantics for multiple hooks. E.g., fall back to ~/.githooks if the repo hook is not executable, or possibly run them both (or even allow multiple instances of a hook in ~/.githooks, which can help organization), and consider the hook a failure if any of them fail. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, Oct 28, 2013 at 11:10:13PM +0100, Thomas Rast wrote: > * In your list > > > Fixes: > > Reported-by: > > Suggested-by: > > Improved-by: > > Acked-by: > > Reviewed-by: > > Tested-by: > > Signed-off-by: > > and I might add > > Cherry-picked-from: > Reverts: > > if one were to phrase that as a footer/pseudoheader, observe that > there are only two kinds of these: footers that contain identities, > and footers that contain references to commits. I think people put other things in, too. For example, cross-referencing bug-tracker ids. In fact, if I saw "fixes: XXX", I would expect the latter to be a tracker id. People do this a lot with GitHub issues, because GitHub will auto-close issue 123 if a commit with "fixes #123" is pushed to master. Because of the "#", no pseudo-header is needed, but I have also seen people use the footer style (I don't have any examples on-hand, though). That being said, in your examples: > So why not support these use-cases? We could have something like > footer.foo.* configuration, e.g. > > [footer "fixes"] > type = commit > suggest = true > [footer "acked-by"] > type = identity you could easily have "type=text" to handle arbitrary text. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] git clone: is an URL local or ssh
On Mon, Oct 28, 2013 at 01:57:13PM -0700, Junio C Hamano wrote:
> > +i5601=0
> > +# $1 url
> > +# $2 none|host
> > +# $3 path
> > +test_clone_url () {
> > + i5601=$(($i5601 + 1))
> > + >"$TRASH_DIRECTORY/ssh-output" &&
> > + test_might_fail git clone "$1" tmp$i5601 &&
> > {
> > - case "$1" in
> > + case "$2" in
> > none)
> > ;;
> > *)
> > - echo "ssh: $1 git-upload-pack '$2'"
> > + echo "ssh: $2 git-upload-pack '$3'"
> > esac
> > } >"$TRASH_DIRECTORY/ssh-expect" &&
>
> This looks like a strange use of {} (not an issue this patch
> introduced, though). Shouldn't this suffice?
>
> case ... in
> ...
> esac >"$TRASH_DIRECTORY/ssh-expect"
Yeah, I think you are right. This one is my fault from 8d3d28f; I think
in an earlier draft I had more complex logic that needed the {}, and
then never dropped them when it got simplified.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] git clone: is an URL local or ssh
On Mon, Oct 28, 2013 at 09:16:19PM +0100, Torsten Bögershausen wrote:
> Commit 8d3d28f5 added test cases for URLs which should be ssh.
>
> Add more tests testing all the combinations:
> -IPv4 or IPv6
> -path starting with "/" or with "/~"
> -with and without the ssh:// scheme
>
> Add tests for ssh:// with port number.
>
> When a git repository "foo:bar" exist, git clone will call
> absolute_path() and git_connect() will be called with
> something like "/home/user/projects/foo:bar"
>
> Tighten the test and use "foo:bar" instead of "./foo:bar",
> refactor clear_ssh() and expect_ssh() into test_clone_url().
>
> "git clone foo/bar:baz" should not be ssh:
> Make the rule
> "if there is a slash before the first colon, it is not ssh"
> more strict by using the same is_local() in both connect.c
> and transport.c. Add a test case.
>
> Bug fixes for corner cases:
> - Handle clone of [::1]:/~repo correctly (2e7766655):
> Git should call "ssh ::1 ~repo", not ssh ::1 /~repo
> This was caused by looking at (host != url), which never
> worked for host names with literal IPv6 adresses, embedded by []
> Support for the [] URLs was introduced in 356bece0a.
>
> - Do not tamper local URLs starting with '[' which have a ']'
IMHO, this would have been a little easier to follow as separate
patches, as there are a few things going on. I think the changes to the
code are fine, though.
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 1d1c875..a126f08 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -294,39 +294,95 @@ test_expect_success 'setup ssh wrapper' '
> export TRASH_DIRECTORY
> '
>
> -clear_ssh () {
> - >"$TRASH_DIRECTORY/ssh-output"
> -}
> -
> -expect_ssh () {
I'd prefer if you left the expect_ssh interface intact, as it is
potentially useful outside of t5601 (but your patch ties it very closely
to a particular set of clone tests). Can you call out to expect_ssh
instead from test_clone_url instead?
> +i5601=0
The name of this variable confused me for a bit. Maybe "counter" or
something would be a little more descriptive?
> +# $1 url
> +# $2 none|host
> +# $3 path
> +test_clone_url () {
> + i5601=$(($i5601 + 1))
> + >"$TRASH_DIRECTORY/ssh-output" &&
> + test_might_fail git clone "$1" tmp$i5601 &&
Do we always want test_might_fail in these tests? I really would prefer
that the test actually accomplish the clone, as then we know that
nothing funny is going on (e.g., multiple invocations of ssh, one of
which is good and one of which is not). I think I gave more specific
examples in the last round of review.
> - (cd "$TRASH_DIRECTORY" && test_cmp ssh-expect ssh-output)
> + (
> + cd "$TRASH_DIRECTORY" &&
> + test_cmp ssh-expect ssh-output &&
> + rm -rf ssh-expect ssh-output
> + )
Here we clean up at the end of the test, which is a good improvement on
my existing functions. But I notice that you also clear ssh-output at
the beginning of the test, still (which you must do to not leave a
polluted state after a failing test). I think this might be better as:
test_when_finished '
(cd "$TRASH_DIRECTORY" && rm -f ssh-expect ssh-output)
'
and then the "clear_ssh" can just go away (and your reset of ssh-output,
which is the analogous line), as tests can expect a clean state.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Git v1.8.4.2 test failure in ./t5570-git-daemon.sh
On Tue, Oct 29, 2013 at 01:54:31AM +0100, Simon Ruderich wrote: > I just compiled Git v1.8.4.2 on Debian Wheezy amd64 and test > t5570 fails (with GIT_TEST_GIT_DAEMON=1): > [...] > Bisecting leads to this commit: > > commit 68b939b2f097b6675c4aaa17869aa81b25cb > Author: Jeff King > Date: Wed Sep 18 16:05:13 2013 -0400 > > clone: send diagnostic messages to stderr This is already fixed by Brian Gernhardt's 360a326 (t5570: Update for clone-progress-to-stderr branch, 2013-10-21). Junio, that patch seems to have gone onto jc/upload-pack-send-symref, but should have gone onto jk/clone-progress-to-stderr. The latter made it into maint for v1.8.4.2, but the former did not. I think it was just a simple mixup caused by Brian sending two fixups to t5570 as series, when they are really fixups for two different topics. Not worth an immediate v1.8.4.3, I think, but you may want to cherry-pick 360a326 onto maint if there is another release before v1.8.5. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] t: use perl instead of "$PERL_PATH" where applicable
As of the last commit, we can use "perl" instead of
"$PERL_PATH" when running tests, as the former is now a
function which uses the latter. As the shorter "perl" is
easier on the eyes, let's switch to using it everywhere.
This is not quite a mechanical s/$PERL_PATH/perl/
replacement, though. There are some places where we invoke
perl from a script we generate on the fly, and those scripts
do not have access to our internal shell functions. The
result can be double-checked by running:
ln -s /bin/false bin-wrappers/perl
make test
which continues to pass even after this patch.
Signed-off-by: Jeff King
---
This one is noisy and optional on top of 2/3. We could also just fix up
cases as we see them going forward.
t/gitweb-lib.sh | 2 +-
t/lib-git-svn.sh | 4 ++--
t/lib-terminal.sh | 4 ++--
t/t0202-gettext-perl.sh | 4 ++--
t/t1010-mktree.sh | 4 ++--
t/t3300-funny-names.sh| 6 +++---
t/t4014-format-patch.sh | 2 +-
t/t4020-diff-external.sh | 2 +-
t/t4029-diff-trailing-space.sh| 2 +-
t/t4103-apply-binary.sh | 4 ++--
t/t4116-apply-reverse.sh | 4 ++--
t/t4200-rerere.sh | 8
t/t5300-pack-object.sh| 8
t/t5303-pack-corruption-resilience.sh | 4 ++--
t/t5551-http-fetch.sh | 2 +-
t/t6011-rev-list-with-bad-commit.sh | 2 +-
t/t6013-rev-list-reverse-parents.sh | 4 ++--
t/t7508-status.sh | 2 +-
t/t9129-git-svn-i18n-commitencoding.sh| 2 +-
t/t9137-git-svn-dcommit-clobber-series.sh | 8
t/t9300-fast-import.sh| 2 +-
t/t9350-fast-export.sh| 2 +-
t/t9400-git-cvsserver-server.sh | 2 +-
t/t9401-git-cvsserver-crlf.sh | 2 +-
t/t9402-git-cvsserver-refs.sh | 2 +-
t/t9700-perl-git.sh | 4 ++--
t/t9810-git-p4-rcs.sh | 2 +-
t/test-lib-functions.sh | 6 +++---
28 files changed, 50 insertions(+), 50 deletions(-)
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 9e381e0..8cf909a 100644
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -69,7 +69,7 @@ gitweb_run () {
# written to web server logs, so we are not interested in that:
# we are interested only in properly formatted errors/warnings
rm -f gitweb.log &&
- "$PERL_PATH" -- "$SCRIPT_NAME" \
+ perl -- "$SCRIPT_NAME" \
>gitweb.output 2>gitweb.log &&
perl -w -e '
open O, ">gitweb.headers";
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index c5e55b1..b0ec12f 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -29,7 +29,7 @@ export svnrepo
svnconf=$PWD/svnconf
export svnconf
-"$PERL_PATH" -w -e "
+perl -w -e "
use SVN::Core;
use SVN::Repos;
\$SVN::Core::VERSION gt '1.1.0' or exit(42);
@@ -146,7 +146,7 @@ stop_httpd () {
}
convert_to_rev_db () {
- "$PERL_PATH" -w -- - "$@" <<\EOF
+ perl -w -- - "$@" <<\EOF
use strict;
@ARGV == 2 or die "usage: convert_to_rev_db ";
open my $wr, '+>', $ARGV[1] or die "$!: couldn't open: $ARGV[1]";
diff --git a/t/lib-terminal.sh b/t/lib-terminal.sh
index 58d911d..737df28 100644
--- a/t/lib-terminal.sh
+++ b/t/lib-terminal.sh
@@ -19,7 +19,7 @@ test_expect_success PERL 'set up terminal for tests' '
then
:
elif
- "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl \
+ perl "$TEST_DIRECTORY"/test-terminal.perl \
sh -c "test -t 1 && test -t 2"
then
test_set_prereq TTY &&
@@ -29,7 +29,7 @@ test_expect_success PERL 'set up terminal for tests' '
echo >&4 "test_terminal: need to declare TTY
prerequisite"
return 127
fi
- "$PERL_PATH" "$TEST_DIRECTORY"/test-terminal.perl "$@"
+ perl "$TEST_DIRECTORY"/test-terminal.perl "$@"
}
fi
'
diff --git a/t/t0202-gettext-perl.sh b/t/t0202-gettext-perl.sh
index 428ebb0..a29d166 100755
--- a/t/t0202-gettext-perl.sh
+++ b/t/t0202-gettext-perl.sh
@@ -12,7 +12,7 @@ if ! test_have_prereq PERL; then
test_done
fi
-"$PERL_PATH" -MTest::More -e 0 2>/dev/null || {
+perl -MTest::More -e 0 2>/dev/null || {
skip_all="Perl Test::More unavailable, skipping test"
test_done
}
@@ -22,6 +22,6 @@ test_external_has_tap=1
test_external_without_stderr \
'Perl Git::I18N API' \
-"$PERL_PATH" "$TEST_DIRECTORY"/t0202/test.pl
+perl "$TEST_DIRECTORY"/t0202/test.pl
test_done
diff --git a/t/t1010-mktree.sh b/t/t1010-mktree.sh
index df573c4..b946f87 100755
--- a/t/t1010-mktree.sh
+++ b/t/t1010-mktree.sh
@@ -42,13 +42,13 @@ test_expect_success 'ls-
[PATCH 2/3] t: provide a perl() function which uses $PERL_PATH
Once upon a time, we assumed that calling a bare "perl" in
the test scripts was OK, because we would find the perl from
the user's PATH, and we were only asking that perl to do
basic operations that work even on old versions of perl.
Later, we found that some systems really prefer to use
$PERL_PATH even for these basic cases, because the system
perl misbehaves in some way (e.g., by handling line endings
differently). We then switched "perl" invocations to
"$PERL_PATH" to respect the user's choice.
Having to use "$PERL_PATH" is ugly and cumbersome, though.
Instead, let's provide a perl() shell function that tests
can use, which will transparently do the right thing.
Unfortunately, test writers still have to use $PERL_PATH in
certain situations, so we still need to keep the advice in
the README.
Note that this may fix test failures in t5004, t5503, t6002,
t6003, t6300, t8001, and t8002, depending on your system's
perl setup. All of these can be detected by running:
ln -s /bin/false bin-wrappers/perl
make test
which fails before this patch, and passes after.
Signed-off-by: Jeff King
---
We could always "pollute" bin-wrappers with a broken perl to catch
errors like these, but I think that would also pollute people who put
bin-wrappers into their $PATH. I think we'd need a separate
"test-wrappers" directory, and then put both it and bin-wrappers into
the PATH. I don't know if that is worth it or not.
t/README| 12
t/test-lib-functions.sh | 4
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/t/README b/t/README
index 2167125..06bc5ed 100644
--- a/t/README
+++ b/t/README
@@ -340,7 +340,11 @@ Don't:
- use perl without spelling it as "$PERL_PATH". This is to help our
friends on Windows where the platform Perl often adds CR before
the end of line, and they bundle Git with a version of Perl that
- does not do so, whose path is specified with $PERL_PATH.
+ does not do so, whose path is specified with $PERL_PATH. Note that we
+ provide a "perl" function which uses $PERL_PATH under the hood, so
+ you do not need to worry when simply running perl in the test scripts
+ (but you do, for example, on a shebang line or in a sub script
+ created via "write_script").
- use sh without spelling it as "$SHELL_PATH", when the script can
be misinterpreted by broken platform shell (e.g. Solaris).
@@ -387,7 +391,7 @@ of the test_* functions (see the "Test harness library"
section
below), e.g.:
test_expect_success PERL 'I need Perl' '
-"$PERL_PATH" -e "hlagh() if unf_unf()"
+perl -e "hlagh() if unf_unf()"
'
The advantage of skipping tests like this is that platforms that don't
@@ -520,7 +524,7 @@ library for your script to use.
test_external \
'GitwebCache::*FileCache*' \
- "$PERL_PATH" "$TEST_DIRECTORY"/t9503/test_cache_interface.pl
+ perl "$TEST_DIRECTORY"/t9503/test_cache_interface.pl
If the test is outputting its own TAP you should set the
test_external_has_tap variable somewhere before calling the first
@@ -536,7 +540,7 @@ library for your script to use.
test_external_without_stderr \
'Perl API' \
- "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl
+ perl "$TEST_DIRECTORY"/t9700/test.pl
- test_expect_code
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index a7e9aac..53af452 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -710,3 +710,7 @@ test_ln_s_add () {
git update-index --add --cacheinfo 12 $ln_s_obj "$2"
fi
}
+
+perl () {
+ command "$PERL_PATH" "$@"
+}
--
1.8.4.1.898.g8bf8a41.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] use @@PERL@@ in built scripts
Several of the built shell commands invoke a bare "perl" to
perform some one-liners. This will use the first perl in the
PATH rather than the one specified by the user's SHELL_PATH.
We are not asking these perl invocations to do anything
exotic, so typically any old system perl will do; however,
in some cases the system perl may have unexpected behavior
(e.g., by handling line endings differently). We should err
on the side of using the perl the user pointed us to.
The downside of this is that on systems with a sane perl
setup, we no longer find the perl at runtime, but instead
point to a static perl (like /usr/bin/perl). That means we
will not handle somebody moving perl without rebuilding git,
whereas before we tracked it just fine. This is probably not
a big deal, though, as the built perl scripts already
suffered from this.
Signed-off-by: Jeff King
---
git-am.sh | 4 ++--
git-instaweb.sh | 2 +-
git-request-pull.sh | 2 +-
git-submodule.sh| 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/git-am.sh b/git-am.sh
index 7ea40fe..bbea430 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -302,7 +302,7 @@ split_patches () {
# not starting with Author, From or Date is the
# subject, and the body starts with the next nonempty
# line not starting with Author, From or Date
- perl -ne 'BEGIN { $subject = 0 }
+ @@PERL@@ -ne 'BEGIN { $subject = 0 }
if ($subject > 1) { print ; }
elsif (/^\s+$/) { next ; }
elsif (/^Author:/) { s/Author/From/ ; print ;}
@@ -334,7 +334,7 @@ split_patches () {
# Since we cannot guarantee that the commit message is
in
# git-friendly format, we put no Subject: line and just
consume
# all of the message as the body
- LANG=C LC_ALL=C perl -M'POSIX qw(strftime)' -ne 'BEGIN
{ $subject = 0 }
+ LANG=C LC_ALL=C @@PERL@@ -M'POSIX qw(strftime)' -ne
'BEGIN { $subject = 0 }
if ($subject) { print ; }
elsif (/^\# User /) { s/\# User/From:/ ; print
; }
elsif (/^\# Date /) {
diff --git a/git-instaweb.sh b/git-instaweb.sh
index 01a1b05..e93a238 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -581,7 +581,7 @@ EOF
gitweb_conf() {
cat > "$fqgitdir/gitweb/gitweb_config.perl"
[RFC/PATCH 0/3] perl
On Mon, Oct 28, 2013 at 02:04:20PM -0700, Jonathan Nieder wrote:
> Jeff King wrote:
>
> > Speaking of which, is there any reason to use the ugly "$PERL_PATH"
> > everywhere, and not simply do:
> >
> > perl () {
> > "$PERL_PATH" "$@"
> > }
> >
> > in test-lib.sh?
>
> Sounds like a nice potential improvement to me. :)
One answer to "is there any reason..." is "it will loop infinitely if
you set PERL_PATH=perl". :) However, we can work around that with
"command".
It also may cause problems due to the way one-shot variables are treated
when calling a function versus a command, but we do not seem to set any
variables for invocations perl (and I do not envision it happening
often).
And finally, the other reason I can think of is that we can't apply it
consistently. It only helps where a shell function would activate, which
makes the end result potentially more confusing (especially to somebody
who does not really grok shells and subprocesses). Still, it does not
introduce any _new_ cases that need it, but only helps with a subset of
the cases. So in that sense it is a strict improvement, as we can let
most uses go, but catch only the trickier cases in review.
So I'm on the fence on whether it is a good idea or not, but I wrote up
the patches to play with it. I also noticed that we do not consistently
use $PERL_PATH in some of the built scripts, so I included that fix,
too.
Note that I do not have a system with a broken perl. I simulated a very
broken perl, which is how I found all of the spots to fix. But whether
they are actual bugs that would trigger due to a Windows perl that
handles CRLF differently, I have no clue.
[1/3]: use @@PERL@@ in built scripts
[2/3]: t: provide a perl() function which uses $PERL_PATH
[3/3]: t: use perl instead of "$PERL_PATH" where applicable
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git v1.8.4.2 test failure in ./t5570-git-daemon.sh
Hello, I just compiled Git v1.8.4.2 on Debian Wheezy amd64 and test t5570 fails (with GIT_TEST_GIT_DAEMON=1): --- expect 2013-10-28 23:27:26.792409631 + +++ output 2013-10-28 23:27:26.788409614 + @@ -1 +1,2 @@ +Cloning into 'nowhere'... fatal: remote error: access denied or repository not exported: /nowhere.git [18908] [19625] Disconnected (with error) not ok 9 - clone non-existent --- expect 2013-10-28 23:27:26.944410377 + +++ output 2013-10-28 23:27:26.944410377 + @@ -1 +1,2 @@ +Cloning into 'nowhere'... fatal: remote error: no such repository: /nowhere.git [19727] [19747] Disconnected (with error) not ok 13 - clone non-existent Bisecting leads to this commit: commit 68b939b2f097b6675c4aaa17869aa81b25cb Author: Jeff King Date: Wed Sep 18 16:05:13 2013 -0400 clone: send diagnostic messages to stderr Putting messages like "Cloning into.." and "done" on stdout is un-Unix and uselessly clutters the stdout channel. Send them to stderr. We have to tweak two tests to accommodate this: 1. t5601 checks for doubled output due to forking, and doesn't actually care where the output goes; adjust it to check stderr. 2. t5702 is trying to test whether progress output was sent to stderr, but naively does so by checking whether stderr produced any output. Instead, have it look for "%", a token found in progress output but not elsewhere (and which lets us avoid hard-coding the progress text in the test). This should not regress any scripts that try to parse the current output, as the output is already internationalized and therefore unstable. Regards Simon -- + privacy is necessary + using gnupg http://gnupg.org + public key id: 0x92FEFDB7E44C32F9 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ksummit-attendees] [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Tue, Oct 29, 2013 at 10:09:53AM +1100, Benjamin Herrenschmidt wrote: > On Mon, 2013-10-28 at 09:59 +0100, Christoph Hellwig wrote: > > Btw, can we please take away this discussion from ksummit-attendees? It's > > got > > absolutely nothing to do with kernel summit and is getting fairly annoying. > > Ack. Additionally, iirc, we had decided that > > - We don't cross post multiple lists > > - We drop the annoying subject tags > > As is, all I see is some attempt at doing an lkml dup, which is > pointless I agree too. This whole thread seems to be about noise, and I too thought there was something about not cross-posting between this list and any other list. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ksummit-attendees] [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Tue, Oct 29, 2013 at 10:09:53AM +1100, Benjamin Herrenschmidt wrote: > On Mon, 2013-10-28 at 09:59 +0100, Christoph Hellwig wrote: > > Btw, can we please take away this discussion from ksummit-attendees? It's > > got > > absolutely nothing to do with kernel summit and is getting fairly annoying. > > Ack. Additionally, iirc, we had decided that > > - We don't cross post multiple lists > > - We drop the annoying subject tags > > As is, all I see is some attempt at doing an lkml dup, which is > pointless I agree too. This whole thread seems to be about noise, and I too thought there was something about not cross-posting between this list and any other list. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git test failure: t9903-bash-prompt.sh on darwin8
Hi, I'm seeing a test failure with git-1.8.4 on powerpc-darwin8: [11:00:56] t9903-bash-prompt.sh ... not ok 13 - prompt - interactive rebase not ok 14 - prompt - rebase merge Details here: http://paste.lisp.org/display/139525 The odd thing is that when I run the test t9903 interactively, post-mortem, it passes -- I am unable to reproduce the failure outside of the unprivileged build environment. Interactively, the test always passes regardless of my interaction shell: csh, sh-2.0, bash-4.2. (Noninteractive test happens to use bash-2.0.) So I force the test to halt immediately upon failure to examine the temporary trash directory, and the difference between actual/expected: http://paste.lisp.org/display/139525#4 What could cause the results to differ between by interactive and non-interactive testing of t9903? Is there a real problem? Fang -- David Fang http://www.csl.cornell.edu/~fang/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ksummit-attendees] [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, 2013-10-28 at 09:59 +0100, Christoph Hellwig wrote: > Btw, can we please take away this discussion from ksummit-attendees? It's got > absolutely nothing to do with kernel summit and is getting fairly annoying. Ack. Additionally, iirc, we had decided that - We don't cross post multiple lists - We drop the annoying subject tags As is, all I see is some attempt at doing an lkml dup, which is pointless Ben. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Loan Offer
We offer Loan for 3% if you are interested do send details.contact Mr Tony Bowyer: coastalfinanceloanf...@hotmail.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 18/19] t: add basic bitmap functionality tests
On Fri, Oct 25, 2013 at 02:04:38AM -0400, Jeff King wrote: > diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh > new file mode 100755 > index 000..0868725 > --- /dev/null > +++ b/t/t5310-pack-bitmaps.sh > @@ -0,0 +1,114 @@ > +#!/bin/sh > + > +test_description='exercise basic bitmap functionality' > +. ./test-lib.sh > + > +test_expect_success 'setup repo with moderate-sized history' ' > + for i in $(test_seq 1 10); do > + test_commit $i > + done && > + git checkout -b other HEAD~5 && > + for i in `test_seq 1 10`; do > + test_commit side-$i > + done && Sorry, style nitpick: could you rewrite this command substitution using $() like a few lines above? > +test_expect_success 'setup further non-bitmapped commits' ' > + for i in `test_seq 1 10`; do > + test_commit further-$i > + done > +' Likewise. Best, Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
Johan Herland writes:
> But I still don't see exactly what this option should do (inside "git
> commit") that would end up being useful across most/all projects, and
> not just something that could more easily be implemented in the
> *commit-msg hooks for relevant projects.
[Ok, admittedly I don't really know what to quote from your message,
since I'm mostly responding to the overall concept.]
I like the idea of putting all that in hooks, but I have two
observations:
* Signed-off-by: is already such a case (and was probably also added for
the kernel?) that _could_ have been dealt with using {prepare-,}commit-msg,
but has its own support in various git tools.
* In your list
> Fixes:
> Reported-by:
> Suggested-by:
> Improved-by:
> Acked-by:
> Reviewed-by:
> Tested-by:
> Signed-off-by:
and I might add
Cherry-picked-from:
Reverts:
if one were to phrase that as a footer/pseudoheader, observe that
there are only two kinds of these: footers that contain identities,
and footers that contain references to commits.
So why not support these use-cases? We could have something like
footer.foo.* configuration, e.g.
[footer "fixes"]
type = commit
suggest = true
[footer "acked-by"]
type = identity
where 'suggest' (please suggest a better name) means that git-commit
will put a blank one in the commit message template for you to fill in.
'commit' and 'identity' can have some elementary expansion and
validation tied to them. Some easy extensiblity (hooks?) might not
hurt, but then as you point out, the existing hooks already cover that.
Perhaps we could also have, for Gerrit (cf. [1]):
[footer "change-id"]
type = uuid
though admittedly I haven't investigated if it's okay to just put a
random string there, or it needs to have a specific value.
[1] http://thread.gmane.org/gmane.comp.version-control.git/236429
--
Thomas Rast
t...@thomasrast.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle
Ben Walton writes: > It's an escape. Without it, sed throws: The shell removes it before sed can see it. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #07; Mon, 28)
Hi Karsten Junio C Hamano writes: > * kb/fast-hashmap (2013-10-22) 12 commits > - remove old hash.[ch] implementation > - read-cache.c: fix memory leaks caused by removed cache entries I found more valgrind breakage related to this commit, in t2101.[3567] (sorry for only reporting them so late, I probably missed them in the last run). E.g. I get this: $ ./t2101-update-index-reupdate.sh --valgrind-only=3 ok 1 - update-index --add ok 2 - update-index --again expecting success: git update-index --remove --again && git ls-files -s >current && cmp current expected ==21665== Invalid read of size 1 ==21665==at 0x4C2C762: __GI_strlen (mc_replace_strmem.c:405) ==21665==by 0x484B0E: update_one (update-index.c:305) ==21665==by 0x485466: do_reupdate (update-index.c:582) ==21665==by 0x4858FB: reupdate_callback (update-index.c:696) ==21665==by 0x4EB5E7: get_value (parse-options.c:96) ==21665==by 0x4EBEC5: parse_long_opt (parse-options.c:302) ==21665==by 0x4EC5CD: parse_options_step (parse-options.c:474) ==21665==by 0x486115: cmd_update_index (update-index.c:824) ==21665==by 0x405999: run_builtin (git.c:314) ==21665==by 0x405B2C: handle_internal_command (git.c:477) ==21665==by 0x405C46: run_argv (git.c:523) ==21665==by 0x405DE2: main (git.c:606) ==21665== Address 0x5bee774 is 84 bytes inside a block of size 90 free'd ==21665==at 0x4C2ACDA: free (vg_replace_malloc.c:468) ==21665==by 0x4F9360: remove_index_entry_at (read-cache.c:482) ==21665==by 0x4F9536: remove_file_from_index (read-cache.c:522) ==21665==by 0x4841DF: remove_one_path (update-index.c:68) ==21665==by 0x48422E: process_lstat_error (update-index.c:83) ==21665==by 0x4846BB: process_path (update-index.c:211) ==21665==by 0x484AC2: update_one (update-index.c:301) ==21665==by 0x485466: do_reupdate (update-index.c:582) ==21665==by 0x4858FB: reupdate_callback (update-index.c:696) ==21665==by 0x4EB5E7: get_value (parse-options.c:96) ==21665==by 0x4EBEC5: parse_long_opt (parse-options.c:302) ==21665==by 0x4EC5CD: parse_options_step (parse-options.c:474) [...] not ok 3 - update-index --remove --again # git update-index --remove --again && #git ls-files -s >current && #cmp current expected ok 4 - first commit ok 5 - update-index again ok 6 - update-index --update from subdir ok 7 - update-index --update with pathspec # failed 1 among 7 test(s) 1..7 The errors for tests 5-7 look like they're the same piece of code breaking. -- Thomas Rast t...@thomasrast.ch -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
Ignore this version. The immediate followup quotes PERL_PATH.
On Mon, Oct 28, 2013 at 9:40 PM, Ben Walton wrote:
> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
> semantics for tr whereby string1's length is truncated to the length
> of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
> see string2 padded to the length of string1 using the final character
> in string2. POSIX explicitly doesn't specify the correct behavior
> here, making both equally valid.
>
> This difference means that Solaris' native tr implementations produce
> different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
> tests in t0008-ignores.sh.
>
> Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
>
> Instead, use perl to perform these transliterations which means we
> don't need to worry about the difference at all. Since we're replacing
> tr with perl, we also use perl to replace the sed invocations used to
> transform the files.
>
> Replace four identical transforms with a function named
> broken_c_unquote. Replace the other two identical transforms with a
> fuction named broken_c_unquote_verbose.
>
> Signed-off-by: Ben Walton
> ---
> t/t0008-ignores.sh | 30 ++
> 1 file changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> index 181513a..45f9396 100755
> --- a/t/t0008-ignores.sh
> +++ b/t/t0008-ignores.sh
> @@ -37,6 +37,14 @@ test_stderr () {
> test_cmp "$HOME/expected-stderr" "$HOME/stderr"
> }
>
> +broken_c_unquote () {
> + $PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
> +}
> +
> +broken_c_unquote_verbose () {
> + $PERL_PATH -pe 's/ "/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
> +}
> +
> stderr_contains () {
> regexp="$1"
> if grep "$regexp" "$HOME/stderr"
> @@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
> $global_excludes:2:!globaltwo b/globaltwo
> EOF
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> - tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> - tr "\n" "\0" >expected-default0
> -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> - tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
> test_expect_success '--stdin' '
> expect_from_stdin @@ -692,12 +699,11 @@ EOF
> grep -v '^:: ' expected-all >expected-verbose
> sed -e 's/.* //' expected-verbose >expected-default
>
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
> - tr "\n" "\0" >stdin0
> -sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
> - tr "\n" "\0" >expected-default0
> -sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
> - tr ":\t\n" "\0" >expected-verbose0
> +broken_c_unquote stdin >stdin0
> +
> +broken_c_unquote expected-default >expected-default0
> +
> +broken_c_unquote_verbose expected-verbose >expected-verbose0
>
> test_expect_success '--stdin from subdirectory' '
> expect_from_stdin --
> 1.8.1.2
>
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/README: tests can use perl even with NO_PERL
On Mon, Oct 28, 2013 at 9:04 PM, Jonathan Nieder wrote:
> Jeff King wrote:
>
>> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
>> everywhere, and not simply do:
>>
>> perl () {
>> "$PERL_PATH" "$@"
>> }
>>
>> in test-lib.sh?
>
> Sounds like a nice potential improvement to me. :)
+1.
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
...Forgot to quote PERL_PATH in the previous iteration.
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..b4d98e6 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ "$PERL_PATH" -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ "$PERL_PATH" -pe 's/"/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
[PATCH] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..45f9396 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ $PERL_PATH -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ $PERL_PATH -pe 's/ "/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
Ben Walton writes: > Per the other discussion about replacing all PERL_PATH with a shell > function named perl, should I update this patch to use $PERL_PATH in > the meantime so that it can be batch updated when the function is > added in a separate patch? Yeah, sounds like a good plan, and very much appreciated. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
On Mon, Oct 28, 2013 at 9:04 PM, Ben Walton wrote:
> I'm happy to defer to your judgement on this - If you'd like the tests
> wrapped, I'll do so.
>
> Thanks
> -Ben
>
> On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano wrote:
>> Jonathan Nieder writes:
>>
>>> Johannes Sixt wrote:
>>>
In other tests, we check for prerequisite PERL, i.e., we are prepared
that perl is not available. Shouldn't we do that here, too?
>>>
>>> I think the tests assume there's a perl present even when the PERL
>>> prereq isn't present already. E.g.:
>>>
>>> nul_to_q () {
>>> "$PERL_PATH" -pe 'y/\000/Q/'
>>> }
>>>
>>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>>> in other words, "commands that use perl work". Maybe something
>>> like the following would help?
>>>
>>> Signed-off-by: Jonathan Nieder
>>>
>>> diff --git i/t/README w/t/README
>>> index 2167125..54cd064 100644
>>> --- i/t/README
>>> +++ w/t/README
>>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in
>>> the "Test harness
>>> library" section above and the "test_have_prereq" function for how to
>>> use these, and "test_set_prereq" for how to define your own.
>>>
>>> - - PERL & PYTHON
>>> + - PYTHON
>>>
>>> - Git wasn't compiled with NO_PERL=YesPlease or
>>> - NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>>> - these.
>>> + Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>>> + need Python with this.
>>> +
>>> + - PERL
>>> +
>>> + Git wasn't compiled with NO_PERL=YesPlease.
>>> +
>>> + Even without the PERL prerequisite, tests can assume there is a
>>> + usable perl interpreter at $PERL_PATH, though it need not be
>>> + particularly modern.
>>> +
>>> + Wrap tests for commands implemented in Perl with this.
>>
>> Sounds like a sensible thing to address, but I first parsed it as
>>
>> Wrap (tests for (commands implemented in Perl)) with this.
>>
>> while I think the readers are expected to parse it as
>>
>>Wrap ((tests for commands) implemented in Perl) with this.
>>
Per the other discussion about replacing all PERL_PATH with a shell
function named perl, should I update this patch to use $PERL_PATH in
the meantime so that it can be batch updated when the function is
added in a separate patch?
Thanks
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle
On Mon, Oct 28, 2013 at 5:39 PM, Andreas Schwab wrote: > Ben Walton writes: > >> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh >> index 3fb4b97..0126154 100755 >> --- a/t/t4015-diff-whitespace.sh >> +++ b/t/t4015-diff-whitespace.sh >> @@ -145,7 +145,8 @@ test_expect_success 'another test, with >> --ignore-space-at-eol' 'test_cmp expect >> test_expect_success 'ignore-blank-lines: only new lines' ' >> test_seq 5 >x && >> git update-index x && >> - test_seq 5 | sed "/3/i \\ >> + test_seq 5 | sed "/3/i\\ >> +\ >> " >x && > > Why do you need the \? Since it is inside double quotes the shell > will remove it during expansion. It's an escape. Without it, sed throws: sed: -e expression #1, char 5: expected \ after `a', `c' or `i' Thanks -Ben -- --- Take the risk of thinking for yourself. Much more happiness, truth, beauty and wisdom will come to you that way. -Christopher Hitchens --- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
I'm happy to defer to your judgement on this - If you'd like the tests
wrapped, I'll do so.
Thanks
-Ben
On Mon, Oct 28, 2013 at 7:08 PM, Junio C Hamano wrote:
> Jonathan Nieder writes:
>
>> Johannes Sixt wrote:
>>
>>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>>> that perl is not available. Shouldn't we do that here, too?
>>
>> I think the tests assume there's a perl present even when the PERL
>> prereq isn't present already. E.g.:
>>
>> nul_to_q () {
>> "$PERL_PATH" -pe 'y/\000/Q/'
>> }
>>
>> So in practice the PERL prereq just means "NO_PERL wasn't set", or
>> in other words, "commands that use perl work". Maybe something
>> like the following would help?
>>
>> Signed-off-by: Jonathan Nieder
>>
>> diff --git i/t/README w/t/README
>> index 2167125..54cd064 100644
>> --- i/t/README
>> +++ w/t/README
>> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the
>> "Test harness
>> library" section above and the "test_have_prereq" function for how to
>> use these, and "test_set_prereq" for how to define your own.
>>
>> - - PERL & PYTHON
>> + - PYTHON
>>
>> - Git wasn't compiled with NO_PERL=YesPlease or
>> - NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
>> - these.
>> + Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
>> + need Python with this.
>> +
>> + - PERL
>> +
>> + Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> + Even without the PERL prerequisite, tests can assume there is a
>> + usable perl interpreter at $PERL_PATH, though it need not be
>> + particularly modern.
>> +
>> + Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
> Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>Wrap ((tests for commands) implemented in Perl) with this.
>
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #06; Fri, 25)
On Mon, Oct 28, 2013 at 8:45 PM, Karsten Blees wrote:
>> The new `hashmap.c` covers the first case quite well (albeit slightly
>> more verbosely than I'd like), but in the second case it doesn't quite
>> work. Since the new hash needs to embed the "struct hashmap_entry" on
>> all its values (to allow for separate chaining), having it map to
>> `int` keys requires a struct like this:
>>
>> struct sha1_position {
>> struct hashmap_entry {
>> struct hashmap_entry *next;
>> unsigned int hash;
>> };
>> int position;
>> }
>>
>
> Hmm...isn't that position rather an index into two separately maintained
> arrays? So you'd rather have:
>
> struct sha1_position {
> struct hashmap_entry {
> struct hashmap_entry *next;
> unsigned int hash;
> };
> uint32_t pack_name_hash;
> struct object *object;
> }
No, this is not quite right. We use the separate arrays because the
normal operation mode is to index by position (e.g. we need the nth
object in the extended index); the hash table is an auxiliary
structure to reverse that indexing (e.g. what position does this SHA1
have on the extended index). The information which is always required
to construct bitmaps is position, so we need to store the indexes in a
map.
>> khash on the other hand is capable of storing the position values as
>> part of the hash table itself (i.e. `int **buckets`), and saves us
>> from thousands of bytes of allocations + indirection.
>>
>
> However, khash keeps separate arrays for flags, keys and values, all of them
> overallocated by 1 / load factor (so there's lots of unused space).
> ext_index.objects and ext_index.hashes are also overallocated by the usual
> alloc_nr() factor 1.5.
FWIW The flags for khash are compacted, so they are stored much more
tightly than pointers, even when overallocated.
>
> Regarding memory consumption, I think both implementations will be pretty
> similar. Hashmap allocates many small regions while khash re-allocates a few
> big ones...I really don't know which is better, ideally entries would be
> allocated in chunks to minimize both heap overhead and memcpy disadvantes.
I agree, both implementations probably have very similar memory
characteristics, probably enough not to matter.
> Regarding performance, khash uses open addressing, which requires more key
> comparisons (O(1/(1-load_factor))) than chaining (O(1+load_factor)). However,
> any measurable differences will most likely be dwarfed by IO costs in this
> particular use case.
I don't think this is true. If you actually run a couple insertion and
lookup benchmarks comparing the two implementations, you'll find khash
to be around ~30% faster for most workloads (venturing a guess from
past experience). I am obviously not the author of khash, but I've
found that the theoretical increase in key comparisons is completely
dwarfed by the benefit of increased cache locality during the probing
fase. I still haven't found a faster C hash table implementation than
khash for the general case, that's why I normally use it despite the
worrisome preprocessor crash-party going on in that header file.
> Btw., pack-objects.c::rehash_objects() in patch 03 unnecessarily checks for
> duplicates. That's probably the reason for the high hashcmp times you found
> in the first round of the patch series.
Patch 03 is a refactoring -- the duplicate checking code has been in
pack-objects.c for years. I am not sure duplicate checking is
superfluous at all, there are many cases where you could be
double-inserting objects in a pack-objects run, and you really don't
want to generate packfiles with dupe objects.
Thanks for the feedback!
luv,
vmg
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/README: tests can use perl even with NO_PERL
Jeff King wrote:
> Speaking of which, is there any reason to use the ugly "$PERL_PATH"
> everywhere, and not simply do:
>
> perl () {
> "$PERL_PATH" "$@"
> }
>
> in test-lib.sh?
Sounds like a nice potential improvement to me. :)
Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] git clone: is an URL local or ssh
Torsten Bögershausen writes:
> (This does apply on pu, not on master.
Hmph. At least for me, it applies down to cabb411f (Merge branch
'nd/clone-local-with-colon', 2013-10-14) just fine. Puzzled.
> diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
> index 1d1c875..a126f08 100755
> --- a/t/t5601-clone.sh
> +++ b/t/t5601-clone.sh
> @@ -294,39 +294,95 @@ test_expect_success 'setup ssh wrapper' '
> export TRASH_DIRECTORY
> '
>
> -clear_ssh () {
> - >"$TRASH_DIRECTORY/ssh-output"
> -}
> -
> -expect_ssh () {
> +i5601=0
> +# $1 url
> +# $2 none|host
> +# $3 path
> +test_clone_url () {
> + i5601=$(($i5601 + 1))
> + >"$TRASH_DIRECTORY/ssh-output" &&
> + test_might_fail git clone "$1" tmp$i5601 &&
> {
> - case "$1" in
> + case "$2" in
> none)
> ;;
> *)
> - echo "ssh: $1 git-upload-pack '$2'"
> + echo "ssh: $2 git-upload-pack '$3'"
> esac
> } >"$TRASH_DIRECTORY/ssh-expect" &&
This looks like a strange use of {} (not an issue this patch
introduced, though). Shouldn't this suffice?
case ... in
...
esac >"$TRASH_DIRECTORY/ssh-expect"
> + (
> + cd "$TRASH_DIRECTORY" &&
> + test_cmp ssh-expect ssh-output &&
> + rm -rf ssh-expect ssh-output
Drop "r", please, when you know these are supposed to be files.
> + )
> }
>
> +# url looks ssh like, and is on disc: should be local
> test_expect_success NOT_MINGW,NOT_CYGWIN 'clone local path foo:bar' '
> cp -R src "foo:bar" &&
> + test_clone_url "foo:bar" none &&
> + ( cd tmp$i5601 && git log)
Hmph. What is this "git log" about? Leftover from an earlier
debugging session?
The code change to connect.c part seemed to be OK from a cursory
look.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sha1_file: move comment about return value where it belongs
On Sun, Oct 27, 2013 at 12:34:30AM +0200, Christian Couder wrote: > Commit 5b0864070 (sha1_object_info_extended: make type calculation > optional, Jul 12 2013) changed the return value of the > sha1_object_info_extended function to 0/-1 for success/error. > > Previously this function returned the object type for success or > -1 for error. But unfortunately the above commit forgot to change > or move the comment above this function that says "returns enum > object_type or negative". > > To fix this inconsistency, let's move the comment above the > sha1_object_info function where it is still true. Thanks for catching this. Your analysis and patch looks good to me. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] git clone: is an URL local or ssh
Commit 8d3d28f5 added test cases for URLs which should be ssh.
Add more tests testing all the combinations:
-IPv4 or IPv6
-path starting with "/" or with "/~"
-with and without the ssh:// scheme
Add tests for ssh:// with port number.
When a git repository "foo:bar" exist, git clone will call
absolute_path() and git_connect() will be called with
something like "/home/user/projects/foo:bar"
Tighten the test and use "foo:bar" instead of "./foo:bar",
refactor clear_ssh() and expect_ssh() into test_clone_url().
"git clone foo/bar:baz" should not be ssh:
Make the rule
"if there is a slash before the first colon, it is not ssh"
more strict by using the same is_local() in both connect.c
and transport.c. Add a test case.
Bug fixes for corner cases:
- Handle clone of [::1]:/~repo correctly (2e7766655):
Git should call "ssh ::1 ~repo", not ssh ::1 /~repo
This was caused by looking at (host != url), which never
worked for host names with literal IPv6 adresses, embedded by []
Support for the [] URLs was introduced in 356bece0a.
- Do not tamper local URLs starting with '[' which have a ']'
Signed-off-by: Torsten Bögershausen
---
(This does apply on pu, not on master.
I'm almost sure there are more corner cases, but the
most important things should be covered)
Changes since V1: Comments from Eric Sunshine (thanks)
connect.c| 47 +--
connect.h| 1 +
t/t5601-clone.sh | 98
transport.c | 8 -
4 files changed, 108 insertions(+), 46 deletions(-)
diff --git a/connect.c b/connect.c
index 06e88b0..d61adc9 100644
--- a/connect.c
+++ b/connect.c
@@ -231,6 +231,7 @@ int server_supports(const char *feature)
}
enum protocol {
+ PROTO_LOCAL_OR_SSH = 0,
PROTO_LOCAL = 1,
PROTO_SSH,
PROTO_GIT
@@ -547,6 +548,15 @@ static char *get_port(char *host)
static struct child_process no_fork;
+int is_local(const char *url)
+{
+ const char *colon = strchr(url, ':');
+ const char *slash = strchr(url, '/');
+ return !colon || (slash && slash < colon) ||
+ has_dos_drive_prefix(url);
+}
+
+
/*
* This returns a dummy child_process if the transport protocol does not
* need fork(2), or a struct child_process object if it does. Once done,
@@ -564,9 +574,9 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
char *url;
char *host, *path;
char *end;
- int c;
+ int separator;
struct child_process *conn = &no_fork;
- enum protocol protocol = PROTO_LOCAL;
+ enum protocol protocol = PROTO_LOCAL_OR_SSH;
int free_path = 0;
char *port = NULL;
const char **arg;
@@ -587,20 +597,23 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
*host = '\0';
protocol = get_protocol(url);
host += 3;
- c = '/';
+ separator = '/';
} else {
host = url;
- c = ':';
+ separator = ':';
+ if (is_local(url))
+ protocol = PROTO_LOCAL;
}
/*
* Don't do destructive transforms with git:// as that
* protocol code does '[]' unwrapping of its own.
+* Don't change local URLs
*/
if (host[0] == '[') {
end = strchr(host + 1, ']');
if (end) {
- if (protocol != PROTO_GIT) {
+ if (protocol != PROTO_GIT && protocol != PROTO_LOCAL) {
*end = 0;
host++;
}
@@ -610,17 +623,17 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
} else
end = host;
- path = strchr(end, c);
- if (path && !has_dos_drive_prefix(end)) {
- if (c == ':') {
- if (host != url || path < strchrnul(host, '/')) {
- protocol = PROTO_SSH;
- *path++ = '\0';
- } else /* '/' in the host part, assume local path */
- path = end;
+ path = strchr(end, separator);
+ if (separator == ':') {
+ if (path && protocol == PROTO_LOCAL_OR_SSH) {
+ /* We have a ':' */
+ protocol = PROTO_SSH;
+ *path++ = '\0';
+ } else {/* assume local path */
+ protocol = PROTO_LOCAL;
+ path = end;
}
- } else
- path = end;
+ }
if (!path || !*path)
die("No path specified. See 'man git-pull' for valid url
syntax");
@@ -629,7 +642,7 @@ struct child_process *git_connect(int fd[2], const char
*url_orig,
* null-terminate hostname and point path to
Re: [PATCH] t/README: tests can use perl even with NO_PERL
On Mon, Oct 28, 2013 at 12:22:16PM -0700, Jonathan Nieder wrote:
> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl. Helpers such as
>
> nul_to_q () {
> "$PERL_PATH" -pe 'y/\000/Q/'
> }
>
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
>
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents). Update the test documentation to
> clarify this.
>
> Reported-by: Johannes Sixt
> Signed-off-by: Jonathan Nieder
> ---
Yeah, I think this accurately the conclusions we've come to informally
during review on the list (for a long time we did not even use
$PERL_PATH for such "vanilla" cases, but some people have a broken perl
in their PATH).
Your patch looks good, and I think Ben's patch does not need a PERL
prerequisite. However, it is supposed to use $PERL_PATH, which it does
not.
Speaking of which, is there any reason to use the ugly "$PERL_PATH"
everywhere, and not simply do:
perl () {
"$PERL_PATH" "$@"
}
in test-lib.sh?
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] t/README: tests can use perl even with NO_PERL
Am 28.10.2013 20:22, schrieb Jonathan Nieder:
> The git build system supports a NO_PERL switch to avoid installing
> perl bindings or other features (like "git add --patch") that rely on
> perl on runtime, but even with NO_PERL it has not been possible for a
> long time to run tests without perl. Helpers such as
>
> nul_to_q () {
> "$PERL_PATH" -pe 'y/\000/Q/'
> }
>
> use perl as a better tr or sed and are regularly used in tests without
> worrying to add a PERL prerequisite.
>
> Perl is portable enough that it seems fine to keep relying on it for
> this kind of thing in tests (and more readable than the alternative of
> trying to find POSIXy equivalents). Update the test documentation to
> clarify this.
Thank you for the clarification!
-- Hannes
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #06; Fri, 25)
Am 28.10.2013 17:16, schrieb Vicent Martí:
> On Mon, Oct 28, 2013 at 4:48 PM, Junio C Hamano wrote:
>>> jk/pack-bitmap adds khash.h, which from a first glance looks like yet
>>> another hash table implementation. I was just wondering if kb's new
>>> hash tables can cover the need of pack-bitmap.c too so we can remove
>>> khash.h later..
>>
>> Good thinking ;-).
>
> We use the khash tables to map:
>
> - sha1 (const char *) to (void *)
> - sha1 (const char *) to int
>
> The new `hashmap.c` covers the first case quite well (albeit slightly
> more verbosely than I'd like), but in the second case it doesn't quite
> work. Since the new hash needs to embed the "struct hashmap_entry" on
> all its values (to allow for separate chaining), having it map to
> `int` keys requires a struct like this:
>
> struct sha1_position {
> struct hashmap_entry {
> struct hashmap_entry *next;
> unsigned int hash;
> };
> int position;
> }
>
Hmm...isn't that position rather an index into two separately maintained
arrays? So you'd rather have:
struct sha1_position {
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
uint32_t pack_name_hash;
struct object *object;
}
> khash on the other hand is capable of storing the position values as
> part of the hash table itself (i.e. `int **buckets`), and saves us
> from thousands of bytes of allocations + indirection.
>
However, khash keeps separate arrays for flags, keys and values, all of them
overallocated by 1 / load factor (so there's lots of unused space).
ext_index.objects and ext_index.hashes are also overallocated by the usual
alloc_nr() factor 1.5.
Regarding memory consumption, I think both implementations will be pretty
similar. Hashmap allocates many small regions while khash re-allocates a few
big ones...I really don't know which is better, ideally entries would be
allocated in chunks to minimize both heap overhead and memcpy disadvantes.
Regarding performance, khash uses open addressing, which requires more key
comparisons (O(1/(1-load_factor))) than chaining (O(1+load_factor)). However,
any measurable differences will most likely be dwarfed by IO costs in this
particular use case.
Btw., pack-objects.c::rehash_objects() in patch 03 unnecessarily checks for
duplicates. That's probably the reason for the high hashcmp times you found in
the first round of the patch series.
Cheers,
Karsten
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Oct 2013, #07; Mon, 28)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. It is already 10th week of this cycle, but somehow I completely forgot where in the cycle we were. Sorry about that. I'll tag 1.8.5-rc0 in a few days by the end of this month, and then hopefully we will have two to three -rc weeks after that, aiming for the final 1.8.5 release sometime late November (tentative schedule at http://tinyurl.com/gitCal). As promised/requested, the final steps for 2.0 are in 'next'; they, together with a handful topics that have been merged to 'next' fairly recently, will _not_ be part of the upcoming 1.8.5 release, but will be carried over in 'next' to the next cycle. Also there is 1.8.4.2 maintenance release out. You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * ew/keepalive (2013-10-16) 2 commits (merged to 'next' on 2013-10-16 at 56fd9f3) + http: use curl's tcp keepalive if available (merged to 'next' on 2013-10-14 at 24d786f) + http: enable keepalive on TCP sockets The HTTP transport will try to use TCP keepalive when able. * jc/revision-range-unpeel (2013-10-15) 1 commit (merged to 'next' on 2013-10-16 at d04ddfe) + revision: do not peel tags used in range notation "git rev-list --objects ^v1.0^ v1.0" gave v1.0 tag itself in the output, but "git rev-list --objects v1.0^..v1.0" did not. * jk/remote-literal-string-leakfix (2013-10-15) 1 commit (merged to 'next' on 2013-10-18 at 6abddac) + remote: do not copy "origin" string literal * jk/split-broken-ident (2013-10-15) 1 commit (merged to 'next' on 2013-10-18 at 8f4b8b7) + split_ident: parse timestamp from end of line Make the fall-back parsing of commit objects with broken author or committer lines more robust to pick up the timestamps. * jx/relative-path-regression-fix (2013-10-14) 3 commits (merged to 'next' on 2013-10-18 at b4af45f) + Use simpler relative_path when set_git_dir (merged to 'next' on 2013-10-14 at 704b9ee) + relative_path should honor dos-drive-prefix + test: use unambigous leading path (/foo) for MSYS Will merge to 'master' and later to 'maint'. * sb/repack-in-c (2013-10-22) 1 commit (merged to 'next' on 2013-10-23 at 5d7ac72) + Reword repack documentation to no longer state it's a script Finishing touches to update documentation. * sg/prompt-svn-remote-fix (2013-10-15) 1 commit (merged to 'next' on 2013-10-18 at 20b47eb) + bash prompt: don't use '+=' operator in show upstream code path Bash portability fix. -- [New Topics] * bw/solaris-sed-tr-test-portability (2013-10-28) 2 commits - Avoid difference in tr semantics between System V and BSD - Change sed i\ usage to something Solaris' sed can handle Needs a bit of reroll. * fc/transport-helper-fixes (2013-10-28) 13 commits - test: remote-helper: add test for force pushes - git-remote-testgit: support the new 'force' option - fixup! transport-helper: add 'force' to 'export' helpers - transport-helper: don't update refs in dry-run - transport-helper: add support to delete branches - fast-export: add support to delete refs - fast-import: add support to delete refs - transport-helper: add support for old:new refspec - fast-export: add new --refspec option - fast-export: improve argument parsing - transport-helper: check for 'forced update' message - transport-helper: fix extra lines - transport-helper: add 'force' to 'export' helpers * jh/loose-object-dirs-creation-race (2013-10-28) 1 commit - sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs * js/test-help-format-windows-port-fix (2013-10-28) 1 commit - PATCH] t3200: do not open a HTML manual page when DEFAULT_MAN_FORMAT is html Will merge to 'next' after amending the title. * js/tests-windows-port-fix (2013-10-28) 3 commits - tests: undo special treatment of CRLF for Windows - Windows: a test_cmp that is agnostic to random LF <> CRLF conversions - t5300-pack-object: do not compare binary data using test_cmp Will merge to 'next'. * nd/liteal-pathspecs (2013-10-28) 1 commit - pathspec: stop --*-pathspecs impact on internal parse_pathspec() uses * rs/web-browse-xdg-open (2013-10-28) 1 commit - web--browse: Add support for xdg-open. Will merge to 'next'. * sb/refs-code-cleanup (2013-10-28) 2 commits - cache: remove unused function 'have_git_dir' - refs: remove unused function invalidate_ref_cache Will merge to 'next'. * th/reflog-annotated-tag (2013-10-28) 1 commit - reflog: handle lightweight and annotated tags equally -- [Stalled] * np/pack-v4 (2013-09-18) 90 commits . packv4-parse.c: add tree offset caching . t1050: replace one instance of
[PATCH] t/README: tests can use perl even with NO_PERL
The git build system supports a NO_PERL switch to avoid installing
perl bindings or other features (like "git add --patch") that rely on
perl on runtime, but even with NO_PERL it has not been possible for a
long time to run tests without perl. Helpers such as
nul_to_q () {
"$PERL_PATH" -pe 'y/\000/Q/'
}
use perl as a better tr or sed and are regularly used in tests without
worrying to add a PERL prerequisite.
Perl is portable enough that it seems fine to keep relying on it for
this kind of thing in tests (and more readable than the alternative of
trying to find POSIXy equivalents). Update the test documentation to
clarify this.
Reported-by: Johannes Sixt
Signed-off-by: Jonathan Nieder
---
Junio C Hamano wrote:
> Jonathan Nieder writes:
>> + - PERL
>> +
>> + Git wasn't compiled with NO_PERL=YesPlease.
>> +
>> + Even without the PERL prerequisite, tests can assume there is a
>> + usable perl interpreter at $PERL_PATH, though it need not be
>> + particularly modern.
>> +
>> + Wrap tests for commands implemented in Perl with this.
>
> Sounds like a sensible thing to address, but I first parsed it as
>
> Wrap (tests for (commands implemented in Perl)) with this.
>
> while I think the readers are expected to parse it as
>
>Wrap ((tests for commands) implemented in Perl) with this.
I meant the former --- that is, tests for commands like "git add -p"
should be skipped in a NO_PERL build. Probably clearest to leave out
the third paragraph entirely, like this:
t/README | 15 +++
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/t/README b/t/README
index 2167125..0a939eb 100644
--- a/t/README
+++ b/t/README
@@ -629,11 +629,18 @@ See the prereq argument to the test_* functions in the
"Test harness
library" section above and the "test_have_prereq" function for how to
use these, and "test_set_prereq" for how to define your own.
- - PERL & PYTHON
+ - PYTHON
- Git wasn't compiled with NO_PERL=YesPlease or
- NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
- these.
+ Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
+ need Python with this.
+
+ - PERL
+
+ Git wasn't compiled with NO_PERL=YesPlease.
+
+ Even without the PERL prerequisite, tests can assume there is a
+ usable perl interpreter at $PERL_PATH, though it need not be
+ particularly modern.
- POSIXPERM
--
1.8.4.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git v1.8.4.2
The latest maintenance release Git v1.8.4.2 is now available at the usual places. The release tarballs are found at: http://code.google.com/p/git-core/downloads/list and their SHA-1 checksums are: f2e9317703553b4215700605c15d0f3a30623a9d git-1.8.4.2.tar.gz b0d5e7e24aba1af4a8e1a4fa9c894c3a673bf5d8 git-htmldocs-1.8.4.2.tar.gz aebbb6dc8bca979f8d54bdef51b128deba195c94 git-manpages-1.8.4.2.tar.gz The following public repositories all have a copy of the v1.8.4.2 tag and the maint branch that the tag points at: url = https://kernel.googlesource.com/pub/scm/git/git url = git://repo.or.cz/alt-git.git url = https://code.google.com/p/git-core/ url = git://git.sourceforge.jp/gitroot/git-core/git.git url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core url = https://github.com/gitster/git Also, http://www.kernel.org/pub/software/scm/git/ has copies of the release tarballs. Git v1.8.4.2 Release Notes Fixes since v1.8.4.1 * "git clone" gave some progress messages to the standard output, not to the standard error, and did not allow suppressing them with the "--no-progress" option. * "format-patch --from=" forgot to omit unnecessary in-body from line, i.e. when is the same as the real author. * "git shortlog" used to choke and die when there is a malformed commit (e.g. missing authors); it now simply ignore such a commit and keeps going. * "git merge-recursive" did not parse its "--diff-algorithm=" command line option correctly. * "git branch --track" had a minor regression in v1.8.3.2 and later that made it impossible to base your local work on anything but a local branch of the upstream repository you are tracking from. * "git ls-files -k" needs to crawl only the part of the working tree that may overlap the paths in the index to find killed files, but shared code with the logic to find all the untracked files, which made it unnecessarily inefficient. * When there is no sufficient overlap between old and new history during a "git fetch" into a shallow repository, objects that the sending side knows the receiving end has were unnecessarily sent. * When running "fetch -q", a long silence while the sender side computes the set of objects to send can be mistaken by proxies as dropped connection. The server side has been taught to send a small empty messages to keep the connection alive. * When the webserver responds with "405 Method Not Allowed", "git http-backend" should tell the client what methods are allowed with the "Allow" header. * "git cvsserver" computed the permission mode bits incorrectly for executable files. * The implementation of "add -i" has a crippling code to work around ActiveState Perl limitation but it by mistake also triggered on Git for Windows where MSYS perl is used. * We made sure that we notice the user-supplied GIT_DIR is actually a gitfile, but did not do the same when the default ".git" is a gitfile. * When an object is not found after checking the packfiles and then loose object directory, read_sha1_file() re-checks the packfiles to prevent racing with a concurrent repacker; teach the same logic to has_sha1_file(). * "git commit --author=$name", when $name is not in the canonical "A. U. Thor " format, looks for a matching name from existing history, but did not consult mailmap to grab the preferred author name. * The commit object names in the insn sheet that was prepared at the beginning of "rebase -i" session can become ambiguous as the rebasing progresses and the repository gains more commits. Make sure the internal record is kept with full 40-hex object names. * "git rebase --preserve-merges" internally used the merge machinery and as a side effect, left merge summary message in the log, but when rebasing, there should not be a need for merge summary. * "git rebase -i" forgot that the comment character can be configurable while reading its insn sheet. Also contains a handful of trivial code clean-ups, documentation updates, updates to the test suite, etc. Changes since v1.8.4.1 are as follows: Antoine Pelisse (1): commit: search author pattern against mailmap Christian Couder (1): sha1_file: move comment about return value where it belongs Eric Sunshine (5): rebase -i: fix cases ignoring core.commentchar dir.c::test_one_path(): work around directory_exists_in_index_icase() breakage t3404: make tests more self-contained t3404: rebase -i: demonstrate short SHA-1 collision t3200: fix failure on case-insensitive filesystems Jeff King (8): has_sha1_file: re-check pack directory before giving up upload-pack: send keepalive packets during pack computation upload-pack: bump keepalive default to 5 seconds clone: send diagnostic messages t
Re: [PATCH v3 2/2] merge-base: teach "--fork-point" mode
John Keeping writes: > The --reflog name has the advantage that it makes clear that this is > looking at something more than the commit graph and I don't think > --fork-point does imply that. I think I understand what you are saying, but that "more than the commit graph" part in your reasoning is exactly one of the two reasons why I do not think that it is a good idea to call it with "reflog" in its name. The next round of update to the feature may find a better way to find the fork point than looking at the reflog. What the feature is meant to do, i.e. "find the fork point" can stay the same (i.e. people can use it in their script), while the way how the implementation achieves it (i.e. by looking at reflog) can evolve over time, and by not hardcoding "how" in the name, the users will benefit from the updated behaviour, without having to ask for a better heuristics by using a different option by updating all of their scripts. The other reason (of my two reasons) is because I do not think "finding the fork-point" will stay to be the _only_ feature that uses reflog in merge-base. When a next cool feature to compute something completely different from fork-point happens to be implemented by taking advantage of reflog data, what would we call it? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/15] fetch --tags: fetch tags *in addition to* other stuff
Michael Haggerty writes: >> True but when fetching other references, tags relevant to the >> history being fetched by default should automatically follow, so the >> above explains why "fetch --tags" is not a useful thing to do daily. > > Maybe not necessary in many scenarios, but is it harmful for the common > case of cloning from and then periodically fetching from an upstream? There is no mention of "harmful"; I only said "not useful". And it comes primarily from knowing why "--tags" was introduced in the first place; I should have said "less useful than before, ever since we started to reliably auto-follow". The only "harmful" part is its interaction with "--prune", which your series nicely addresses. > ISTM that the result of the declarative --tags option > > we have all upstream tags > > is easier to reason about than the history-dependent tag-following behavior I'd say "we have all the relevant tags" and "we have all the tags the other side has" are equally valid things to wish for, depending who you are and how your project is organized, and one is not necessarily easy/useful than the other. In case it was unclear, I was not seriously advocating to deprecate/remove "--tags". > Regarding your first point: if it matters which of the duplicates is > chosen, then it can only matter because they differ in some other way > than their reference names, for example in their flags. So a robust way > of de-duping them (if it is possible) would be to compare the two > records and decide which one should take precedence based on this other > information rather than based on which one happened to come earlier in > the list. Then the list order would be immaterial and (for example) it > wouldn't be a problem to reorder the items. But that changes the behaviour to those who has cared to rely on the ordering. With the change to first collect and then sort unstably before deduping, the series already tells them not to rely on the order, and two of us tentatively agreed in this discussion that it is not likely to matter. If later this agreement turns out to be a mistake and there are people who *do* rely on the ordering, the only acceptable fix for them is by making sure we restore the "first one trumps" semantics, not by defining an alternative, arguably better, behaviour---that is not a regression fix. In any case, thanks for working on this topic. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
Jonathan Nieder writes:
> Johannes Sixt wrote:
>
>> In other tests, we check for prerequisite PERL, i.e., we are prepared
>> that perl is not available. Shouldn't we do that here, too?
>
> I think the tests assume there's a perl present even when the PERL
> prereq isn't present already. E.g.:
>
> nul_to_q () {
> "$PERL_PATH" -pe 'y/\000/Q/'
> }
>
> So in practice the PERL prereq just means "NO_PERL wasn't set", or
> in other words, "commands that use perl work". Maybe something
> like the following would help?
>
> Signed-off-by: Jonathan Nieder
>
> diff --git i/t/README w/t/README
> index 2167125..54cd064 100644
> --- i/t/README
> +++ w/t/README
> @@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the
> "Test harness
> library" section above and the "test_have_prereq" function for how to
> use these, and "test_set_prereq" for how to define your own.
>
> - - PERL & PYTHON
> + - PYTHON
>
> - Git wasn't compiled with NO_PERL=YesPlease or
> - NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
> - these.
> + Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
> + need Python with this.
> +
> + - PERL
> +
> + Git wasn't compiled with NO_PERL=YesPlease.
> +
> + Even without the PERL prerequisite, tests can assume there is a
> + usable perl interpreter at $PERL_PATH, though it need not be
> + particularly modern.
> +
> + Wrap tests for commands implemented in Perl with this.
Sounds like a sensible thing to address, but I first parsed it as
Wrap (tests for (commands implemented in Perl)) with this.
while I think the readers are expected to parse it as
Wrap ((tests for commands) implemented in Perl) with this.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
Johannes Sixt wrote:
> In other tests, we check for prerequisite PERL, i.e., we are prepared
> that perl is not available. Shouldn't we do that here, too?
I think the tests assume there's a perl present even when the PERL
prereq isn't present already. E.g.:
nul_to_q () {
"$PERL_PATH" -pe 'y/\000/Q/'
}
So in practice the PERL prereq just means "NO_PERL wasn't set", or
in other words, "commands that use perl work". Maybe something
like the following would help?
Signed-off-by: Jonathan Nieder
diff --git i/t/README w/t/README
index 2167125..54cd064 100644
--- i/t/README
+++ w/t/README
@@ -629,11 +629,20 @@ See the prereq argument to the test_* functions in the
"Test harness
library" section above and the "test_have_prereq" function for how to
use these, and "test_set_prereq" for how to define your own.
- - PERL & PYTHON
+ - PYTHON
- Git wasn't compiled with NO_PERL=YesPlease or
- NO_PYTHON=YesPlease. Wrap any tests that need Perl or Python in
- these.
+ Git wasn't compiled with NO_PYTHON=YesPlease. Wrap any tests that
+ need Python with this.
+
+ - PERL
+
+ Git wasn't compiled with NO_PERL=YesPlease.
+
+ Even without the PERL prerequisite, tests can assume there is a
+ usable perl interpreter at $PERL_PATH, though it need not be
+ particularly modern.
+
+ Wrap tests for commands implemented in Perl with this.
- POSIXPERM
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid difference in tr semantics between System V and BSD
Am 28.10.2013 10:13, schrieb Ben Walton: > Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V > semantics for tr whereby string1's length is truncated to the length > of string2 if string2 is shorter. The BSD semantics, as used by GNU tr > see string2 padded to the length of string1 using the final character > in string2. POSIX explicitly doesn't specify the correct behavior > here, making both equally valid. > > This difference means that Solaris' native tr implementations produce > different results for tr ":\t\n" "\0" than GNU tr. This breaks a few > tests in t0008-ignores.sh. > > Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]". > > Instead, use perl to perform these transliterations which means we > don't need to worry about the difference at all. Since we're replacing > tr with perl, we also use perl to replace the sed invocations used to > transform the files. In other tests, we check for prerequisite PERL, i.e., we are prepared that perl is not available. Shouldn't we do that here, too? But OTOH, I think that we should skip as few test cases as possible in such a basic test as t0008. Just some food for thought... -- Hannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Change sed i\ usage to something Solaris' sed can handle
Ben Walton writes: > diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh > index 3fb4b97..0126154 100755 > --- a/t/t4015-diff-whitespace.sh > +++ b/t/t4015-diff-whitespace.sh > @@ -145,7 +145,8 @@ test_expect_success 'another test, with > --ignore-space-at-eol' 'test_cmp expect > test_expect_success 'ignore-blank-lines: only new lines' ' > test_seq 5 >x && > git update-index x && > - test_seq 5 | sed "/3/i \\ > + test_seq 5 | sed "/3/i\\ > +\ > " >x && Why do you need the \? Since it is inside double quotes the shell will remove it during expansion. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] graph.c: visual difference on subsequent series
Milton Soares Filho writes: > On 28 October 2013 13:41, Junio C Hamano wrote: >> I agree to all of the above, including the ugliness of 'x' ;-) >> >> A "blank" may however be hard to spot, if the range is limited, >> though. For example, > > A 'x' looks like termination points in some specification languages > such as SDL and MSC and thus translates directly to the idea of a > root-commit, at least IMO. For sure it does not stand out as blatantly > as it should, but it gives a general idea without further > distractions, which seems to be the idea of a simple 'git log --graph > --oneline'. > > An idea that have just come to mind is to have a decorator to enforce > this property, like this. > > * HEAD > /* a1 > | * a2 > | * a3 > | x a4 (root-commit) > * b1 > * b2 > x b3 (root-commit) > > This way the user only gets 'distracted' if he explicitly asks for it > (--decorate), with all its colors and whatnot. What do you think? > Should I aim for it? > > Besides anything else, this discussion is becoming very subjective. If I have to choose, I'd rather avoid using 'x' or anything that have to override '*', not just 'x' being ugly, but the approach to _replace_ the "revision-mark" (usually '*' but sometimes '<', '^', etc) forces us to give priority between "root-ness" and other kinds of information (e.g. "left-ness"). That was the primary reason I liked Keshav's suggestion to use one extra line _below_ the root, which will allow us to still keep the existing information unlike what we discussed in our back-and-forth during the initial review. I also think a blank (or divider) below the root commits does make it visually obvious that nothing comes _before_ the root commit in the history, which probably even removes the need to paint the tracks of histories leading to different roots in different colours. I hope the above shows that my reaction was much less subjective than my response sounded ;-) Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] graph.c: visual difference on subsequent series
On 28 October 2013 13:41, Junio C Hamano wrote: > I agree to all of the above, including the ugliness of 'x' ;-) > > A "blank" may however be hard to spot, if the range is limited, > though. For example, A 'x' looks like termination points in some specification languages such as SDL and MSC and thus translates directly to the idea of a root-commit, at least IMO. For sure it does not stand out as blatantly as it should, but it gives a general idea without further distractions, which seems to be the idea of a simple 'git log --graph --oneline'. An idea that have just come to mind is to have a decorator to enforce this property, like this. * HEAD /* a1 | * a2 | * a3 | x a4 (root-commit) * b1 * b2 x b3 (root-commit) This way the user only gets 'distracted' if he explicitly asks for it (--decorate), with all its colors and whatnot. What do you think? Should I aim for it? Besides anything else, this discussion is becoming very subjective. I've received private feedbacks thanking for the changeset and not a word against the poor 'x'. Maybe it's time to talk to a UI designer or let a benevolent dictator set this quarrel off ;-) []s, milton -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] inconsistent `git reflog show` output, possibly `git fsck` output
Junio C Hamano writes: > Roberto Tyley writes: >> On 21/09/2013 23:16, Keshav Kini wrote: >>> [SNIP] >>> This situation came about because the BFG Repo-Cleaner doesn't write new >>> reflog entries after creating its new objects and moving refs around. >> >> True enough - I don't think the BFG does write new entires to the >> reflog when it does the final ref-update, and it would be nicer if it >> did. I'll get that fixed. > > (sorry for replying late) > > So this can be closed as "BFG not writing reflog in a consistent > way, and 'git reflog show' is acting GIGO way"? Or was there > something the core side needs to do? Hi Junio, Below I'm resending a mail that I sent to the list earlier, but not to you or Roberto personally, as I just realized. So in case you didn't see it before, here it is -- if you did see it before, sorry for the noise. Hi Junio, Thanks for your reply. In my original mail, immediately after the snippet Roberto quoted above, I said, "But that aside, I think how git handles the situation might be a bug." To wit: > It seems to me that one of two things should be the case. Either 1) it > should be considered impossible to have a reflog for a ref X which > doesn't contain a chain of commits leading up to the current location of > X; or 2) if reflogs are allowed not to form an unbroken chain of commits > leading to X, then `git reflog show` should at least make sure to > actually display a commit ID corresponding to the second field of each > reflog entry it reads, and not some other commit ID. > > In the first case, the bug is that `git fsck` doesn't catch the > supposedly impossible situation that exists in the repository I've > described in this email. In the second case, the bug is that `git reflog > show` has bad output. Before this is closed, I would appreciate it if I could get some feedback from git developers on the above two paragraphs. Thanks, Keshav -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] graph.c: visual difference on subsequent series
Junio C Hamano writes: > [administrivia: please avoid culling addresses from To:/Cc: lines] Yikes, sorry about that. I've been sending messages through Gmane rather than via email, and I didn't realize the list didn't automatically send messages to the appropriate people who are only reading the list via actual email (as I am not such a person). > Keshav Kini writes: >> What about just putting an extra blank line after every root commit line >> (possibly except the last one)? That should make it plenty easy to see >> where the root commits are in --oneline mode. I think it would actually >> be easier to spot at a glance than replacing `*` with `x` because it >> creates a gap in all columns of the output, rather than only in column >> 1. Also, this is very subjective but I think it looks kind of ugly to >> use "x" :P > > I agree to all of the above, including the ugliness of 'x' ;-) > > A "blank" may however be hard to spot, if the range is limited, > though. For example, > > $ git log --graph --oneline a4.. > * HEAD > /* a1 > | * a2 > | * a3 > * b1 > * b2 > * b3 > > where "a4", which is a root, is the sole parent of "a3" and HEAD is > a merge between "a1" and "b1" might produce something like this, > while we may get this from the same history, when shown unlimited: > > $ git log --graph --oneline > * HEAD > /* a1 > | * a2 > | * a3 > | * a4 > | > * b1 > * b2 > * b3 > > A divider line might make it visually a lot more strong, i.e. > > $ git log --graph --oneline > * HEAD > /* a1 > | * a2 > | * a3 > | * a4 > | ~~~ > * b1 > * b2 > * b3 > > but I am not sure if it is too distracting. I would be fine with that, fwiw. We can also turn it on and off with a config option if people really don't like it, I suppose... -Keshav -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #06; Fri, 25)
Vicent Martí writes: > On Mon, Oct 28, 2013 at 4:48 PM, Junio C Hamano wrote: >>> jk/pack-bitmap adds khash.h, which from a first glance looks like yet >>> another hash table implementation. I was just wondering if kb's new >>> hash tables can cover the need of pack-bitmap.c too so we can remove >>> khash.h later.. >> ... > khash on the other hand is capable of storing the position values as > part of the hash table itself (i.e. `int **buckets`), and saves us > from thousands of bytes of allocations + indirection. My "Good thinking ;-)" comment was primarily meant as "somebody needs to at least think about the possibility and consider pros and cons", and you thought about it already ;-). In short, kb's hash table does not cover the need for pack-bitmap, so we should keep two at least for now, until (and/or unless) either side can be shown (and/or extended) to cover the need for the other one as well. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: fetch: tag following too ambitious?
Michael Haggerty writes: > When investigating the exact semantics of tag-following, I discovered > that the tag auto-following behavior of "git fetch" is more ambitious > than I would have expected: it fetches any tag that references an object > that is known to the local repository, *even if that object is not > currently reachable* (i.e., neither reachable before the fetch or after > the fetch of non-auto-followed references). This makes it hard to > renounce interest in a branch. > > Suppose there is a remote repo with > > o---o---o<- master > \ > o---A---B <- pu > > When I clone this repo, of course I get all of the commits and both > branches. > > Now suppose I decide I'm not interested in "branch" anymore, so I delete > its remote-tracking branch from my repository and change the config to > only fetch "master": > > git config remote.origin.fetch \ > '+refs/heads/master:refs/remotes/origin/master' > git update-ref -d refs/remotes/origin/pu > > It looks like I'm free of the "pu" branch, right? > > But if a week later somebody pushes a tag "t" to origin that points at > commit A, and then I do > > git fetch origin > > then Git (un)helpfully fetches tag "t" into my repo, because even though > commit "A" isn't reachable in my repo, it hasn't been pruned yet from > the object database. > > I admit this is not likely to be a serious problem in practice, but I > found it surprising and strangely disturbing. I would call it a bug. Sounds like a bug to me. Does upload-pack to pack-object codepath actually pack the tag object and give it to you, or is it done all by reconnecting an existing and dangling tag back to your ref namespace? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #06; Fri, 25)
On Mon, Oct 28, 2013 at 4:48 PM, Junio C Hamano wrote:
>> jk/pack-bitmap adds khash.h, which from a first glance looks like yet
>> another hash table implementation. I was just wondering if kb's new
>> hash tables can cover the need of pack-bitmap.c too so we can remove
>> khash.h later..
>
> Good thinking ;-).
We use the khash tables to map:
- sha1 (const char *) to (void *)
- sha1 (const char *) to int
The new `hashmap.c` covers the first case quite well (albeit slightly
more verbosely than I'd like), but in the second case it doesn't quite
work. Since the new hash needs to embed the "struct hashmap_entry" on
all its values (to allow for separate chaining), having it map to
`int` keys requires a struct like this:
struct sha1_position {
struct hashmap_entry {
struct hashmap_entry *next;
unsigned int hash;
};
int position;
}
khash on the other hand is capable of storing the position values as
part of the hash table itself (i.e. `int **buckets`), and saves us
from thousands of bytes of allocations + indirection.
I am not sure whether the consistency of having a single hash map
warrants the performance and memory hits when operating on the
extended index.
Please advice.
luv,
vmg
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Oct 2013, #06; Fri, 25)
Duy Nguyen writes: > On Sat, Oct 26, 2013 at 6:23 AM, Junio C Hamano wrote: >> * kb/fast-hashmap (2013-10-22) 12 commits >> - remove old hash.[ch] implementation >> - read-cache.c: fix memory leaks caused by removed cache entries >> - name-hash.c: remove cache entries instead of marking them CE_UNHASHED >> - name-hash.c: use new hash map implementation for cache entries >> - name-hash.c: remove unreferenced directory entries >> - name-hash.c: use new hash map implementation for directories >> - diffcore-rename.c: use new hash map implementation >> - diffcore-rename.c: simplify finding exact renames >> - diffcore-rename.c: move code around to prepare for the next patch >> - buitin/describe.c: use new hash map implementation >> - add a hashtable implementation that supports O(1) removal >> - submodule: don't access the .gitmodules cache entry after removing it >> >> Improvements to our hash table to get it to meet the needs of the >> msysgit fscache project, with some nice performance improvements. >> >> The preparatory clean-up to submodule from Jens is at the bottom. I >> also squashed in a fix-up by Karsten found at $gmane/236468 (please >> double-check the result). > > jk/pack-bitmap adds khash.h, which from a first glance looks like yet > another hash table implementation. I was just wondering if kb's new > hash tables can cover the need of pack-bitmap.c too so we can remove > khash.h later.. Good thinking ;-). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rev-parse --parseopt: add the --sticked-long mode
"Philip Oakley" writes: > Isn't the origin of the description that it looks like a stick (cane), > and 'sticked' is a modern verbing of that form? That's what I'd > assumed anyway. > > Googleing "Sticked option" only linked back to Git. I know web is not the authoritative source of information ;-) but it seems that looking for "sticked vs stuck" seems to give me many explanations that boils down to what this says: http://en.wiktionary.org/wiki/Talk:sticked and http://en.wiktionary.org/wiki/stick#English lists "sticked" as "archaic" when the word is used for "To glue; to attach; to adhere". -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] graph.c: visual difference on subsequent series
[administrivia: please avoid culling addresses from To:/Cc: lines] Keshav Kini writes: > What about just putting an extra blank line after every root commit line > (possibly except the last one)? That should make it plenty easy to see > where the root commits are in --oneline mode. I think it would actually > be easier to spot at a glance than replacing `*` with `x` because it > creates a gap in all columns of the output, rather than only in column > 1. Also, this is very subjective but I think it looks kind of ugly to > use "x" :P I agree to all of the above, including the ugliness of 'x' ;-) A "blank" may however be hard to spot, if the range is limited, though. For example, $ git log --graph --oneline a4.. * HEAD /* a1 | * a2 | * a3 * b1 * b2 * b3 where "a4", which is a root, is the sole parent of "a3" and HEAD is a merge between "a1" and "b1" might produce something like this, while we may get this from the same history, when shown unlimited: $ git log --graph --oneline * HEAD /* a1 | * a2 | * a3 | * a4 | * b1 * b2 * b3 A divider line might make it visually a lot more strong, i.e. $ git log --graph --oneline * HEAD /* a1 | * a2 | * a3 | * a4 | ~~~ * b1 * b2 * b3 but I am not sure if it is too distracting. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 10/19] pack-bitmap: add support for bitmap indexes
Jeff King writes: > On Fri, Oct 25, 2013 at 08:26:29PM -0400, Jeff King wrote: > >> On Fri, Oct 25, 2013 at 04:06:19PM -0700, Junio C Hamano wrote: >> >> > Jeff King writes: >> > >> > > diff --git a/pack-bitmap.c b/pack-bitmap.c >> > > new file mode 100644 >> > > index 000..73c52fd >> > > --- /dev/null >> > > +++ b/pack-bitmap.c >> > > @@ -0,0 +1,965 @@ >> > > +#include >> > > + >> > > +#include "cache.h" >> > >> > You among all people already know why this is bad, no? >> >> Yes, I am well aware of why we do not want it. I thought I removed that, >> but it appears I missed one. Sorry. > > In addition to that one, there are a few other system header includes: > > - the ewah/*.c files include the necessary standard headers, and do > not include git-compat-util.h at all. I do not really consider these > "git code", but rather a black box we have imported into the tree > to ease the dependency chain. So in that sense, they operate like > xdiff/*.c or compat/regex/*.c, which also compile on > their own (and can get away with it because they are mostly standard > C and do not do "system" things. Right; I didn't comment on the bare inclusions of system headers in these files exactly because I shared that reasoning. > However, the code in the ewah/ directory has been hacked up a bit > from its original, and ewah_io.c _does_ include "git-compat-util.h". > So it may make sense to consider our copy a fork and git-ify it > more. Yeah, sounds very sensible. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/15] fetch --prune: prune only based on explicit refspecs
Michael Haggerty writes: > On 10/24/2013 11:11 PM, Junio C Hamano wrote: > ... >> We should just lose "It is similar to using" from 10/15 and start >> over, perhaps? Add the first paragraph of the below in 10/15 and >> add the rest in 11/15, or something. >> >> --tags:: >> Request that all tags be fetched from the remote >> under the same name (i.e. `refs/tags/X` is created in >> our repository by copying their `refs/tags/X`), in >> addition to whatever is fetched by the same `git >> fetch` command without this option on the command >> line. >> + >> When `refs/tags/*` hierarchy from the remote is copied only >> because this option was given, they are not subject to be >> pruned when `--prune` option (or configuration variables >> like `fetch.prune` or `remote..prune`) is in effect. >> >> That would make it clear that they are subject to pruning when --mirror >> or an explicit refspec refs/tags/*:refs/tags/* is given, as tags are >> not fetched "only because of --tags" in such cases. > > I see your point. What do you think about the following version, which > is a bit more compact and refers the reader to --prune for the full story: > > -t:: > --tags:: > Fetch all tags from the remote (i.e., fetch remote tags > `refs/tags/*` into local tags with the same name), in addition > to whatever else would otherwise be fetched. Using this > option does not subject tags to pruning, even if --prune is > used (though tags may be pruned anyway if they are also the > destination of an explicit refspec; see '--prune'). I like the first sentence of yours better. The second sentence feels somewhat iffy, though. --tags refs/tags/*:refs/tags/* will allow tags to be pruned, so s/option does not/option alone does not/ needs to be done to be precise, at least. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] merge-base: teach "--fork-point" mode
Martin von Zweigbergk writes:
>> + bases = get_merge_bases_many(derived, revs.nr, revs.commit, 0);
>> + ...
>> + if (revs.nr <= i)
>> + return 1; /* not found */
>> +
>> + printf("%s\n", sha1_to_hex(bases->item->object.sha1));
>> + free_commit_list(bases);
>> + return 0;
>
> Should free_commit_list also be called in the two "return 1" cases
> above? I suppose the process will exit soon after this, but that seems
> to be true for all three cases.
You are right that the above is inconsistent. Because the code
intends to be called only once in the lifetime of the program,
it calls get_merge_bases_many() with cleanup set to 0, so in that
sense, not freeing them anywhere may make it even more clear that
this function expects to be shortly followed by a process exit.
>> diff --git a/t/t6010-merge-base.sh b/t/t6010-merge-base.sh
>> index f80bba8..4f09db0 100755
>> --- a/t/t6010-merge-base.sh
>> +++ b/t/t6010-merge-base.sh
>> @@ -230,4 +230,31 @@ test_expect_success 'criss-cross merge-base for
>> octopus-step' '
>> test_cmp expected.sorted actual.sorted
>> '
>>
>> +test_expect_success 'using reflog to find the fork point' '
>> + git reset --hard &&
>> + git checkout -b base $E &&
>> +
>> + (
>> + for count in 1 2 3 4 5
>> + do
>> + git commit --allow-empty -m "Base commit #$count" &&
>> + git rev-parse HEAD >expect$count &&
>> + git checkout -B derived &&
>> + git commit --allow-empty -m "Derived #$count" &&
>> + git rev-parse HEAD >derived$count &&
>> + git checkout base || exit 1
>
> I think this creates a history like
>
> ---E---B1--B2--B3--B4--B5 (base)
> \ \ \ \ \
> D1 D2 D3 D4 D5 (derived)
>
> So I think the following test would pass even if you drop the
> --fork-point. Did you mean to create a fan-shaped history by resetting
> base to $E on every iteration above?
Just showing that I didn't think things deeply ;-) I do agree that a
fan-shaped history would show what we want to do a lot better.
Thanks.
>> + git merge-base --fork-point base $(cat
>> derived$count) >actual &&
>> + test_cmp expect$count actual || exit 1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: [PATCH] git-subtree: Avoid using echo -n even indirectly
(Resending without HTML, so that it reaches the ML).
On Fri, Oct 11, 2013 at 11:32 AM, Paolo Giarrusso wrote:
>
> On Wed, Oct 9, 2013 at 11:11 PM, Jonathan Nieder wrote:
> > Paolo Giarrusso wrote:
> >
> >> Seeing the email, I wonder whether there's hope something like that
> >> can be preserved in an email, and whether the code should use some
> >> escape sequence instead.
> >
> > Yes, please. Mind if I amend it to
> >
> > printf "%s\r" "$revcount/$revmax ($createcount)" >&2
> >
> > ?
>
> Please do go ahead, by all means (arguably as a different commit, but
> those are minor details).
What happened? Did you go ahead, as you wrote? Is the patch somewhere?
Arguably it should go into the maint branch, but I think it didn't —
otherwise https://github.com/git/git/pull/61 should have stopped being
mergeable.
This also makes me wonder whether you use any tracker at all — but
unless there is one that I missed, that's a separate discussion.
> > [...]
> >>> say()
> >>> {
> >>> if [ -z "$quiet" ]; then
> >>> echo "$@" >&2
> >>>fi
> >>> }
> >
> > I agree with the other reviewers that this should be fixed to use
> > printf, too, but that's another topic.
> Seconded.
--
Paolo G. Giarrusso - Ph.D. Student, Philipps-University Marburg
http://www.informatik.uni-marburg.de/~pgiarrusso/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
fetch: tag following too ambitious?
When investigating the exact semantics of tag-following, I discovered that the tag auto-following behavior of "git fetch" is more ambitious than I would have expected: it fetches any tag that references an object that is known to the local repository, *even if that object is not currently reachable* (i.e., neither reachable before the fetch or after the fetch of non-auto-followed references). This makes it hard to renounce interest in a branch. Suppose there is a remote repo with o---o---o<- master \ o---A---B <- pu When I clone this repo, of course I get all of the commits and both branches. Now suppose I decide I'm not interested in "branch" anymore, so I delete its remote-tracking branch from my repository and change the config to only fetch "master": git config remote.origin.fetch \ '+refs/heads/master:refs/remotes/origin/master' git update-ref -d refs/remotes/origin/pu It looks like I'm free of the "pu" branch, right? But if a week later somebody pushes a tag "t" to origin that points at commit A, and then I do git fetch origin then Git (un)helpfully fetches tag "t" into my repo, because even though commit "A" isn't reachable in my repo, it hasn't been pruned yet from the object database. I admit this is not likely to be a serious problem in practice, but I found it surprising and strangely disturbing. I would call it a bug. Thoughts? Michael -- Michael Haggerty mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Mon, Oct 28, 2013 at 10:02 AM, Michael Haggerty wrote: > On 10/27/2013 08:14 AM, Josh Triplett wrote: >> On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: >>> On 10/27/2013 02:34 AM, Josh Triplett wrote: >>> I wonder if the two features could >>> be combined in some way? >>> >>> The main difference between the two features is how they are intended to >>> be used: --fixup is to fix a commit that hasn't been pushed yet (where >>> the user intends to squash the commits together), whereas --fixes is to >>> mark a commit as a fix to a commit that has already been pushed (where >>> the commits will remain separate). But there seems to be a common >>> concept here. >>> >>> For example, what happens if a --fixes commit is "rebase -i"ed at the >>> same time as the commit that it fixes? It might make sense to do the >>> autosquash thing just like with a --fixup/--squash commit. (Otherwise >>> the SHA-1 in the "Fixes:" line will become invalid anyway.) >> >> Most definitely not, no, at least not without an explicit option to >> enable that. Consider the case of backporting a series of patches and >> preserving the relative history of those patches, to make it easier to >> match up a set of patches. At most, it might be a good idea for >> cherry-pick or similar to provide an updated Fixes tag for the new hash >> of the older commit. Personally, I'd argue against doing this even with >> --autosquash. I could see the argument for an --autosquash-fixes, but I >> can't think of a real-world scenario where what would come up. >> >> Generally, if history is still editable, you should just squash in the >> fix to the original commit, and if history is no longer editable (which >> is the use case for "Fixes:" lines), the squash case simply won't come >> up, offering little point to adding special support for that case. > > In your last paragraph you explain exactly why these two features are > similar and why it is thinkable to make the way that they are handled > depend on the context. Exactly because one would never rebase a > "Fixes:" commit and the commit it is fixing at the same time, they would > never be squashed together. And ISTM that in most cases whenever they > *are* being rebased at the same time, then one would want to squash them > together. So it might be possible to mark both types of commits the > same way and then squash/not squash them depending on the context and > the --autosquash option. In general, we should be careful with introducing features that exhibit different consequences based on the context in which they are used, but in this case, I believe I agree with you. The existence of "Fixes:" in a commit should be a just as valid hint to --autosquash as a commit message starting with "fixup!" or "squash!" (obviously, the "Fixes:" commit should be handled like a "squash!" and not like a "fixup!", so that we don't haphazardly discard the commit message accompanying "Fixes:"). >>> I see that there a consistency check that the --fixes argument is a >>> valid commit. But is there/should there be a check that it is an >>> ancestor of the commit being created? Is there/should there be a check >>> that both of these facts remain true if the the commit containing it is >>> rebased, cherry-picked, etc? >> >> That sounds like a nice future enhancement, sure. I don't have any plans to >> add such a check myself, though. Also note that --fixup and --squash >> don't have such a check either; if you want to add one, you should add >> it for all three options at once. > > A hook-based solution could do this. But a built-in "all-purpose" > handler like "footer.Fixes.arg=commit", which was intended to be > reusable, wouldn't be able to do such footer-specific extra work without > having to create new special cases in git each time. Which begs the question (posed to all, not specifically to you): Why would we want solve this issue in config instead of in hooks? The hooks will always be more flexible and less dependent on making changes in git.git. (...a suitably flexible hook could even use the config options discussed above as input...) In both cases, we need the user to actively enable the functionality (either installing hooks, or setting up config), and in both cases we could bundle Git with defaults that solve the common cases, so that is not a useful differentiator between the two approaches. I would even venture to ask: If we end up solving this problem in config and not in hooks, then why do we bother having hooks in the first place? ...Johan -- Johan Herland, www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Well-past commit dates unsupported
Hi there, I'm trying to use a well-past date for a git commit, before the UNIX Epoch, but this does not work for the reasons below. I'm on Mac OS X 10.8, git version 1.8.3.4, and `sizeof(time_t) == 8`. The date I'm trying to set is October 4, 1958, that is around timestamp -354808800. First technique, using the commit `--date` flags with ISO 8601: it says "invalid date". Second technique, described in the git ml archive, not using the porcelain: http://article.gmane.org/gmane.comp.version-control.git/152497/ git commit git cat-file -p HEAD > tmp.txt # at this point, edit the file to replace the timestamp git hash-object -t commit -w tmp.txt #=> 2ee8fcc02658e23219143f5bcfe6f9a4615745f9 git update-ref -m 'commit: foo' refs/heads/master \ 2ee8fcc02658e23219143f5bcfe6f9a4615745f9 Commit date is effectively updated, but `git show` clamps the date to zero (`Jan 1 1970`). `tig(1)` displays `55 years ago` so the actual commit date is properly stored. Last issue: when trying to push this commit to a remote repository: #=> remote: error: object 2ee8fcc02658e23219143f5bcfe6f9a4615745f9:invalid # author/committer line - bad date #=> remote: fatal: Error in object #=> error: unpack failed: index-pack abnormal exit Finally, when running `test-date` from git sources: ./test-date show -354808800 #=> -354808800 -> in the future Is it a deliberate git behavior? Is fully supporting past or far future dates in the roadmap, since the referenced message above from 2010? Should I try to make a patch? Thanks! PS: use case for past dates is to match the historical law changes. As others did before, I'm trying to map the history of France Constitution to git. -- Jean Lauliac -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] git-fc, a friendly fork of Git
Hi, Why a fork? Well, the short answer is; my patches are not being applied. What is git-fc? It is a friendly fork, and by that I mean that it's a fork that won't deviate from the mainline, it is more like a branch. This branch will move forward close to Git's mainline, and it could be merged at any point in time, if the maintainer wished to do so. git-fc doesn't include experimental code, or half-assed features, so you can expect the same level of stability as Git's mainline. Also, it doesn't remove any feature, or do any backwards incompatible changes, so you can replace git with git-fc and you wouldn't notice the difference. The delta comes in the extra features, that is all. What do you get if you use git-fc? === @ shortcut === Many people have suggested a shortcut for the non-particularly-intuitive "HEAD", but none of these suggestions seemed very appealing, or feasible. Because Git already has an ref@op revision syntax, where if you remove the ref, HEAD is implied, I thought @ could be thought as HEAD. This change was welcome and accepted by the Git mainline, and it even was on track for v1.8.4 but it was dropped last minute because of some issues that are fixed now, and you probably will see it in v1.8.5. But why wait? :) === Nice 'branch -v' === If you have configured the upstream tracking branch for your branches (I wrote a blog post about them), when you do 'git branch -v' you see something like this: fc/branch/fast 177dcad [ahead 2] branch: reorganize verbose options fc/stageabb6ad5 [ahead 14] completion: update 'git reset' ... fc/transport/improv eb4d3c7 [ahead 10] transport-helper: don't update ... While that provides useful information, it doesn't show the upstream tracking branch, just says "ahead 2" but "ahead 2" compared to what? If you do 'git branch -vv', then you see the answer: fc/branch/fast 177dcad [master: ahead 2] branch: reorganize ... fc/stageabb6ad5 [master: ahead 14] completion: update ... fc/transport/improv eb4d3c7 [master: ahead 10] transport-helper: don't ... Unfortunately both options take a lot of time (relative to most Git commands which are instantaneous), because computing the "ahead 2" takes a lot of time. So I decided to switch things around, so 'git branch -v' gives you: fc/branch/fast 177dcad [master] branch: reorganize verbose options fc/stageabb6ad5 [master] completion: update 'git reset' new ... fc/transport/improv eb4d3c7 [master] transport-helper: don't update refs ... And it does so instantaneously. === Default aliases === Many (if not all) version control system tools have shortcuts for their most common operations; hg ci, svn co, cvs st. But not Git. You can configure your own aliases manually, but you might have some trouble if you use somebody else's machine. Adding default aliases is trivial, it helps everyone, and it doesn't hurt anyone, yet the patch to do so was rejected. For now, there are only four aliases, but more can be added later if they are requested. co = checkout ci = commit rb = rebase st = status If you have already these aliases, or mapped to something else, your aliases would take precedence over the default ones, so you won't have any problems. === Streamlined remote helpers === I have spent a lot of time working on git-remote-hg and git-remote-bzr, and although they are relatively new, they have proven to be quite stable and solid, yet they are only part of the "contrib" area side by side with much simpler and way less solid scripts. In order these in Git mainline you might need a bit of tinkering, and it's not straight-forward to package them for distributions. With git-fc they are installed by default, and in the right way, making things easier for distributions. === Improvements to the transport helper === The two way bridges between Git and Mercurial/Bazaar already work quite well, but they lack some features, specifically you cannot do --force, or --dry-run, or use an old:new refspec. If you are not familiar with the old:new refspec; you can do 'git push master:my-master', which would push your 'master' branch, as if it was named 'my-master' in the remote repository. This is extremely useful if you are really serious about using Git as a transparent client to access a Mercurial repository. === New core.mode configuration === Git is already preparing users for the v2.0 release which would bring minor backward compatibility breakage, but some people would rather get rid of the warnings which are going to stay probably for many releases more and just move to the new behavior already. Testing Git v2.0 behavior today would not only help git-fc, but also the Git mainline, and you can do that by setting core.mode = next, so if you do this and provide feedback about any issues, that would be greatly appreciated. Unfortunately you cannot test the v2.0 behavior in Git mainline because they rejected the patches, but you can in git-fc
[PATCH] Avoid difference in tr semantics between System V and BSD
Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) uses the System V
semantics for tr whereby string1's length is truncated to the length
of string2 if string2 is shorter. The BSD semantics, as used by GNU tr
see string2 padded to the length of string1 using the final character
in string2. POSIX explicitly doesn't specify the correct behavior
here, making both equally valid.
This difference means that Solaris' native tr implementations produce
different results for tr ":\t\n" "\0" than GNU tr. This breaks a few
tests in t0008-ignores.sh.
Possible fixes for this are to make string2 be "\0\0\0" or "[\0*]".
Instead, use perl to perform these transliterations which means we
don't need to worry about the difference at all. Since we're replacing
tr with perl, we also use perl to replace the sed invocations used to
transform the files.
Replace four identical transforms with a function named
broken_c_unquote. Replace the other two identical transforms with a
fuction named broken_c_unquote_verbose.
Signed-off-by: Ben Walton
---
t/t0008-ignores.sh | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 181513a..4ebda99 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -37,6 +37,14 @@ test_stderr () {
test_cmp "$HOME/expected-stderr" "$HOME/stderr"
}
+broken_c_unquote () {
+ perl -pe 's/^"//; s/\\//; s/"$//; tr/\n/\0/' "$@"
+}
+
+broken_c_unquote_verbose () {
+ perl -pe 's/"/ /; s/\\//; s/"$//; tr/:\t\n/\0/' "$@"
+}
+
stderr_contains () {
regexp="$1"
if grep "$regexp" "$HOME/stderr"
@@ -606,12 +614,11 @@ cat <<-EOF >expected-verbose
$global_excludes:2:!globaltwo b/globaltwo
EOF
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin' '
expect_from_stdin expected-verbose
sed -e 's/.* //' expected-verbose >expected-default
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' stdin | \
- tr "\n" "\0" >stdin0
-sed -e 's/^"//' -e 's/\\//' -e 's/"$//' expected-default | \
- tr "\n" "\0" >expected-default0
-sed -e 's/ "/ /' -e 's/\\//' -e 's/"$//' expected-verbose | \
- tr ":\t\n" "\0" >expected-verbose0
+broken_c_unquote stdin >stdin0
+
+broken_c_unquote expected-default >expected-default0
+
+broken_c_unquote_verbose expected-verbose >expected-verbose0
test_expect_success '--stdin from subdirectory' '
expect_from_stdin http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On 10/27/2013 08:14 AM, Josh Triplett wrote: > On Sun, Oct 27, 2013 at 06:42:44AM +0100, Michael Haggerty wrote: >> On 10/27/2013 02:34 AM, Josh Triplett wrote: >> [...] >> First of all, let me show my ignorance. How formalized is the use of >> metadata lines at the end of a commit message? I don't remember seeing >> documentation about such lines in general (as opposed to documentation >> about particular types of lines). Is the format defined well enough >> that tools that don't know about a particular line could nonetheless >> preserve it correctly? Is there/should there be a standard recommended >> order of metadata lines? (For example, should "Fixes:" lines always >> appear before "Signed-off-by" lines, or vice versa?) If so, is it >> documented somewhere and preserved by tools when such lines are >> added/modified? Should there be support for querying such lines? > > While it isn't very well documented in git itself, metadata lines are > quite standardized. See Documentation/SubmittingPatches and > Documentation/development-process/5.Posting in the Linux kernel, for an > explanation of "Reported-by:", "Tested-by:", "Reviewed-by:", > "Suggested-by:", and "Acked-by:". And git itself looks for a very > specific format; the has_conforming_footer function looks for a footer > consisting exclusively of rfc2822-style (email-style) header lines to > decide whether to append "Signed-off-by:" (and now "Fixes:") directly to > that block or to create a new block. It would be nice to document exactly what "rfc2822-style" means in this context (e.g., are line breaks supported? Encoding changes? etc.) so that (1) new inventors of trailer lines can make sure that they conform to what Git expects and (2) Git could someday add some generic facilities for handling these fields (e.g., adding/removing/tidying them in a commit-msg hook; grepping through them by name) and be relatively sure that it is not breaking somebody's metadata. I'm not saying that it's your job; only that it would be helpful for ideas like yours. > [...] >> I wonder if the two features could >> be combined in some way? >> >> The main difference between the two features is how they are intended to >> be used: --fixup is to fix a commit that hasn't been pushed yet (where >> the user intends to squash the commits together), whereas --fixes is to >> mark a commit as a fix to a commit that has already been pushed (where >> the commits will remain separate). But there seems to be a common >> concept here. >> >> For example, what happens if a --fixes commit is "rebase -i"ed at the >> same time as the commit that it fixes? It might make sense to do the >> autosquash thing just like with a --fixup/--squash commit. (Otherwise >> the SHA-1 in the "Fixes:" line will become invalid anyway.) > > Most definitely not, no, at least not without an explicit option to > enable that. Consider the case of backporting a series of patches and > preserving the relative history of those patches, to make it easier to > match up a set of patches. At most, it might be a good idea for > cherry-pick or similar to provide an updated Fixes tag for the new hash > of the older commit. Personally, I'd argue against doing this even with > --autosquash. I could see the argument for an --autosquash-fixes, but I > can't think of a real-world scenario where what would come up. > > Generally, if history is still editable, you should just squash in the > fix to the original commit, and if history is no longer editable (which > is the use case for "Fixes:" lines), the squash case simply won't come > up, offering little point to adding special support for that case. In your last paragraph you explain exactly why these two features are similar and why it is thinkable to make the way that they are handled depend on the context. Exactly because one would never rebase a "Fixes:" commit and the commit it is fixing at the same time, they would never be squashed together. And ISTM that in most cases whenever they *are* being rebased at the same time, then one would want to squash them together. So it might be possible to mark both types of commits the same way and then squash/not squash them depending on the context and the --autosquash option. > [...] >> I see that there a consistency check that the --fixes argument is a >> valid commit. But is there/should there be a check that it is an >> ancestor of the commit being created? Is there/should there be a check >> that both of these facts remain true if the the commit containing it is >> rebased, cherry-picked, etc? > > That sounds like a nice future enhancement, sure. I don't have any plans to > add such a check myself, though. Also note that --fixup and --squash > don't have such a check either; if you want to add one, you should add > it for all three options at once. A hook-based solution could do this. But a built-in "all-purpose" handler like "footer.Fixes.arg=commit", which was intended to be reusable, would
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
Junio C Hamano writes:
> There are unbound number of kinds of trailers people would want to
> add, depending on their projects' needs. We should not have to add
> a specific support for a tailer like this one, before thinking
> through to see if we can add generic support for adding arbitrary
> trailers to avoid code and interface bloat.
>
> Think of the existing --signoff as a historical mistake. Such a
> generic "adding arbitrary trailers" support, when done properly,
> should be able to express what "--signoff" does, and we should be
> able to redo "--signoff" as a special case of that generic "adding
> arbitrary trailers" support, and at that point, "Fixes:" trailer the
> kernel project wants to use should fall out as a natural consequence.
Thinking aloud further, what I had in mind was along the lines of
the following.
* The most generic external interface would be spelled as
--trailer [=]
where can be things like "signoff", "closes", "acked-by",
"change-id", "fixes", etc.; they can be taken from an unbounded
set. The historical "--signoff" can become a short-hand for
"--trailer signoff". More than one "--trailer" option can be
given on a single command line.
* The token is used to look into the configuration, e.g.,
[commitTrailer "signoff"]
style = append-norepeat
trailer = Signed-off-by
command = echo "$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL>"'
[commitTrailer "change-id"]
style = append-only-if-missing
trailer = Change-Id
command = 'git hash-object -t commit --stdin <$GIT_PROTO_COMMIT'
[commitTrailer "fixes"]
style = overwrite
trailer = Fixes
command = 'git log -1 --oneline --format="%h (%s)" --abbrev-commit=14
$ARG'
where
- "commitTrailer..style" defines the interaction with
existing trailer of the same kind (e.g. S-o-b: accumulates by
appending, but we try not to repeat the same sign-off twice
which would show you forwarding your own message you are the
last person in the Sign-off chain; Fixes: if there is already
one will remove the old one and replaces; etc.);
- "commitTrailer..trailer" defines the trailer label at
the beginning of the trailer line;
- "commitTrailer..command" gives the command to run to
obtain the payload after the "trailer" label. A handful
obvious and useful variables are exported for the command to
use, and is exported as $ARG, if present.
With the most generic syntax, with the above commitTrailer.fixes.*
configuration, I would imagine that you can say something like:
git commit --trailer fixes="v2.6.12^{/^i386: tweak frobnitz}"
to say that the first commit you find traversing the history of
v2.6.12 whose title is "i386: tweak frobnitz" was faulty, and you
are creating a commit that corrects its mistake.
Giving some default configuration to often used trailer types
(e.g. configuration for "--trailer signoff") and promoting some
commonly used ones into a separate built-in option (e.g. an option
"--signoff" that does not have to say "--trailer signoff") are
entirely separate issues, and only time can nudge us into evaluating
individual types of trailers.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid broken Solaris tr
On Tue, Jun 18, 2013 at 11:31 PM, Junio C Hamano wrote:
Sorry for the very slow reply. This got lost in my inbox and I forgot about it.
> Ben Walton writes:
>
>> Solaris' tr (both /usr/bin/ and /usr/xpg4/bin) fail to handle the case
>> where the first argument is a multi-character set and the second is a
>> single null character.
>
> Almost all the tr invocations look like converting LF to NUL, except
> for two that squash a colon ':', HT and LF all to NUL. Is Solaris's
> tr fine with the former but not the latter?
In retrospect, this isn't brokenness, just a difference in System V vs
BSD semantics for tr, both of which are allowed by POSIX since the
behaviour in question is specifically unspecified by the standard. The
System V behaviour is to require a 1:1 map between string1 and string2
transformations whereas BSD behaviour (when len(string2) <
len(string1)) is to pad string2 with the last character in string2
until the lengths are equal.
>
>> We make this change globally in t0008-ignores instead of just for the
>> cases where it matters in order to maintain consistency.
>
> I am not suggesting to keep 'tr "\n" "\0"', but just wanted to make
> sure I am reading the first paragraph correctly. If we are
> rewriting, we should do so consistently.
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' stdin >stdin0
>
> What is -pne? Is it the same as -pe?
>
> tr/\n/\0/ (or y/\n/\0/) may be more faithful to the original.
>
>
>> +perl -pne 's/^"//; s/\\//; s/"$//; s/\n/\0/g' expected-default > \
>> +expected-default0
>
> Ditto. We may want to give the same script used in the above two
> (and twice again in the later hunk) more descriptive name, e.g.
>
> broken_c_unquote () {
> perl -pe '... that script ...' "$@"
> }
>
> broken_c_quote stdin >stdin0
>
> Side note: the script is broken as a generic C-unquote function in
> multiple ways. It does not work if it has more than one backslash
> quoted characters, it does not understand \t, \b, \015, \\, etc. to
> name two.
>
> But the breakage does not matter for the strings used in the test
> vector.
I've updated the patch and will forward it shortly.
Thanks
-Ben
--
---
Take the risk of thinking for yourself. Much more happiness,
truth, beauty and wisdom will come to you that way.
-Christopher Hitchens
---
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ksummit-attendees] [PATCH] commit: Add -f, --fixes option to add Fixes: line
Btw, can we please take away this discussion from ksummit-attendees? It's got absolutely nothing to do with kernel summit and is getting fairly annoying. Thanks! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On 10/28/2013 08:16 AM, Josh Triplett wrote:
> On Sun, Oct 27, 2013 at 06:52:18PM -0700, Junio C Hamano wrote:
>> There are unbound number of kinds of trailers people would want to
>> add, depending on their projects' needs. We should not have to add
>> a specific support for a tailer like this one, before thinking
>> through to see if we can add generic support for adding arbitrary
>> trailers to avoid code and interface bloat.
>>
>> Think of the existing --signoff as a historical mistake. Such a
>> generic "adding arbitrary trailers" support, when done properly,
>> should be able to express what "--signoff" does, and we should be
>> able to redo "--signoff" as a special case of that generic "adding
>> arbitrary trailers" support, and at that point, "Fixes:" trailer the
>> kernel project wants to use should fall out as a natural consequence.
>
> Well, the add_signoff_extra function I added makes it easy to add any
> kind of trailing data you want to a commit; the question just becomes
> what the UI looks like to drive that.
>
> Would you be OK with a solution that pushes the specific supported
> footer lines into git's configuration, and then supplies default
> configuration for common cases such as Fixes? The option could become
> -f/--footer, and the configuration would specify how to parse various
> arguments of -f and turn them into something. For example:
>
> [footer "Fixes"]
> abbrev = f
> arg = commit
> format = %h ('%s')
It could be even more decoupled, for example like this:
[footer "Fixes"]
type = pipe
cmd = awk '{ print $1 }' | git log --stdin --no-walk --abbrev=12
--pretty=format:\"Fixes: %h ('%s')\"
Note that the command is written to be idempotent; that way git could
re-pipe the old value(s) of the footer though the command if necessary.
And it can handle multiple lines, since some callback scripts might
want to see all of them at once.
Michael
--
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] commit: Add -f, --fixes option to add Fixes: line
On Sun, Oct 27, 2013 at 06:52:18PM -0700, Junio C Hamano wrote:
> There are unbound number of kinds of trailers people would want to
> add, depending on their projects' needs. We should not have to add
> a specific support for a tailer like this one, before thinking
> through to see if we can add generic support for adding arbitrary
> trailers to avoid code and interface bloat.
>
> Think of the existing --signoff as a historical mistake. Such a
> generic "adding arbitrary trailers" support, when done properly,
> should be able to express what "--signoff" does, and we should be
> able to redo "--signoff" as a special case of that generic "adding
> arbitrary trailers" support, and at that point, "Fixes:" trailer the
> kernel project wants to use should fall out as a natural consequence.
Well, the add_signoff_extra function I added makes it easy to add any
kind of trailing data you want to a commit; the question just becomes
what the UI looks like to drive that.
Would you be OK with a solution that pushes the specific supported
footer lines into git's configuration, and then supplies default
configuration for common cases such as Fixes? The option could become
-f/--footer, and the configuration would specify how to parse various
arguments of -f and turn them into something. For example:
[footer "Fixes"]
abbrev = f
arg = commit
format = %h ('%s')
git commit -f Cc:sta...@vger.kernel.org -f f:bad-commit ...
The Cc line there would go unparsed since there's no specific support
for it, while the 'f:bad-commit' would get expanded by the configuration
above to parse bad-commit as a committish and format it using the
specified pretty format.
Look reasonable? I could start out by adding support for footer lines
that take commits as arguments and format them using arbitrary pretty
strings, and leave room for future expansion to support footers that
reference idents (given some way to expand idents from some shorter
form, otherwise there's no point).
- Josh Triplett
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
