threads / patch / 4011

patch, 3 partsfetch: optionally store the current remote information in the config

Subject: [PATCH 3/3] fetch: optionally store the current remote information in the config

## tl;dr

16 messages between Apr 30, 2006 and May 3, 2006. Diffs are folded; open one to read it.

replies: 15people: 4as markdown or json

Johannes Schindelin· Apr 30, 2006, 13:24 UTC · lore
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
Show changes to 3 files +31 −2

Documentation/fetch-options.txt, git-fetch.sh, git-pull.sh

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
Jakub Narebski· Apr 30, 2006, 14:19 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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
Johannes Schindelin· Apr 30, 2006, 15:52 UTC · re: Jakub Narebski · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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

Jakub Narebski· Apr 30, 2006, 16:07 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Johannes Schindelin wrote:
Show 6 quoted lines
> 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
sean· Apr 30, 2006, 14:30 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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

Show 20 quoted lines
> 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
Johannes Schindelin· Apr 30, 2006, 15:49 UTC · re: sean · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Hi,
On Sun, 30 Apr 2006, sean wrote:
Show 29 quoted lines
> 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.

Show 6 quoted lines
> 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

sean· Apr 30, 2006, 16:37 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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

Show 5 quoted lines
> > 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.
[...]
Show 5 quoted lines
> 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
Jakub Narebski· Apr 30, 2006, 16:51 UTC · re: sean · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

sean wrote:
Show 13 quoted lines
> 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
Show 7 quoted lines
> [...]
> 
> > 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
sean· Apr 30, 2006, 17:19 UTC · re: Jakub Narebski · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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
Jakub Narebski· Apr 30, 2006, 17:35 UTC · re: sean · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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
Johannes Schindelin· Apr 30, 2006, 17:09 UTC · re: sean · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Hi,
On Sun, 30 Apr 2006, sean wrote:
Show 9 quoted lines
> 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.
Show 5 quoted lines
> > 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

sean· Apr 30, 2006, 17:28 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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.

Show 11 quoted lines
> 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

Junio C Hamano· Apr 30, 2006, 22:21 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 9 quoted lines
> 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: " ;-).

Show 5 quoted lines
>> 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.

Junio C Hamano· May 2, 2006, 08:59 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

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.

Johannes Schindelin· May 2, 2006, 12:42 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Hi,
On Tue, 2 May 2006, Junio C Hamano wrote:
Show 10 quoted lines
> 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.
Show 7 quoted lines
> 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.
Show 7 quoted lines
> 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

Junio C Hamano· May 3, 2006, 00:36 UTC · re: Johannes Schindelin · lore

Re: [PATCH 3/3] fetch: optionally store the current remote information in the config

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 7 quoted lines
>> 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).

Show 6 quoted lines
>> 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 ;-).

← back to recent threads