git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/3] fetch: optionally store the current remote information in the config
@ 2006-04-30 13:24 Johannes Schindelin
  2006-04-30 14:19 ` Jakub Narebski
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Johannes Schindelin @ 2006-04-30 13:24 UTC (permalink / raw)
  To: git


Instead of editing files, you can now say

	git pull --store junio \
		git://git.kernel.org/pub/scm/git/git.git next:next

and next time, just

	git pull junio

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	This is what the patch series is all about.

	If there is no interest in a feature like this, let's just forget
	about the whole "remote info in config" thing.

	If there is interest, I could add the same functionality to
	builtin-push.

 Documentation/fetch-options.txt |    6 ++++++
 git-fetch.sh                    |   19 +++++++++++++++++++
 git-pull.sh                     |    8 ++++++--
 3 files changed, 31 insertions(+), 2 deletions(-)

6bd937b0de211465e9664f8dc890fc5066617b73
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 13f34d3..caf98de 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -16,6 +16,12 @@
 	fetches is a descendant of `<lbranch>`.  This option
 	overrides that check.
 
+-S, \--store <nick>::
+	Store the URL and the refnames in the config file so that
+	`git fetch <nick>` repeats the exercise.
+	If the nick exists already, edit the URL, but append the
+	refnames.
+
 \--no-tags::
 	By default, `git-fetch` fetches tags that point at
 	objects that are downloaded from the remote repository
diff --git a/git-fetch.sh b/git-fetch.sh
index 280f62e..ac122da 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -15,8 +15,10 @@ no_tags=
 tags=
 append=
 force=
+keep=
 verbose=
 update_head_ok=
+store=
 exec=
 upload_pack=
 while case "$#" in 0) break ;; esac
@@ -34,6 +36,10 @@ do
 	-f|--f|--fo|--for|--forc|--force)
 		force=t
 		;;
+	-S|--s|--st|--sto|--stor|--store)
+		store="$2"
+		shift
+		;;
 	-t|--t|--ta|--tag|--tags)
 		tags=t
 		;;
@@ -235,6 +241,12 @@ then
 	fi
 fi
 
+if test "$store"
+then
+    git-repo-config remote."$store".url $remote ||
+	die "Could not store into $store"
+fi
+
 fetch_main () {
   reflist="$1"
   refs=
@@ -243,6 +255,11 @@ fetch_main () {
   do
       refs="$refs$LF$ref"
 
+      if test "$store"
+      then
+	  git-repo-config remote."$store".pull "$ref" '^$'
+      fi
+
       # These are relative path from $GIT_DIR, typically starting at refs/
       # but may be HEAD
       if expr "z$ref" : 'z\.' >/dev/null
@@ -381,6 +398,8 @@ fetch_main () {
 
 fetch_main "$reflist"
 
+store=
+
 # automated tag following
 case "$no_tags$tags" in
 '')
diff --git a/git-pull.sh b/git-pull.sh
index 4611ae6..ab0fba3 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -8,7 +8,7 @@ USAGE='[-n | --no-summary] [--no-commit]
 LONG_USAGE='Fetch one or more remote refs and merge it/them into the current HEAD.'
 . git-sh-setup
 
-strategy_args= no_summary= no_commit=
+strategy_args= no_summary= no_commit= store=
 while case "$#,$1" in 0) break ;; *,-*) ;; *) break ;; esac
 do
 	case "$1" in
@@ -31,6 +31,10 @@ do
 		esac
 		strategy_args="${strategy_args}-s $strategy "
 		;;
+	-S|--store)
+		store="-S $2"
+		shift
+		;;
 	-h|--h|--he|--hel|--help)
 		usage
 		;;
@@ -43,7 +47,7 @@ do
 done
 
 orig_head=$(git-rev-parse --verify HEAD) || die "Pulling into a black hole?"
-git-fetch --update-head-ok "$@" || exit 1
+git-fetch --update-head-ok $store "$@" || exit 1
 
 curr_head=$(git-rev-parse --verify HEAD)
 if test "$curr_head" != "$orig_head"
-- 
1.3.1.g38c00-dirty

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 13:24 [PATCH 3/3] fetch: optionally store the current remote information in the config Johannes Schindelin
@ 2006-04-30 14:19 ` Jakub Narebski
  2006-04-30 15:52   ` Johannes Schindelin
       [not found] ` <20060430103046.35c1385f.seanlkml@sympatico.ca>
  2006-05-02  8:59 ` Junio C Hamano
  2 siblings, 1 reply; 16+ messages in thread
From: Jakub Narebski @ 2006-04-30 14:19 UTC (permalink / raw)
  To: git

There was thread about storing somewhere default branch we merge to during
pull, instead of using always surrent one. Different schemes were proposed,
most of them depending on the remotes configuration being available [also]
in config file.

Perhaps it would be easiest to extend existing notation in the following
way: 
  <from>:<to>[:<merge>]

By the way: it would be nice to have command/script to trasform freely
between 'remotes/' and config file.


P.S. I wonder if it would be difficult to implement 'include <file>' for
config file...

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
       [not found] ` <20060430103046.35c1385f.seanlkml@sympatico.ca>
@ 2006-04-30 14:30   ` sean
  2006-04-30 15:49     ` Johannes Schindelin
  0 siblings, 1 reply; 16+ messages in thread
From: sean @ 2006-04-30 14:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

On Sun, 30 Apr 2006 15:24:22 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> Instead of editing files, you can now say
> 
> 	git pull --store junio \
> 		git://git.kernel.org/pub/scm/git/git.git next:next
> 
> and next time, just
> 
> 	git pull junio
> 
> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> 
> ---
> 
> 	This is what the patch series is all about.
> 
> 	If there is no interest in a feature like this, let's just forget
> 	about the whole "remote info in config" thing.
> 
> 	If there is interest, I could add the same functionality to
> 	builtin-push.

Well I agree with you that doing something like this is important.  We
should take this moment of moving things to the config file to correct
the terminology and help make things clear.  We're not storing "Pull:"
information, we're storing config/remote.$NICK.fetch data.  It's really
used just by fetch, pull just happens to call fetch.

Along that same line of reasoning, it seems more appropriate to use 
git fetch --store ...  rather than git pull --store ... to set this
information.   And there needs to be a way to change and delete the 
nick information, perhaps git fetch store junio ""  would delete the
entry. Or maybe people should just be instructed to use git-repo-config
for setting, changing and deleting?

Pull needs additional logic that allows it to merge from the proper
local branch after it calls fetch.  Right now it just uses whatever 
fetch sets as FETCH_HEAD.  It's not clear to me what is set as 
FETCH_HEAD when multiple refs are fetched from the remote.  It'll 
be even more confusing once it's possible to fetch from multiple 
remotes at once.

As for these specific patches, it doesn't appear that your change to
builtin-push allows the push variable to hold more than one remote 
repo URI or even more than one refspec, or did I misread that?
Also it seems that the refspec is used from the config file even if
the user tries to override it by specifying an alternative on the
command line.

Sean

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 14:30   ` sean
@ 2006-04-30 15:49     ` Johannes Schindelin
       [not found]       ` <20060430123709.11fcdd5f.seanlkml@sympatico.ca>
  2006-04-30 22:21       ` Junio C Hamano
  0 siblings, 2 replies; 16+ messages in thread
From: Johannes Schindelin @ 2006-04-30 15:49 UTC (permalink / raw)
  To: sean; +Cc: git

Hi,

On Sun, 30 Apr 2006, sean wrote:

> On Sun, 30 Apr 2006 15:24:22 +0200 (CEST)
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> > Instead of editing files, you can now say
> > 
> > 	git pull --store junio \
> > 		git://git.kernel.org/pub/scm/git/git.git next:next
> > 
> > and next time, just
> > 
> > 	git pull junio
> > 
> > Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> > 
> > ---
> > 
> > 	This is what the patch series is all about.
> > 
> > 	If there is no interest in a feature like this, let's just forget
> > 	about the whole "remote info in config" thing.
> > 
> > 	If there is interest, I could add the same functionality to
> > 	builtin-push.
> 
> Well I agree with you that doing something like this is important.  We
> should take this moment of moving things to the config file to correct
> the terminology and help make things clear.  We're not storing "Pull:"
> information, we're storing config/remote.$NICK.fetch data.  It's really
> used just by fetch, pull just happens to call fetch.

I have no strong feelings either way.

> Along that same line of reasoning, it seems more appropriate to use 
> git fetch --store ...  rather than git pull --store ... to set this
> information.

Both works.

> And there needs to be a way to change and delete the nick information, 
> perhaps git fetch store junio ""  would delete the entry. Or maybe 
> people should just be instructed to use git-repo-config for setting, 
> changing and deleting?

The latter should be done, because "git fetch" really is about fetching, 
not playing games with the config.

> Pull needs additional logic that allows it to merge from the proper
> local branch after it calls fetch.  Right now it just uses whatever 
> fetch sets as FETCH_HEAD.  It's not clear to me what is set as 
> FETCH_HEAD when multiple refs are fetched from the remote.  It'll 
> be even more confusing once it's possible to fetch from multiple 
> remotes at once.

FETCH_HEAD can contain multiple refs. And I don't get the part about 
fetching from multiple remotes: my patch does not allow for that.

> As for these specific patches, it doesn't appear that your change to
> builtin-push allows the push variable to hold more than one remote 
> repo URI or even more than one refspec, or did I misread that?

But it does! Note the "uri_[current_uri++]" part of the patch.

> Also it seems that the refspec is used from the config file even if
> the user tries to override it by specifying an alternative on the
> command line.

No. It is only used when there were no refspecs specified on the command 
line:

        if (refspec_nr == 0)
                set_refspecs((const char**)refspecs_, current_refspec);


Ciao,
Dscho

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 14:19 ` Jakub Narebski
@ 2006-04-30 15:52   ` Johannes Schindelin
  2006-04-30 16:07     ` Jakub Narebski
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Schindelin @ 2006-04-30 15:52 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

Hi,

On Sun, 30 Apr 2006, Jakub Narebski wrote:

> There was thread about storing somewhere default branch we merge to during
> pull, instead of using always surrent one. Different schemes were proposed,
> most of them depending on the remotes configuration being available [also]
> in config file.

I was not following that thread closely, since it became too confusing for 
me. However, I think that my patch could be a start in that direction.

> By the way: it would be nice to have command/script to trasform freely
> between 'remotes/' and config file.

If you set the environment variable GIT_REWRITE_REMOTES to "true", and 
call git-parse-remotes.sh, it will do the rewriting to the config file. 
Obviously, I did not test that part of the patch all that well.

> P.S. I wonder if it would be difficult to implement 'include <file>' for
> config file...

You really need that?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 15:52   ` Johannes Schindelin
@ 2006-04-30 16:07     ` Jakub Narebski
  0 siblings, 0 replies; 16+ messages in thread
From: Jakub Narebski @ 2006-04-30 16:07 UTC (permalink / raw)
  To: git

Johannes Schindelin wrote:

> On Sun, 30 Apr 2006, Jakub Narebski wrote:
> 
>> P.S. I wonder if it would be difficult to implement 'include <file>' for
>> config file...
> 
> You really need that?

Need? Not exactly. I don't think git ever reach complexity of Apache or
Samba configuration files, and _need_ for includes. Still dividing separate
areas of configuration (core, user, default commands options, remotes) has
it's merits.

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
       [not found]       ` <20060430123709.11fcdd5f.seanlkml@sympatico.ca>
@ 2006-04-30 16:37         ` sean
  2006-04-30 16:51           ` Jakub Narebski
  2006-04-30 17:09           ` Johannes Schindelin
  0 siblings, 2 replies; 16+ messages in thread
From: sean @ 2006-04-30 16:37 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

On Sun, 30 Apr 2006 17:49:06 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:


> > the terminology and help make things clear.  We're not storing "Pull:"
> > information, we're storing config/remote.$NICK.fetch data.  It's really
> > used just by fetch, pull just happens to call fetch.
>
> I have no strong feelings either way.

Yeah, once you "get" it, it's not a problem; but it's not easy when you're
just learning git to separate fetch and pull.  It's made harder if git 
can't even keep them straight internally. :o/

[...]

> The latter should be done, because "git fetch" really is about fetching, 
> not playing games with the config.

Then we should also remove the --store option from pull and fetch.  It
can be set with git-repo-config.

> FETCH_HEAD can contain multiple refs. 

Which head does git-pull then use to merge, all of them?

> And I don't get the part about fetching from multiple remotes: 
> my patch does not allow for that.

Actually it does :o)  User just needs multiple remote.$nick.url entries 
in his config.

> But it does! Note the "uri_[current_uri++]" part of the patch.
[...]
> No. It is only used when there were no refspecs specified on the command 
> line:
> 
>         if (refspec_nr == 0)
>                 set_refspecs((const char**)refspecs_, current_refspec);

Right you are, on both counts.

Sean

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 16:37         ` sean
@ 2006-04-30 16:51           ` Jakub Narebski
       [not found]             ` <20060430131936.43598f6f.seanlkml@sympatico.ca>
  2006-04-30 17:09           ` Johannes Schindelin
  1 sibling, 1 reply; 16+ messages in thread
From: Jakub Narebski @ 2006-04-30 16:51 UTC (permalink / raw)
  To: git

sean wrote:

> On Sun, 30 Apr 2006 17:49:06 +0200 (CEST)
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> 
>> > the terminology and help make things clear.  We're not storing "Pull:"
>> > information, we're storing config/remote.$NICK.fetch data.  It's really
>> > used just by fetch, pull just happens to call fetch.
>>
>> I have no strong feelings either way.
> 
> Yeah, once you "get" it, it's not a problem; but it's not easy when you're
> just learning git to separate fetch and pull.  It's made harder if git
> can't even keep them straight internally. :o/

Well, it could also contain default head we merge to (instead of using what
fetch set as FETCH_HEAD, usually current head while fetching), as

        pull = master:origin:merger

> [...]
> 
> > The latter should be done, because "git fetch" really is about fetching, 
> > not playing games with the config.
> 
> Then we should also remove the --store option from pull and fetch.  It
> can be set with git-repo-config.

The --store option is similar to using 'git checkout -b newbranch' as a
shortcut for 'git branch newbranch' followed by 'git checkout newbranch'.

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 16:37         ` sean
  2006-04-30 16:51           ` Jakub Narebski
@ 2006-04-30 17:09           ` Johannes Schindelin
       [not found]             ` <20060430132819.3af8e9d1.seanlkml@sympatico.ca>
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Schindelin @ 2006-04-30 17:09 UTC (permalink / raw)
  To: sean; +Cc: git

Hi,

On Sun, 30 Apr 2006, sean wrote:

> On Sun, 30 Apr 2006 17:49:06 +0200 (CEST)
> Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> 
> > The latter should be done, because "git fetch" really is about fetching, 
> > not playing games with the config.
> 
> Then we should also remove the --store option from pull and fetch.  It
> can be set with git-repo-config.

Well, with "--store", "git fetch" still fetches. It just happens to write 
down -- for convenience -- the possibly long url and the refspecs.

> > FETCH_HEAD can contain multiple refs. 
> 
> Which head does git-pull then use to merge, all of them?

The first one.

> > And I don't get the part about fetching from multiple remotes: 
> > my patch does not allow for that.
> 
> Actually it does :o)  User just needs multiple remote.$nick.url entries 
> in his config.

You are right. But you are also wrong. The patch uses

	git-repo-config --get remote.$nick.url

which fails if there are more than one matching line. Note that 
"--get-all" is used to get _all_ remote.$nick.pull lines...

But of course, Linus "built git-push in" so that multiple urls are 
allowed and handled. It is probably confusing, if you can push but 
cannot fetch with the same remote information... But then, I fail to see 
how you could possibly specify the refspecs for the different urls.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
       [not found]             ` <20060430131936.43598f6f.seanlkml@sympatico.ca>
@ 2006-04-30 17:19               ` sean
  2006-04-30 17:35                 ` Jakub Narebski
  0 siblings, 1 reply; 16+ messages in thread
From: sean @ 2006-04-30 17:19 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: git

On Sun, 30 Apr 2006 18:51:54 +0200
Jakub Narebski <jnareb@gmail.com> wrote:
 
> Well, it could also contain default head we merge to (instead of using what
> fetch set as FETCH_HEAD, usually current head while fetching), as
> 
>         pull = master:origin:merger

Then lets take a simple case; we clone a new repo, and it has:

	[remote.origin]
	  url = git://outthere.com
	  fetch = master:origin:master
	  fetch = next:next

And we create two new branches:

	git branch br1 next ; git branch br2 next

Now say that we want a bare "git pull" to cause a merge from 
the "next" branch regardless of which new branch we have checked
out.   In the above scheme we have to do something like:

	   fetch = next:next:br1:br2

That doesn't look right.  It seems better to have:
	
	[branch.origin]
		description = "Pristine master from Junio"
	[branch.br1]
		description = "blah"
		defaultMerge = "next"
	[branch.br2]
		description = "More blah"
		LastMerge = 03/27/2008 3am
		defaultMerge = "next"
		qgitTagColor = Blue

> The --store option is similar to using 'git checkout -b newbranch' as a
> shortcut for 'git branch newbranch' followed by 'git checkout newbranch'.

Okay.

Sean

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
       [not found]             ` <20060430132819.3af8e9d1.seanlkml@sympatico.ca>
@ 2006-04-30 17:28               ` sean
  0 siblings, 0 replies; 16+ messages in thread
From: sean @ 2006-04-30 17:28 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

On Sun, 30 Apr 2006 19:09:18 +0200 (CEST)
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

> > Which head does git-pull then use to merge, all of them?
> 
> The first one.

Which can be confusing since you can only specify one "first one"
in a remotes file (or .git/config)  yet you can pull while 
having any random local branch being checkout out, each with
a different merge branch being appropriate.

> You are right. But you are also wrong. The patch uses
> 
> 	git-repo-config --get remote.$nick.url
> 
> which fails if there are more than one matching line. Note that 
> "--get-all" is used to get _all_ remote.$nick.pull lines...
> 
> But of course, Linus "built git-push in" so that multiple urls are 
> allowed and handled. It is probably confusing, if you can push but 
> cannot fetch with the same remote information... But then, I fail to see 
> how you could possibly specify the refspecs for the different urls.

Yeah, I was speaking only of git push, but see you were speaking of 
fetch.

Anyway, thanks for helping me understand your proposal, it seems
flexible enough to handle just about any case one might want to
throw at it.

Cheers,
Sean

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 17:19               ` sean
@ 2006-04-30 17:35                 ` Jakub Narebski
  0 siblings, 0 replies; 16+ messages in thread
From: Jakub Narebski @ 2006-04-30 17:35 UTC (permalink / raw)
  To: git

sean wrote:

> [branch.origin]
> description = "Pristine master from Junio"

I'd like to add
> pristine = true

and for git to warn if we try to commit to branch which is supposed to be
pristine (tracking external repository).

-- 
Jakub Narebski
Warsaw, Poland

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 15:49     ` Johannes Schindelin
       [not found]       ` <20060430123709.11fcdd5f.seanlkml@sympatico.ca>
@ 2006-04-30 22:21       ` Junio C Hamano
  1 sibling, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2006-04-30 22:21 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sun, 30 Apr 2006, sean wrote:
>
>> Well I agree with you that doing something like this is important.  We
>> should take this moment of moving things to the config file to correct
>> the terminology and help make things clear.  We're not storing "Pull:"
>> information, we're storing config/remote.$NICK.fetch data.  It's really
>> used just by fetch, pull just happens to call fetch.
>
> I have no strong feelings either way.

I have a strong feeling that naming them "Pull: " was a mistake
to begin with and they should have been called "Fetch: " ;-).

>> Along that same line of reasoning, it seems more appropriate to use 
>> git fetch --store ...  rather than git pull --store ... to set this
>> information.
>
> Both works.

I'd rather not see --store added to either fetch nor pull.

While I would agree --store would appear to be a convenient
short-hand for first timers, I strongly suspect this will cause
more confusion and complexity down the line.  If a topic that
interests you appears today at the remote, and you start
following it by adding --store when you fetch from it, and later
when the topic disappears at the remote in two weeks because it
is fully cooked and merged into somewhere else, you would need
to have a way to --unstore it somehow, and at that time the
first timer needs to learn repo-config to deal with that failure
anyway.  Or you need to teach git-fetch that fetch failure of a
branch is actually OK as long as all of the following holds
true:

 - the branch was added with --store in the past; the user is
   not actively asking for it in the current request, saying "I
   want to fetch from there THIS TIME".

 - the fetch failed only because the remote repository droped
   the branch (IOW you need to tell connection and protocol
   failure from "branch disappeared"case),

 - the fetch is not being done to merge it into the current branch,

or something complicated like that, and unstore it automatically
for the user.

Other than that, I do not have strong feelings against using the
standard .git/config to store what we store in .git/remotes/*
currently, and I also suspect doing so would be a prerequiste
first step to do "per local branch configuration" (e.g. "when on
this branch, merge from this branch by default") some people
seem to want.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-04-30 13:24 [PATCH 3/3] fetch: optionally store the current remote information in the config Johannes Schindelin
  2006-04-30 14:19 ` Jakub Narebski
       [not found] ` <20060430103046.35c1385f.seanlkml@sympatico.ca>
@ 2006-05-02  8:59 ` Junio C Hamano
  2006-05-02 12:42   ` Johannes Schindelin
  2 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2006-05-02  8:59 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> 	This is what the patch series is all about.
>
> 	If there is no interest in a feature like this, let's just forget
> 	about the whole "remote info in config" thing.

Well, I took the liberty of adjusting the first one in the
series and tonight's "pu" has that one and the second one.
I haven't touched the third one yet, though.

About the second one, I think it probably is a good idea to
rename the "refspec used for fetch" as Sean suggested earlier.
I do not like that hidden environment variable that sits in the
command I use everyday, waiting to be triggered to update my
.config file, possibly by my PEBCAK mistake when I did not want
it to do so.

I am not quite sure what this bit is about in the second one:

        sed -n \
        -e "s/^URL: /remote.$name.url . /p" \
        -e "s/^Pull: /remote.$name.pull ^$ /p" \
        -e "s/^Push: /remote.$name.push ^$ /p" \
	< "$f"

I am getting this out of the above:

        remote.ko.url . xxx.kernel.org:/pub/scm/git/git.git/
        remote.ko.pull ^$ master:refs/tags/ko-master
        remote.ko.pull ^$ next:refs/tags/ko-next
        remote.ko.pull ^$ +pu:refs/tags/ko-pu
        remote.ko.pull ^$ maint:refs/tags/ko-maint
        remote.ko.push ^$ heads/master
        remote.ko.push ^$ heads/next
        remote.ko.push ^$ +heads/pu
        remote.ko.push ^$ heads/maint

but I suspect that is not what you intended...

I think easy conversion tool is a good idea, but I would sleep
better if it is outside of git-fetch/push chain and is available
elsewhere, perhaps in contrib/ area.

On a slightly related topic, I think my aversion to your "push
remotes into config" series the last time was primarily because
I do not trust repo-config.  Reading an already built config
seems to work OK and I do not worry too much, but I am still
wary of letting it write.  Typing "git repo-config" in a freshly
initialized empty repository seems to segfault, which does not
help my confidence level either.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-05-02  8:59 ` Junio C Hamano
@ 2006-05-02 12:42   ` Johannes Schindelin
  2006-05-03  0:36     ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Johannes Schindelin @ 2006-05-02 12:42 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi,

On Tue, 2 May 2006, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > 	This is what the patch series is all about.
> >
> > 	If there is no interest in a feature like this, let's just forget
> > 	about the whole "remote info in config" thing.
> 
> Well, I took the liberty of adjusting the first one in the
> series and tonight's "pu" has that one and the second one.
> I haven't touched the third one yet, though.

I don't think it is worth introducing yet another way to specify 
short-cuts for remote information, if there is not at least one problem 
which can get solved easier with it than with the other two ways.

> About the second one, I think it probably is a good idea to
> rename the "refspec used for fetch" as Sean suggested earlier.

Okay.

> I do not like that hidden environment variable that sits in the
> command I use everyday, waiting to be triggered to update my
> .config file, possibly by my PEBCAK mistake when I did not want
> it to do so.

I will refactor it.

> I am not quite sure what this bit is about in the second one:
> 
>         sed -n \
>         -e "s/^URL: /remote.$name.url . /p" \
>         -e "s/^Pull: /remote.$name.pull ^$ /p" \
>         -e "s/^Push: /remote.$name.push ^$ /p" \
> 	< "$f"

That is obviously wrong. Will fix while refactoring.

> I think easy conversion tool is a good idea, but I would sleep
> better if it is outside of git-fetch/push chain and is available
> elsewhere, perhaps in contrib/ area.

Will do.

> On a slightly related topic, I think my aversion to your "push
> remotes into config" series the last time was primarily because
> I do not trust repo-config.  Reading an already built config
> seems to work OK and I do not worry too much, but I am still
> wary of letting it write.  Typing "git repo-config" in a freshly
> initialized empty repository seems to segfault, which does not
> help my confidence level either.

I fixed this error (see separate patch). This was reintroduced by 
carelessly checking argv[1] for "--list" and "-l", even if argc < 2. I am 
sorry that I did not review that patch.

I tried to make really sure that repo-config works as expected by 
introducing quite a few test cases into t1300, but evidently I forgot to 
check for things that do not usually break, like calling without 
arguments. This is fixed with the patch I just sent out.

This patch also introduces the "--get-regexp" flag to repo-config, which 
makes up for the lacking shell wildcards (you can ask questions like: 
which keys in the config end in "coatl"?).

As for the trust in repo-config writing the config: it is all done by 
calling git_config_set() or git_config_set_multivar(), which you use 
yourself to set core.repositoryformatversion, among other values.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/3] fetch: optionally store the current remote information in the config
  2006-05-02 12:42   ` Johannes Schindelin
@ 2006-05-03  0:36     ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2006-05-03  0:36 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> Well, I took the liberty of adjusting the first one in the
>> series and tonight's "pu" has that one and the second one.
>> I haven't touched the third one yet, though.
>
> I don't think it is worth introducing yet another way to specify 
> short-cuts for remote information, if there is not at least one problem 
> which can get solved easier with it than with the other two ways.

I think the biggest contribution this series might bring in is
to send a message that we would want to have things in config,
not outside -- otherwise people might be tempted to do "while on
this branch use this remote to fetch/pull from by default"
outside config (perhaps abusing .git/branches, which _is_ per
branch configuration).

>> I do not like that hidden environment variable that sits in the
>> command I use everyday, waiting to be triggered to update my
>> .config file, possibly by my PEBCAK mistake when I did not want
>> it to do so.
>
> I will refactor it.

Thanks.

> I fixed this error (see separate patch). This was reintroduced by 
> carelessly checking argv[1] for "--list" and "-l", even if argc < 2.

Thanks.

> As for the trust in repo-config writing the config: it is all done by 
> calling git_config_set() or git_config_set_multivar(), which you use 
> yourself to set core.repositoryformatversion, among other values.

Since that happens in a freshly created empty config, I do not
have to have a high confidence for that case, compared to the
case it has to muck with random configuration files people
already have populated with arbitrary gunk.  In any case, I
haven't seen breakage myself recently so hopefully it is safe
enough now ;-).

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2006-05-03  0:36 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-30 13:24 [PATCH 3/3] fetch: optionally store the current remote information in the config Johannes Schindelin
2006-04-30 14:19 ` Jakub Narebski
2006-04-30 15:52   ` Johannes Schindelin
2006-04-30 16:07     ` Jakub Narebski
     [not found] ` <20060430103046.35c1385f.seanlkml@sympatico.ca>
2006-04-30 14:30   ` sean
2006-04-30 15:49     ` Johannes Schindelin
     [not found]       ` <20060430123709.11fcdd5f.seanlkml@sympatico.ca>
2006-04-30 16:37         ` sean
2006-04-30 16:51           ` Jakub Narebski
     [not found]             ` <20060430131936.43598f6f.seanlkml@sympatico.ca>
2006-04-30 17:19               ` sean
2006-04-30 17:35                 ` Jakub Narebski
2006-04-30 17:09           ` Johannes Schindelin
     [not found]             ` <20060430132819.3af8e9d1.seanlkml@sympatico.ca>
2006-04-30 17:28               ` sean
2006-04-30 22:21       ` Junio C Hamano
2006-05-02  8:59 ` Junio C Hamano
2006-05-02 12:42   ` Johannes Schindelin
2006-05-03  0:36     ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).