* Re: [PATCH] transport-helper: remove duplicate free()
From: Daniel Barkalow @ 2009-12-02 16:23 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git, Sverre Rabbelier, Junio C Hamano
In-Reply-To: <20091202153950.c18095b7.rctay89@gmail.com>
On Wed, 2 Dec 2009, Tay Ray Chuan wrote:
> Remove the free() invocation on transport->data in release_helper(), as
> disconnect_helper() has already done so.
You need to remove the one in disconnect_helper, because the entire point
of disconnect_helper as opposed to release_helper is to *not* free that
memory. If you remove this one, you'll access freed memory in any case
where the helper has to be quit and restarted.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format
From: Junio C Hamano @ 2009-12-02 16:45 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Junio C Hamano, Horst H. von Brand, git
In-Reply-To: <4B163B49.4070606@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> This made me wonder a bit: Do we have a policy regarding the use of
> "git-command" vs. "git command" in git shell scripts such as this one?
Yes. Anything that sets GIT_EXEC_PATH correctly can use git-foo form.
^ permalink raw reply
* Re: Git GUI client SmartGit released
From: Shawn O. Pearce @ 2009-12-02 16:48 UTC (permalink / raw)
To: Marc Strapetz; +Cc: git
In-Reply-To: <4B161B15.2020106@syntevo.com>
Marc Strapetz <marc.strapetz@syntevo.com> wrote:
> We are proud to announce the general availability of our Git client
> SmartGit[1]:
>
> http://www.syntevo.com/smartgit/index.html
Congrats on your release.
I noticed you use JGit and the Trilead SSH client.
I'm curious, did you guys replace JSch because its a pile of junk?
Did you patch JGit to use Trilead SSH instead of JSch? If so,
would you be interested in contributing that change back to JGit?
I'm rather fed up with JSch... :-)
--
Shawn.
^ permalink raw reply
* Re: Marking commits as transitory for git bisect?
From: Junio C Hamano @ 2009-12-02 16:49 UTC (permalink / raw)
To: David Kastrup; +Cc: Michael J Gruber, git
In-Reply-To: <87tyw9wsfr.fsf@lola.goethe.zz>
David Kastrup <dak@gnu.org> writes:
> Further comments/insights?
Just make sure that you state in the log message of the commits in the
series something like:
- The way the series is split for easier review has to break X and Y
and this commit does not even compile;
- This is the second half of restructuring, and it compiles again;
- Now A is fully restructured, test X works again, but Y is still broken;
- This finally completes the series and everything works again.
for _human consumption_. Bisect is not the only thing that needs to know
about these commits that are intentionally broken. The reviewers and
people who test before accepting the history with these commits in their
history need to know about the known breakages, too.
Because you are talking about breakages that are _known_ when you make the
series, you can afford to follow this recommendation. Object replacement
mechanism is _not_ the answer---it's whole point is that you can add it as
an afterthought by (virtually) squashing the series into one.
This way, the initial reviewers and a person who happened to hit such an
intentionally broken commit during bisect can deal with the situation
exactly the same way. I.e. with "git log".
^ permalink raw reply
* Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports
From: Shawn O. Pearce @ 2009-12-02 17:04 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091202055543.GC31244@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:
> > Why both connect-r and invoke-r? Why isn't connect-r sufficient
> > here? Isn't it sufficient for any service that runs over git:// ?
>
> Invoke supports those --upload-pack &co (a'la ssh://). connect
> doesn't (a'la to git://).
Drop invoke-r.
Modify transport-helper.c to allow pushing TRANS_OPT_UPLOADPACK and
TRANS_OPT_RECEIVEPACK down into the helper via the option capability.
I'd rename connect-r to just connect.
For the command line:
$ git fetch --upload-pack='/path to my /git-upload-pack' origin
The conversation with the helper will be:
> capabilities
< option
< connect
<
> option uploadpack /path to my /git-upload-pack
< ok
> connect git-upload-pack
<
Which gives the helper the full information it needs to pass along
the --upload-pack command line argument to the remote system.
--
Shawn.
^ permalink raw reply
* Re: What is the status of merging with whitespace conflicts?
From: Junio C Hamano @ 2009-12-02 17:04 UTC (permalink / raw)
To: Martin Langhoff; +Cc: Git Mailing List
In-Reply-To: <46a038f90912020234g3094cccbxd8ca80c7c0b32ca4@mail.gmail.com>
I am not working on it, and I didn't hear from anybody who is. If you
limit the whitespace-only changes to the ones that do not cross line
boundary, it wouldn't be too hard to add. Probably it would take less
than one summer for a good student.
^ permalink raw reply
* Re: [BUG?] git-merge appends extra string to user's message?
From: Junio C Hamano @ 2009-12-02 17:09 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: git, Miklos Vajna
In-Reply-To: <20091202192030.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> When "git merge" is run with a message, it appends an extra message
> that it would add if it were run without one....
>
> I gave my own message because I didn't want git to add the
> standard message (if I wanted to, I wouldn't have given one,
> or I would have prepared it using git-fmt-merge-msg command).
>
> Is it possible to tell git-merge not to do this, or will it
> break compatibility and is a bad idea to change it?
>
> I noticed that this was proposed before:
>
> http://thread.gmane.org/gmane.comp.version-control.git/77965/focus=80059
>
> you were in favor of changing this behavior, and there was no objection
> from the list.
I am still in favor, and I think we should do this change. I know Miklos
said it is intentional, but I think he merely means "The version rewritten
in C does so intentionally because the version before rewrite did so", not
necessarily because "the version before rewrite did so intentionally with
good reasons."
Especially because we are going to update "merge" and eventually drop the
"merge <msg> HEAD <commit>..." syntax, the first step should be to make
it equivalent to "merge -m <msg> <commit..."
How does this look?
I stole a major part of your message to the proposed commit log.
-- >8 --
Subject: [PATCH] merge: do not add standard message when message is given with -m option
Even if the user explicitly gave her own message to "git merge", the
command still added its standard merge message. It resulted in a
useless repetition like this:
% git merge -m "Merge early part of side branch" `git rev-parse side~2`
% git show -s
commit 37217141e7519629353738d5e4e677a15096206f
Merge: e68e646 a1d2374
Author: しらいし ななこ <nanako3@lavabit.com>
Date: Wed Dec 2 14:33:20 2009 +0900
Merge early part of side branch
Merge commit 'a1d2374f8f52f4e8a53171601a920b538a6cec23'
The gave her own message because she didn't want git to add the
standard message (if she wanted to, she wouldn't have given one,
or she would have prepared it using git-fmt-merge-msg command).
Noticed by Nanako Shiraishi
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin-merge.c | 14 ++++++++------
t/t7604-merge-custom-message.sh | 7 ++-----
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/builtin-merge.c b/builtin-merge.c
index 2104577..981fe4b 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -71,7 +71,7 @@ static int option_parse_message(const struct option *opt,
if (unset)
strbuf_setlen(buf, 0);
else if (arg) {
- strbuf_addf(buf, "%s\n\n", arg);
+ strbuf_addf(buf, "%s%s", buf->len ? "\n\n" : "", arg);
have_message = 1;
} else
return error("switch `m' requires a value");
@@ -943,11 +943,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* codepath so we discard the error in this
* loop.
*/
- for (i = 0; i < argc; i++)
- merge_name(argv[i], &msg);
- fmt_merge_msg(option_log, &msg, &merge_msg);
- if (merge_msg.len)
- strbuf_setlen(&merge_msg, merge_msg.len-1);
+ if (!have_message) {
+ for (i = 0; i < argc; i++)
+ merge_name(argv[i], &msg);
+ fmt_merge_msg(option_log, &msg, &merge_msg);
+ if (merge_msg.len)
+ strbuf_setlen(&merge_msg, merge_msg.len-1);
+ }
}
if (head_invalid || !argc)
diff --git a/t/t7604-merge-custom-message.sh b/t/t7604-merge-custom-message.sh
index de977c5..269cfdf 100755
--- a/t/t7604-merge-custom-message.sh
+++ b/t/t7604-merge-custom-message.sh
@@ -22,15 +22,12 @@ test_expect_success 'setup' '
git tag c2
'
-cat >expected <<\EOF
-custom message
-Merge commit 'c2'
-EOF
test_expect_success 'merge c2 with a custom message' '
git reset --hard c1 &&
+ echo >expected "custom message" &&
git merge -m "custom message" c2 &&
- git cat-file commit HEAD | sed -e "1,/^$/d" > actual &&
+ git cat-file commit HEAD | sed -e "1,/^$/d" >actual &&
test_cmp expected actual
'
--
1.6.6.rc0
^ permalink raw reply related
* Re: [PATCH] builtin-commit: add --date option
From: Miklos Vajna @ 2009-12-02 17:11 UTC (permalink / raw)
To: git
In-Reply-To: <1259627252-21615-1-git-send-email-vmiklos@frugalware.org>
[-- Attachment #1: Type: text/plain, Size: 587 bytes --]
On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
> +--date=<date>::
> + Override the date used in the commit. The format is the Git
> + native one and is `<time> SP <offutc>`.
I just noticed that fmt_ident() calls parse_date(), so other formats are
supported as well.
Is there any documentation describing what does parse_date() accept?
Based on t0006-date.sh and the comments in the source, I see 3 supported
formats:
1) <unix timestamp> <timezone>
2) A format like 2008-12-02 18:04:00
3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [RFC PATCH 4/8] Support remote helpers implementing smart transports
From: Shawn O. Pearce @ 2009-12-02 17:12 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091202055543.GC31244@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> On Tue, Dec 01, 2009 at 11:22:33AM -0800, Shawn O. Pearce wrote:
>
> > Why 'OK'? Currently remote-helpers return an empty blank line
> > to any successful command, not 'OK'.
>
> Changed to "" (i.e. blank line) for next round.
Arrrgh. Just to correct myself... the 'option' command uses 'ok',
'error', 'unsupported' as its response messages. Which means
'option' breaks the blank-line-is-ok convention I tried to hold
you to above.
I consider this a mistake on my part. 'option' should respond with
a blank line on success just like 'fetch' or 'push' does.
--
Shawn.
^ permalink raw reply
* Re: [BUG?] git-merge appends extra string to user's message?
From: Miklos Vajna @ 2009-12-02 17:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nanako Shiraishi, git
In-Reply-To: <7veindi9pe.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 453 bytes --]
On Wed, Dec 02, 2009 at 09:09:49AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
> I am still in favor, and I think we should do this change. I know Miklos
> said it is intentional, but I think he merely means "The version rewritten
> in C does so intentionally because the version before rewrite did so", not
> necessarily because "the version before rewrite did so intentionally with
> good reasons."
Exactly.
> How does this look?
Looks fine.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* warning in git version 1.6.6.rc0.114.gc8648
From: Alejandro Riveira @ 2009-12-02 17:21 UTC (permalink / raw)
To: git
Whenever i do a "git pull" to check updates to the kernel
or git repo i recieve
warning: 'git merge <msg> HEAD <commit>' is deprecated. Please update
your script to use 'git merge -m <msg> <commit>' instead.
In future versions of git, this syntax will be removed.
Already up-to-date.
git pull should be updated to not use a deprecated form of git merge.
Regards
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Junio C Hamano @ 2009-12-02 17:26 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: git
In-Reply-To: <20091202160446.GA32667@Knoppix>
Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> On Tue, Dec 01, 2009 at 10:35:58PM -0800, Junio C Hamano wrote:
>> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
>>
>> I didn't mean the line count by "large". I was referring to the size of
>> change at the conceptual level. As Daniel already explained, it has been
>> one of the design assumption so far that there are built-in mappings from
>> some common <scheme>:// to backend "helpers".
>
> No implicit mappings from <scheme>:// to helpers existed before this series
> (except for forcing in URL, which are different). Thus, any mapping had to
> be explicit and built-in.
> ...
> So instead of mapping explicitly, those are effectively mapped by filesystem
> (that's after the fixes for next round that make helpers hardlinked instead
> of copied).
Sure, it may be a good change; didn't I say that in the part you omitted
from your quote? But it is conceptually a big change nevertheless, and it
is to be debated --- that makes the parts leading to 6/8 rather a large
change, which was my primary point in the message you are responding to,
so I think we are in agreement that it would have been nicer if the other
bits that are independently useful (like the debug one) were earlier in
the series.
The other minor point in my message was that this is to be debated (which
I think you are doing now), but I am not the best person to debate the
design of this part with. Daniel and Shawn are the guys who have
primarily worked on the helper interface and relationship between the
helpers and the transport layer, and will have much better insights.
By the way, are you the helpful git expert we often see on #git IRC
channel who goes by the same name? Welcome to the list; I don't remember
seeing you here, and let me thank you for having helped many new to
intermediate git users over there for a long time.
^ permalink raw reply
* Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format
From: Michael J Gruber @ 2009-12-02 17:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Horst H. von Brand, git
In-Reply-To: <7vws15jpe7.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 02.12.2009 17:45:
> Michael J Gruber <git@drmicha.warpmail.net> writes:
>
>> This made me wonder a bit: Do we have a policy regarding the use of
>> "git-command" vs. "git command" in git shell scripts such as this one?
>
> Yes. Anything that sets GIT_EXEC_PATH correctly can use git-foo form.
I know they can. That was in the part you snipped ;)
The questions is: Should they? Should we avoid mixing both forms in one
script?
Michael
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Johannes Schindelin @ 2009-12-02 17:39 UTC (permalink / raw)
To: Ilari Liusvaara; +Cc: Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <20091202160446.GA32667@Knoppix>
Hi,
On Wed, 2 Dec 2009, Ilari Liusvaara wrote:
> And if mappings http -> curl, https -> curl, ftp -> curl are to remain
> explicit in main git binary, I would put them into table and build stub
> remote-curl if NO_CURL is defined instead of special casing the error in
> main git binary (but I consider that worse than just removing the
> association from main git binary).
This is definitely a good direction, and it would be even better if the
absence of the remote helper was also handled gracefully. Just think
about a (as of now fictious) git-remote-http.rpm relying on git-core.rpm
and libcurl.rpm. If you do not want to access http:// URLs, you can
install just git-core. Once you encounter an http:// URL you need to
access, you install git-remote-http. Keeping git-core. (I like to call
this setup "modular".)
Of course, I never understood why the backend should know the
implementation detail that it is based on cURL, so it would be even more
modular (at least by my definition) if there was no hard-coded mapping
(Sverre -- Cc'ed -- seemed to like URLs of the form "svn::http://..." and
"cvs::pserver..." to trigger looking for a remote helper explicitely). I
find the compiled-in mapping rather limiting.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] builtin-commit: add --date option
From: Junio C Hamano @ 2009-12-02 17:35 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20091202171117.GY31763@genesis.frugalware.org>
Miklos Vajna <vmiklos@frugalware.org> writes:
> On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> +--date=<date>::
>> + Override the date used in the commit. The format is the Git
>> + native one and is `<time> SP <offutc>`.
>
> I just noticed that fmt_ident() calls parse_date(), so other formats are
> supported as well.
>
> Is there any documentation describing what does parse_date() accept?
>
> Based on t0006-date.sh and the comments in the source, I see 3 supported
> formats:
>
> 1) <unix timestamp> <timezone>
>
> 2) A format like 2008-12-02 18:04:00
>
> 3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)
The above are all supported (you can label 2 as ISO even though the
official ISO8601 wants "T" instead of " " between date and time).
For more amusing ones, see
http://article.gmane.org/gmane.comp.version-control.git/12241
and follow the discussion there ;-)
^ permalink raw reply
* Re: [PATCH] git-pull.sh: Fix call to git-merge for new command format
From: Junio C Hamano @ 2009-12-02 17:49 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Horst H. von Brand, git
In-Reply-To: <4B16A410.5090802@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
>> Yes. Anything that sets GIT_EXEC_PATH correctly can use git-foo form.
>
> I know they can. That was in the part you snipped ;)
You asked about the presense of "a policy", and you got an answer.
> The questions is: Should they? Should we avoid mixing both forms in one
> script?
Should we avoid it? Yes but not very enthusiastically. We should make
sure that new invocations anybody adds use dashless form, but I would
recommend against a "let's remove use of dashed form" patch _unless_ you
find a time when the project is really quiet and there is nothing else
going on.
The whole point of GIT_EXEC_PATH trick is to allow continued use of the
dashed form, so that we do not have to suffer from code churn and patches
to implement real changes do not have to crash with such clean-ups.
As we'll be changing "git pull", we should use dashless form in the
vicinity of the real change (which is only one line) while at it, like
this.
-- >8 --
From: Horst H. von Brand <vonbrand@inf.utfsm.cl>
Date: Tue, 1 Dec 2009 19:44:11 -0300
Subject: [PATCH] git-pull.sh: Fix call to git-merge for new command format
Now "git merge <msg> HEAD" is officially deprecated, we should
clean our own use as well.
Signed-off-by: Horst H. von Brand <vonbrand@inf.utfsm.cl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
git-pull.sh | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index bfeb4a0..fcf6c81 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -216,7 +216,7 @@ fi
merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
test true = "$rebase" &&
- exec git-rebase $diffstat $strategy_args --onto $merge_head \
+ exec git rebase $diffstat $strategy_args --onto $merge_head \
${oldremoteref:-$merge_head}
-exec git-merge $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
- "$merge_name" HEAD $merge_head $verbosity
+exec git merge $verbosity $diffstat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
+ -m "$merge_name" $merge_head
^ permalink raw reply related
* Re: warning in git version 1.6.6.rc0.114.gc8648
From: Junio C Hamano @ 2009-12-02 17:50 UTC (permalink / raw)
To: Alejandro Riveira; +Cc: git
In-Reply-To: <hf67m0$r10$1@ger.gmane.org>
Alejandro Riveira <ariveira@gmail.com> writes:
> Whenever i do a "git pull" to check updates to the kernel
> or git repo i recieve
>
> warning: 'git merge <msg> HEAD <commit>' is deprecated. Please update
> your script to use 'git merge -m <msg> <commit>' instead.
> In future versions of git, this syntax will be removed.
> Already up-to-date.
>
> git pull should be updated to not use a deprecated form of git merge.
Yes we are aware of the issue and have a patch to do so which requires
another change which we also already have patch for. It will be fixed
before 1.6.6-rc1
Thanks for reporting.
^ permalink raw reply
* Re: [PATCH] builtin-commit: add --date option
From: Junio C Hamano @ 2009-12-02 17:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Miklos Vajna, git
In-Reply-To: <7vpr6xgtxn.fsf@alter.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Miklos Vajna <vmiklos@frugalware.org> writes:
> ...
>> Is there any documentation describing what does parse_date() accept?
>>
>> Based on t0006-date.sh and the comments in the source, I see 3 supported
>> formats:
>>
>> 1) <unix timestamp> <timezone>
>>
>> 2) A format like 2008-12-02 18:04:00
>>
>> 3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)
>
> The above are all supported (you can label 2 as ISO even though the
> official ISO8601 wants "T" instead of " " between date and time).
>
> For more amusing ones, see
>
> http://article.gmane.org/gmane.comp.version-control.git/12241
>
> and follow the discussion there ;-)
I should have mentioned ones with more importance in real-life before
referring you to the amusing ones: US and European dates.
date.c::match_multi_number() groks these:
mm/dd/yy[yy] (US)
dd.mm.yy[yy] (European)
^ permalink raw reply
* Re: equal-tree-merges as way to make rebases fast-forward-able
From: Junio C Hamano @ 2009-12-02 18:03 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Junio C Hamano, Bernhard R. Link, git
In-Reply-To: <20091202192026.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
>> In any case, at least this patch will make it start behaving a bit
>> more sanely.
>
> Thank you; it fixes the bug for me. Do I have to say
>
> Tested-by: Nanako Shiraishi <nanako3@lavabit.com>
>
> to ask you to include it in the new release?
>
>> -- >8 --
>> Subject: Do not misidentify "git merge foo HEAD" as an old-style invocation
Thanks for reminding me. I almost forgot that I did that patch.
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Sverre Rabbelier @ 2009-12-02 18:06 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Ilari Liusvaara, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de>
Heya,
On Wed, Dec 2, 2009 at 18:39, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> This is definitely a good direction, and it would be even better if the
> absence of the remote helper was also handled gracefully.
Yes, that is definitely an improvement we can and should make
regardless of how we handle http(s) and ftp(s), since currently "git
clone nonsense::http://...." will error out with the message that
"git-remote-nonsense" cannot be found.
> Of course, I never understood why the backend should know the
> implementation detail that it is based on cURL, so it would be even more
> modular (at least by my definition) if there was no hard-coded mapping.
Agreed.
> Sverre -- Cc'ed -- seemed to like URLs of the form "svn::http://..." and
> "cvs::pserver..." to trigger looking for a remote helper explicitely. I
> find the compiled-in mapping rather limiting.
Yes, I do think the double-colon syntax is very nice. That is, someone
who sees "git clone svn::http://" is likely to understand that it is a
svn repo over http that git treats specially.
However, I am not convinced that we should do any magic to map
"foo://" to git-remote-foo. On the other hand, I do think it makes
sense to have something modular that allows "git-remote-http" to be
implemented as a separate package that can be installed.
Perhaps instead of the current special case where "git-remote-curl" is
invoked, it would make more sense to instead special case on "http://"
(etc) and invoke "git-remote-http" in that case. So "git clone svn://"
would not work, but "git clone svn::svn://" would (as is the case
now), as well as "git clone http://" being handled by
"git-remote-http".
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Junio C Hamano @ 2009-12-02 18:07 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Ilari Liusvaara, Sverre Rabbelier, Junio C Hamano, git
In-Reply-To: <alpine.DEB.1.00.0912021832480.4985@pacific.mpi-cbg.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> This is definitely a good direction, and it would be even better if the
> absence of the remote helper was also handled gracefully. Just think
> about a (as of now fictious) git-remote-http.rpm relying on git-core.rpm
> and libcurl.rpm. If you do not want to access http:// URLs, you can
> install just git-core. Once you encounter an http:// URL you need to
> access, you install git-remote-http. Keeping git-core. (I like to call
> this setup "modular".)
The "modular" setup is a good thing to do, but I do not know how it
relates to the change Ilari did. Isn't it simply a matter of excluding
git-remote-curl from the current set of binaries to be shipped with
git-core.rpm and making a separate git-remote-http.rpm to contain it, or
does it involve a lot more than that from the packager's side?
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Junio C Hamano @ 2009-12-02 18:41 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Johannes Schindelin, Ilari Liusvaara, Junio C Hamano, git
In-Reply-To: <fabb9a1e0912021006x2905578bo16dbcaedc0d97bc6@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
>> Of course, I never understood why the backend should know the
>> implementation detail that it is based on cURL, so it would be even more
>> modular (at least by my definition) if there was no hard-coded mapping.
>
> Agreed.
I don't get this point at all.
Backend is _very_ aware of how it is implemented itself. Naming one
implementation git-remote-http is to declare that "I am the one and only
implementation of http handler" and forces another implementation of http
handler, perhaps based on different toolkit than libcurl, to forever be a
second class citizen that need to use name other than 'http'.
The "mapping" you two are calling "hard-coded" may be "hard-coded" but is
a better kind of hard-coding than hard-coding "http" to "this particular
implementation" implicitly like you two seem to be advocating. Think of
it as having one extra layer of indirection.
When the second implementation of http handler proves to be better than
the current one, we can flip the mapping, and anybody who were using
"http://" to access some repository will automatically updated to use the
new backend instead of the old one. With your scheme, you probably could
change the name of the old "http" backend to "http-deprecated" and the new
one from "second-class-citizen-http" to "http" to achieve a similar
effect, but I do not think it is as nice as having one extra level of
indirection.
> However, I am not convinced that we should do any magic to map
> "foo://" to git-remote-foo. On the other hand, I do think it makes
> sense to have something modular that allows "git-remote-http" to be
> implemented as a separate package that can be installed.
As I said, I do think modular is good, but I think what Dscho is
advocating does not have much to achieve that goal.
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Ilari Liusvaara @ 2009-12-02 18:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Sverre Rabbelier, git
In-Reply-To: <7vskbtfdvo.fsf@alter.siamese.dyndns.org>
On Wed, Dec 02, 2009 at 10:07:55AM -0800, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> The "modular" setup is a good thing to do, but I do not know how it
> relates to the change Ilari did.
Removing core dependency on NO_CURL is required for that. 6/8 is one
way to do that given support for dispatching <scheme>:// to remote
helpers (1/8).
> Isn't it simply a matter of excluding
> git-remote-curl from the current set of binaries to be shipped with
> git-core.rpm and making a separate git-remote-http.rpm to contain it, or
> does it involve a lot more than that from the packager's side?
See above. There's also issues with git remote helper execution, namely
inability to properly handle failure before cap exchange.
With properly fixed main git binary, it is just matter of splitting
remote-xxx executable(s) relating to HTTP to seperate package.
-Ilari
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Sverre Rabbelier @ 2009-12-02 18:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Ilari Liusvaara, git
In-Reply-To: <7vk4x5fcbf.fsf@alter.siamese.dyndns.org>
Heya,
On Wed, Dec 2, 2009 at 19:41, Junio C Hamano <gitster@pobox.com> wrote:
> When the second implementation of http handler proves to be better than
> the current one, we can flip the mapping, and anybody who were using
> "http://" to access some repository will automatically updated to use the
> new backend instead of the old one. With your scheme, you probably could
> change the name of the old "http" backend to "http-deprecated" and the new
> one from "second-class-citizen-http" to "http" to achieve a similar
> effect, but I do not think it is as nice as having one extra level of
> indirection.
I don't see how what you are talking about is any different. With the
mapping the executable of the alternative implementation still needs
to have a different name, no?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [RFC PATCH 0/8] Git remote helpers to implement smart transports.
From: Junio C Hamano @ 2009-12-02 18:52 UTC (permalink / raw)
To: Sverre Rabbelier
Cc: Junio C Hamano, Johannes Schindelin, Ilari Liusvaara, git
In-Reply-To: <fabb9a1e0912021050r4fd96ed4rf863917a965a42fb@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> writes:
> I don't see how what you are talking about is any different. With the
> mapping the executable of the alternative implementation still needs
> to have a different name, no?
Sure, but please search for "second class citizen" in my message.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox