Git development
 help / color / mirror / Atom feed
* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-02-04  2:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <7vk4utybur.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> And this is a fix-up for the mismerge.  I didn't touch max-pack-size stuff.
> 
>  fast-import.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/fast-import.c b/fast-import.c
> index ca21082..a6730d0 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2800,7 +2800,10 @@ static int parse_one_option(const char *option)
>  	if (!prefixcmp(option, "max-pack-size=")) {
>  		option_max_pack_size(option + 14);
>  	} else if (!prefixcmp(option, "big-file-threshold=")) {
> -		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
> +		unsigned long v;
> +		if (!git_parse_ulong(option + 19, &v))
> +			return 0;
> +		big_file_threshold = v;

Yup, looks good to me.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Nicolas Pitre @ 2010-02-04  2:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Sverre Rabbelier, git
In-Reply-To: <7v7hqtzrmq.fsf@alter.siamese.dyndns.org>

On Wed, 3 Feb 2010, Junio C Hamano wrote:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> > The strtoumax call got messed up.  Squash this into your merge:
> >
> > diff --git a/fast-import.c b/fast-import.c
> > index e6ebcf6..9c65a24 100644
> > --- a/fast-import.c
> > +++ b/fast-import.c
> > @@ -2800,7 +2800,7 @@ static int parse_one_option(const char *option)
> >  	if (!prefixcmp(option, "max-pack-size=")) {
> >  		option_max_pack_size(option + 14);
> >  	} else if (!prefixcmp(option, "big-file-threshold=")) {
> > -		big_file_threshold = strtoumax(option + 21, NULL, 0) * 1024 * 1024;
> > +		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
> >  	} else if (!prefixcmp(option, "depth=")) {
> >  		option_depth(option + 6);
> >  	} else if (!prefixcmp(option, "active-branches=")) {
> >  
> >> You may want to add the new option to the output from "cmd -h" and
> >> probably description of the configuration in the doc before any of this
> >> gets official.
> >
> > I'll send an additional patch in a minute with these documentation
> > related updates.
> 
> Well, well, well....
> 
> The documentation says this is counted in bytes, but somehow neither of us
> found the above " * 1024 * 1024" suspicious.
> 
> Shouldn't it be at least like this?  It would probably be a good idea to
> use git_parse_ulong() or somesuch while we are at it.

Yes, definitely.  I'm about to post a patch moving --max-pack-size in 
that direction too.  I just had to fix a couple other unsuspected issues 
to get there though.  Patches will follow shortly.


Nicolas

^ permalink raw reply

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Junio C Hamano @ 2010-02-04  2:27 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Shawn O. Pearce, Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <7vock5yby2.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> "Shawn O. Pearce" <spearce@spearce.org> writes:
>
>> In my v3 patch I thought I replaced this code with:
>>
>> +               else if (!prefixcmp(a, "--big-file-threshold=")) {
>> +                       unsigned long v;
>> +                       if (!git_parse_ulong(a + 21, &v))
>> +                               usage(fast_import_usage);
>> +                       big_file_threshold = v;
>>
>> So we relied on git_parse_ulong to handle unit suffixes as well.
>
> Yeah, you did; but it didn't carried through the merge process across the
> code restructure to add "option_blah" stuff.  Sorry about that.

And this is a fix-up for the mismerge.  I didn't touch max-pack-size stuff.

 fast-import.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index ca21082..a6730d0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2800,7 +2800,10 @@ static int parse_one_option(const char *option)
 	if (!prefixcmp(option, "max-pack-size=")) {
 		option_max_pack_size(option + 14);
 	} else if (!prefixcmp(option, "big-file-threshold=")) {
-		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
+		unsigned long v;
+		if (!git_parse_ulong(option + 19, &v))
+			return 0;
+		big_file_threshold = v;
 	} else if (!prefixcmp(option, "depth=")) {
 		option_depth(option + 6);
 	} else if (!prefixcmp(option, "active-branches=")) {

^ permalink raw reply related

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Junio C Hamano @ 2010-02-04  2:25 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <20100204020756.GP14799@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> In my v3 patch I thought I replaced this code with:
>
> +               else if (!prefixcmp(a, "--big-file-threshold=")) {
> +                       unsigned long v;
> +                       if (!git_parse_ulong(a + 21, &v))
> +                               usage(fast_import_usage);
> +                       big_file_threshold = v;
>
> So we relied on git_parse_ulong to handle unit suffixes as well.

Yeah, you did; but it didn't carried through the merge process across the
code restructure to add "option_blah" stuff.  Sorry about that.

Looking at the output from

    $ git grep -n -e ' \* 1024 \* 1024' -- '*.c'

I noticed another issue.  Don't we need the same thing for max_packsize?
Or is that _too much_ of a backward incompatible change that we should
wait until 1.7.1?

^ permalink raw reply

* Re: pack.packSizeLimit, safety checks
From: Junio C Hamano @ 2010-02-04  2:14 UTC (permalink / raw)
  To: Nicolas Pitre; +Cc: Junio C Hamano, Sergio, git
In-Reply-To: <alpine.LFD.2.00.1002011240510.1681@xanadu.home>

Nicolas Pitre <nico@fluxnic.net> writes:

> Thing is... I don't know if the --max-pack-size argument is really that 
> used.  I'd expect people relying on that feature to use the config 
> variable instead,...

I suspect one of us need to be careful not to forget this thing...

-- >8 --
Subject: pack-objects --max-pack-size=<n> counts in bytes

The --window-memory argument and pack.packsizelimit configuration used by
the same program counted in bytes and honored the standard k/m/g suffixes.
Make this option do the same for consistency.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/RelNotes-1.7.0.txt   |    6 ++++++
 Documentation/git-pack-objects.txt |    3 ++-
 builtin-pack-objects.c             |    7 +++----
 3 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/Documentation/RelNotes-1.7.0.txt b/Documentation/RelNotes-1.7.0.txt
index 323ae54..adf8824 100644
--- a/Documentation/RelNotes-1.7.0.txt
+++ b/Documentation/RelNotes-1.7.0.txt
@@ -46,6 +46,12 @@ Notes on behaviour change
    environment, and diff.*.command and diff.*.textconv in the config
    file.
 
+ * "git pack-objects --max-pack-size=<n>" used to count in megabytes,
+   which was inconsistent with its corresponding configuration
+   variable and other options the command takes.  Now it counts in bytes
+   and allows standard k/m/g suffixes to be given.
+
+
 Updates since v1.6.6
 --------------------
 
diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt
index 097a147..fdaf775 100644
--- a/Documentation/git-pack-objects.txt
+++ b/Documentation/git-pack-objects.txt
@@ -106,7 +106,8 @@ base-name::
 	default.
 
 --max-pack-size=<n>::
-	Maximum size of each output packfile, expressed in MiB.
+	Maximum size of each output packfile, expressed in bytes.  The
+	size can be suffixed with "k", "m", or "g".
 	If specified,  multiple packfiles may be created.
 	The default is unlimited, unless the config variable
 	`pack.packSizeLimit` is set.
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 4a41547..33e11d7 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -2203,11 +2203,10 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!prefixcmp(arg, "--max-pack-size=")) {
-			char *end;
-			pack_size_limit_cfg = 0;
-			pack_size_limit = strtoul(arg+16, &end, 0) * 1024 * 1024;
-			if (!arg[16] || *end)
+			unsigned long ul = 0;
+			if (!git_parse_ulong(arg + 16, &ul))
 				usage(pack_usage);
+			pack_size_limit_cfg = ul;
 			continue;
 		}
 		if (!prefixcmp(arg, "--window=")) {

^ permalink raw reply related

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Shawn O. Pearce @ 2010-02-04  2:07 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <7v7hqtzrmq.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> wrote:
> 
> Well, well, well....
> 
> The documentation says this is counted in bytes, but somehow neither of us
> found the above " * 1024 * 1024" suspicious.
> 
> Shouldn't it be at least like this?  It would probably be a good idea to
> use git_parse_ulong() or somesuch while we are at it.
> 
>  fast-import.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/fast-import.c b/fast-import.c
> index ca21082..ea1ac0f 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2800,7 +2800,7 @@ static int parse_one_option(const char *option)
>  	if (!prefixcmp(option, "max-pack-size=")) {
>  		option_max_pack_size(option + 14);
>  	} else if (!prefixcmp(option, "big-file-threshold=")) {
> -		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
> +		big_file_threshold = strtoumax(option + 19, NULL, 0);

In my v3 patch I thought I replaced this code with:

+               else if (!prefixcmp(a, "--big-file-threshold=")) {
+                       unsigned long v;
+                       if (!git_parse_ulong(a + 21, &v))
+                               usage(fast_import_usage);
+                       big_file_threshold = v;

So we relied on git_parse_ulong to handle unit suffixes as well.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] fast-import: Stream very large blobs directly to pack
From: Junio C Hamano @ 2010-02-04  2:01 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, Sverre Rabbelier, git, Nicolas Pitre
In-Reply-To: <20100201152826.GE8916@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> The strtoumax call got messed up.  Squash this into your merge:
>
> diff --git a/fast-import.c b/fast-import.c
> index e6ebcf6..9c65a24 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2800,7 +2800,7 @@ static int parse_one_option(const char *option)
>  	if (!prefixcmp(option, "max-pack-size=")) {
>  		option_max_pack_size(option + 14);
>  	} else if (!prefixcmp(option, "big-file-threshold=")) {
> -		big_file_threshold = strtoumax(option + 21, NULL, 0) * 1024 * 1024;
> +		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
>  	} else if (!prefixcmp(option, "depth=")) {
>  		option_depth(option + 6);
>  	} else if (!prefixcmp(option, "active-branches=")) {
>  
>> You may want to add the new option to the output from "cmd -h" and
>> probably description of the configuration in the doc before any of this
>> gets official.
>
> I'll send an additional patch in a minute with these documentation
> related updates.

Well, well, well....

The documentation says this is counted in bytes, but somehow neither of us
found the above " * 1024 * 1024" suspicious.

Shouldn't it be at least like this?  It would probably be a good idea to
use git_parse_ulong() or somesuch while we are at it.

 fast-import.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index ca21082..ea1ac0f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2800,7 +2800,7 @@ static int parse_one_option(const char *option)
 	if (!prefixcmp(option, "max-pack-size=")) {
 		option_max_pack_size(option + 14);
 	} else if (!prefixcmp(option, "big-file-threshold=")) {
-		big_file_threshold = strtoumax(option + 19, NULL, 0) * 1024 * 1024;
+		big_file_threshold = strtoumax(option + 19, NULL, 0);
 	} else if (!prefixcmp(option, "depth=")) {
 		option_depth(option + 6);
 	} else if (!prefixcmp(option, "active-branches=")) {

^ permalink raw reply related

* Re: http getpass function in msysgit
From: Frank Li @ 2010-02-04  1:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, msysGit
In-Reply-To: <7vaavp3i2n.fsf@alter.siamese.dyndns.org>

>
>  - If GIT_ASKPASS is not set:
>
>   - If SSH_ASKPASS is present, then use that from getpass() for any and
>     all places that would want to get "password" like things;
>
>   - Otherwise consult the terminal as before;
>
>  - If GIT_ASKPASS is set:
>
>   - If SSH_ASKPASS is not set, then export the value of GIT_ASKPASS as
>     such as well, so that whenever we spawn "ssh", the same GIT_ASKPASS
>     program will be used as a fallback.
>
> That way, if the user already has set up SSH_ASKPASS, we will use the same
> familiar dialog without forcing the user do anything extra.  If the user
> only sets GIT_ASKPASS without doing SSH_ASKPASS, we would also use it to
> drive the ssh session.  In either case, the user doesn't need to worry
> about multiple configuration or dialog interface.
>
>> It is nice to use one dialog for all cases. git-svn also have the same problem.
>

Can we direct use SSH_ASKPASS for all getpass and don't use GIT_ASKPASS at all?
The logic will be come simple.

^ permalink raw reply

* Re: [gitolite] repo config for delegated projects
From: Sitaram Chamarty @ 2010-02-04  1:18 UTC (permalink / raw)
  To: git discussion list, Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <20100203202249.GA27125@lapse.rw.madduck.net>

On Thu, Feb 04, 2010 at 09:22:49AM +1300, martin f krafft wrote:
> Dear Sitaram, dear Teemo, dear gitolite-fans,
> 
> src/gl-compile-conf:261 prohibits delegated repositories to make use
> of the functionality to configure config variables of the
> repositories:
> 
>   die "$WARN $fragment attempting to set repo configuration\n"
>     if $fragment ne 'master';
> 
> This is a bit unfortunate and makes me reconsider the use of
> delegations.
> 
> What is the reason for this restriction?

Like Teemu said, inability to think through all the possible
repurcussions of allowing a delegated admin to set config
variables.  There are too many of them for me to go through,
and they'll keep changing.

To recap, what gitolite wants to do is broadly the
following:

  - no one who is not admin can do anything to a repo that
    the config file does not permit him to do (this is not
    affected by the topic of this email; just adding it for
    completeness)

  - the main admin (who has RW/RW+ access to all of the
    gitolite-admin repo) cannot get shell access on the
    server.  This is a relatively new restriction; initially
    I did not think to keep these two privileges separate

  - a delegated admin cannot manage any sort of access to
    repos that the main admin did not delegate to him.

> 
> Are there settings that are potentially compromising?
> 
> Would it be worth to consider making it configurable (e.g.
> ~/.gitolite.rc) whether to allow delegated repos to set config
> variables?

I wouldn't mind making it configurable, with the default
being off.  Rather than a blanket

    $ALLOW_DELEGATE_CONFIGS = 1;

how about

    $DELEGATED_CONFIGS = "hooks.mailinglist,hooks.showrev";

(to take Teemu's example config file and the config
variables he uses), so that you (or whoever has shell
access, which is required for changing RC file) can sort of
limit the potential damage.

And the defaults would all be commented out anyway so people
who don't car about this will never have to worry about it.

Regards,

Sitaram

^ permalink raw reply

* Re: [gitolite] symlink hooks instead of copying them
From: martin f krafft @ 2010-02-04  1:46 UTC (permalink / raw)
  To: git discussion list; +Cc: Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <20100204012840.GC497@atcmail.atc.tcs.com>

[-- Attachment #1: Type: text/plain, Size: 1464 bytes --]

also sprach Sitaram Chamarty <sitaram@atc.tcs.com> [2010.02.04.1428 +1300]:
> I'm ok with symlinking stuff; a couple of "cp" commands
> would change to "ln" :)  Let me try it out (and make sure it
> works for upgrades also...)

ln -sf even.



also sprach Sitaram Chamarty <sitaram@atc.tcs.com> [2010.02.04.1435 +1300]:
> I forgot... part of the reason this "copy all hooks over each time
> you run install" is also to give people an easy way to update the
> hooks when the repo was *copied* from elsewhere, and not *created*
> by gitolite in the first place.
> 
> Basically I'm paranoid about that "update" hook, without which the
> branch level access control doesn't work at all.

Wouldn't it thus make sense to check during authentication that the
symlink exists and points to the right file, and to deny access
completely if that isn't the case?

> So this will still need to be done. Or you'll have to provide some
> other command that will sweep through all repos in the $REPO_BASE
> and check that the symlink is pointing to the right place etc etc.

Having a mass-update command for this might be nice, but I suppose
it's also a trivial shell one-liner...

  for i (**/*.git/hooks/update) \
    ln -sf ~git/.gitolite/src/hooks/update $i

(this is zsh, not sure bash can do this yet)

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
apt-get source --compile gentoo
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Re: [gitolite] symlink hooks instead of copying them
From: Sitaram Chamarty @ 2010-02-04  1:35 UTC (permalink / raw)
  To: martin f krafft; +Cc: git discussion list, Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <20100203204723.GA30157@lapse.rw.madduck.net>

On Thu, Feb 04, 2010 at 09:47:23AM +1300, martin f krafft wrote:
> Dear Sitaram, dear Teemo, dear gitolite-fans,
> 
> Gitolite currently copies hooks to repositories. For upgrades, it
> must thus ensure that all hooks are also upgraded.

I forgot... part of the reason this "copy all hooks over
each time you run install" is also to give people an easy
way to update the hooks when the repo was *copied* from
elsewhere, and not *created* by gitolite in the first place.

Basically I'm paranoid about that "update" hook, without
which the branch level access control doesn't work at all.

So this will still need to be done. Or you'll have to
provide some other command that will sweep through all repos
in the $REPO_BASE and check that the symlink is pointing to
the right place etc etc.

Any other ways of doing this?  I'd rather keep it as is, if
it's OK with you, except for changing the cp to ln of course.

^ permalink raw reply

* Re: faceted search interface
From: Junio C Hamano @ 2010-02-04  1:31 UTC (permalink / raw)
  To: Alex Ksikes; +Cc: git
In-Reply-To: <E1NceXz-0003VA-QG@mail.chiefmall.com>

Alex Ksikes <alex@chiefmall.com> writes:

> I have just put together a system to automatically build a faceted
> search interface and search engine from any data. Would you mind having
> a look at it and letting me know what you think of it?
>
> Here is a particular instance on IMDb (the internet movie database)....

The query system might be interesting, but what does this have to do with
this mailing list?

^ permalink raw reply

* Re: [gitolite] symlink hooks instead of copying them
From: Sitaram Chamarty @ 2010-02-04  1:28 UTC (permalink / raw)
  To: martin f krafft; +Cc: git discussion list, Sitaram Chamarty, Teemu Matilainen
In-Reply-To: <20100203204723.GA30157@lapse.rw.madduck.net>

On Thu, Feb 04, 2010 at 09:47:23AM +1300, martin f krafft wrote:
> Dear Sitaram, dear Teemo, dear gitolite-fans,
> 
> Gitolite currently copies hooks to repositories. For upgrades, it
> must thus ensure that all hooks are also upgraded.
> 
> It occurs to me that this might be easier done using symlinks, or
> with a file that includes the master hook(s) in
> ~/.gitolite/src/hooks. Then, the hooks just have to be upgraded in
> one place.
> 
> Do you see a reason not to do this via symlinks?

If you mean just the gitolite-specific hooks (the update
hook for all repos, and the post-update hook for the admin
repo) then no problem.

The other hooks I'd rather not assume anything about.  The
current scheme forces an overwrite of the gitolite-specific
hooks, as well as any hooks given in src/hooks, each time an
"install" is done.  It does not touch any *other* hooks,
which allows the admin to (via command line) place specific
hooks in specific repos manually if he wishes to.

I'm ok with symlinking stuff; a couple of "cp" commands
would change to "ln" :)  Let me try it out (and make sure it
works for upgrades also...)

^ permalink raw reply

* Re: http getpass function in msysgit
From: Junio C Hamano @ 2010-02-04  1:28 UTC (permalink / raw)
  To: Frank Li; +Cc: Junio C Hamano, git, msysGit
In-Reply-To: <1976ea661002022148s544be10bie912295e04189864@mail.gmail.com>

Frank Li <lznuaa@gmail.com> writes:

> You can choose environment name you like. I choose HTTP_ASKPASS just
> because getpass only used at http.c
> ssl_cert_password = getpass("Certificate Password: ");
>
> OpenSSH is separated application and use own SSH_ASKPASS to ask password ...
>
> May GIT_ASKPASS is optional name.

If that is the case, probably it is easiest for the end users if you
arrange it this way:

 - If GIT_ASKPASS is not set:

   - If SSH_ASKPASS is present, then use that from getpass() for any and
     all places that would want to get "password" like things;

   - Otherwise consult the terminal as before;

 - If GIT_ASKPASS is set:

   - If SSH_ASKPASS is not set, then export the value of GIT_ASKPASS as
     such as well, so that whenever we spawn "ssh", the same GIT_ASKPASS
     program will be used as a fallback.

That way, if the user already has set up SSH_ASKPASS, we will use the same
familiar dialog without forcing the user do anything extra.  If the user
only sets GIT_ASKPASS without doing SSH_ASKPASS, we would also use it to
drive the ssh session.  In either case, the user doesn't need to worry
about multiple configuration or dialog interface.

> It is nice to use one dialog for all cases. git-svn also have the same problem.

People often seem to use "/usr/lib{,exec}/openssh/gnome-ssh-askpass" as
SSH_ASKPASS.  It takes the prompt from its command line argument, reads
the input, and spits it out to its standard output so that calling program
can capture it.  It would be a good interface to conform to to minimize
the work we need to support this.

^ permalink raw reply

* Re: Question about git rebase --onto
From: Junio C Hamano @ 2010-02-04  1:12 UTC (permalink / raw)
  To: pascal; +Cc: git list
In-Reply-To: <4B686CAC.7020103@obry.net>

Pascal Obry <pascal@obry.net> writes:

>> Possible user errors I can think of are:
>> ...
> I'm not on this case.
> ...
> I'm not in this case either.

Thenk there are some _other_ differences between your situation and what I
can construct from your problem description:

    It seems to me that:

       $ git co topic
       $ git rebase --onto master topic~2 topic

    Used to do the job and rebase the topic branch as expected....

which I tried myself before giving you a reply, and I didn't end up on a
detached HEAD.  But I cannot guess what that difference you are not
telling us is (and you probably don't know it either, I suspect).

Writing reproducible test case would be a great way to diagnose this.

^ permalink raw reply

* Re: git-mv redux: there must be something else going on
From: Junio C Hamano @ 2010-02-04  0:48 UTC (permalink / raw)
  To: Ron Garret; +Cc: git
In-Reply-To: <ron1-9FA846.14332803022010@news.gmane.org>

Ron Garret <ron1@flownet.com> writes:

> A and B start with a file named config.  A and B both make edits.  In 
> addition, B renames config to be config1 and creates a new, very similar 
> file called config2.  B then merges from A with the expectation that B's 
> edits to config would end up in config1 and not config2.  It seems to me 
> that without tracking renames, it would be luck of the draw which file 
> the patch got applied to.

I don't think the above is necessarily "rename" issue, but touches an
interesting point -- it is so "interesting" to the point that no sane SCM
would even consider that is a problem they need to solve.

If config1 and config2 are about two different ways to configure the
software (e.g. two different build for different customers), and change
made by A was to accomodate new configuration option made in the upstream,
B might even want to have that addition reflected in _both_ of his
configuration files, config1 and config2.

Earlier in this message, I said that this is not an issue SCM should even
be solving, because a sane way to handle this would _not_ be to copy and
edit config1/config2 and keep track of them in SCM; instead, saner people
would maintain a build procedure (e.g. Makefile target) to transform the
template "config" into necessary "config1" and "config2" customized
variants.

^ permalink raw reply

* Re: extra headers in commit objects
From: A Large Angry SCM @ 2010-02-04  0:41 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: demerphq, Nicolas Pitre, git
In-Reply-To: <20100203192612.GD14799@spearce.org>

Shawn O. Pearce wrote:
> demerphq <demerphq@gmail.com> wrote:
>> On 3 February 2010 19:15, Nicolas Pitre <nico@fluxnic.net> wrote:
>>> On Wed, 3 Feb 2010, Shawn O. Pearce wrote:
>>>
>>>> Am I correct that core C developers are still under the opinion
>>>> that extra headers in a commit object aren't encouraged?
>>> I would say so.
>>>
>>> [...]
>>>> At the end of the day, is it a bug that C git doesn't support
>>>> working with extra commit headers? ?IMHO, no, because, we've
>>>> rejected these in the past, and its not part of the Git standard.
>>>> And other implementations shouldn't be trying to sell it that way.
>>> Agreed. ?And this was discussed in great length on this list on few
>>> occasions already (probably more than a year back).
>> One problem, is that if you take the approach you say then you
>> basically guarantee that a new git that DOES add new headers will
>> break an old git that doesnt know about the headers, and actually
>> doesnt care about them either.
> 
> As I understand it, the current stance is:
> 
> 1) A compliant Git implementation ignores any headers it doesn't
>    recognize that appear *after* the optional "encoding" header.
> 
> 2) A compliant Git implementation does not produce any additional
>    headers in a commit object, because other implementations cannot
>    perform any machine based reasoning on them.
> 
> 3) All implementations would (eventually) treat all headers equally,
>    that is they all understand what author, committer, encoding are
>    and process them the same way.  Any new headers should equally
>    be fully cross-implementation.
> 
>> So it would essentially mean that if you ever have to change the
>> commit format you will be in a position where new git commits will be
>> incompatible by design with old git commits.
> 
> So, we can change the format by adding a new header, after the
> optional "encoding" header.
> 
> But such a change needs to be something that an older Git will
> safely ignore (due to rule 1), and something that a newer Git can
> make really effective use of (due to rule 2 and 3).  And that newer
> Git must also safely deal with commits missing that new header, due
> to the huge number of commits out in the wild without said header.
> 
> And don't even get me started on amending commits with new unknown
> headers.  Existing implementions of Git tools will drop the extra
> headers during the amend, because the headers are viewed as part
> of the commit object data... and during an amend you are making a
> totally new object.
> 
> For example, git-gui would drop any extra headers during an amend,
> because its running `git commit-tree` directly without any way to
> tell commit-tree this is for an amend of an existing commit, vs. a
> completely new commit... because either way its a new commit object.
> 
>> Shouldn't an old git just ignore headers from a new git?
> 
> Yes, see above.
>  

4) C-git "owns" the header name space. The git ML is _the_ controlling 
standards body.

^ permalink raw reply

* Re: extra headers in commit objects
From: Junio C Hamano @ 2010-02-04  0:38 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: demerphq, Nicolas Pitre, git
In-Reply-To: <20100203210407.GG14799@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> We all know that some other systems, e.g. SVN, permit adding
> additional properties to commits, and that often these are used
> to make statements like "Fixed bug NNNN", and bug tracking systems
> integrate into SVN by reading or updating those properties.
>
> So you, Nico, myself, might all agree that "bug" does not belong
> in the header, but many others see it like SVN sees additional
> properties on a revision, and thus it goes there.
>
> Hence the artifical example.  It seems that it is not that artifical
> outside of our mailing list.

Aren't the meta-properties like "Fixed bug NNNN" something you can add
after the fact, even in SVN?

We have that in "notes".  I never said people are wrong for wanting to
record additional information _about_ commits somewhere (and I didn't say
"artificial" at all---it was you who said it was a "made-up" example).

My point was that they do not belong to the commit _header_, and "but many
others see" doesn't contradict with that.  Many others may feel the need
to be able to express random things _about_ the commit; it does not mean
these random things have to go _in_ the commit.

^ permalink raw reply

* Re: git-mv redux: there must be something else going on
From: Ron Garret @ 2010-02-04  0:10 UTC (permalink / raw)
  To: git
In-Reply-To: <32541b131002031518t1017d351xcf9071f0a937474e@mail.gmail.com>

In article 
<32541b131002031518t1017d351xcf9071f0a937474e@mail.gmail.com>,
 Avery Pennarun <apenwarr@gmail.com> wrote:

> On Wed, Feb 3, 2010 at 5:33 PM, Ron Garret <ron1@flownet.com> wrote:
> > Here's a realistic case where keeping explicit track of renames could be
> > useful.
> >
> > A and B start with a file named config.  A and B both make edits.  In
> > addition, B renames config to be config1 and creates a new, very similar
> > file called config2.  B then merges from A with the expectation that B's
> > edits to config would end up in config1 and not config2.  It seems to me
> > that without tracking renames, it would be luck of the draw which file
> > the patch got applied to.
> 
> The problem is that this single "realistic case" is not actually very
> common, and it's dwarfed by the other realistic cases: developer
> forgets to use 'git mv' to rename the file; developer accidentally
> deletes a file, commits, and then readds it later; etc.

Makes sense.

> Have I been bitten by exactly your example?  Yup.  But I've been
> bitten by lots of other related things too, and explicit rename
> tracking (at least in svn) has quite frequently made the problems
> *worse*.  In my personal experience, git screws up less often.  The
> fact that it's also elegant is a nice bonus too :)
> 
> More about this: http://marc.info/?l=git&m=114123702826251

Thanks, that's a great read!

rg

^ permalink raw reply

* Re: git-mv redux: there must be something else going on
From: Ron Garret @ 2010-02-04  0:10 UTC (permalink / raw)
  To: git
In-Reply-To: <76718491002031555i2c1558f9qe0c97d07ceb86bb6@mail.gmail.com>

In article 
<76718491002031555i2c1558f9qe0c97d07ceb86bb6@mail.gmail.com>,
 Jay Soffian <jaysoffian@gmail.com> wrote:

> On Wed, Feb 3, 2010 at 6:18 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
> > More about this: http://marc.info/?l=git&m=114123702826251
> 
> I think the canonical email on the subject is this one:
> 
> http://article.gmane.org/gmane.comp.version-control.git/217
> 

The upshot seems to be this:

> And that "where did this come from" decision should be done at _search_ 
> time, not commit time.

And I'm mostly convinced, except for the one screw case that I outlined 
above.  In that case the search-time result is ambiguous, and file 
tracking information could be used to resolve the ambiguity.  But it 
certainly does seem like a rare enough situation that it's not worth 
worrying about.

I think I'm starting to git it :-)

rg

^ permalink raw reply

* Re: git-mv redux: there must be something else going on
From: Jay Soffian @ 2010-02-03 23:55 UTC (permalink / raw)
  To: Avery Pennarun; +Cc: Ron Garret, git
In-Reply-To: <32541b131002031518t1017d351xcf9071f0a937474e@mail.gmail.com>

On Wed, Feb 3, 2010 at 6:18 PM, Avery Pennarun <apenwarr@gmail.com> wrote:
> More about this: http://marc.info/?l=git&m=114123702826251

I think the canonical email on the subject is this one:

http://article.gmane.org/gmane.comp.version-control.git/217

:-)

j.

^ permalink raw reply

* Re: [PATCH] grep: simple test for operation in a bare repository
From: René Scharfe @ 2010-02-03 23:50 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Junio C Hamano
In-Reply-To: <4B69BD7C.4010608@lsrfire.ath.cx>

Am 03.02.2010 19:16, schrieb René Scharfe:
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
>  t/t7002-grep.sh |   17 +++++++++++++++++
>  1 files changed, 17 insertions(+), 0 deletions(-)

Err, no, that won't do.  Sorry.

The test script fails to demonstrate the issue I've run into.  It runs
successfully, but running git grep manually fails:

	$ cd t/trash\ directory.t7002-grep/.git/bare_test_repo/
	$ git grep bla HEAD
	fatal: This operation must be run in a work tree

I have to dig a bit deeper and try to come back with a better test script.

René

^ permalink raw reply

* Re: git-mv redux: there must be something else going on
From: Avery Pennarun @ 2010-02-03 23:18 UTC (permalink / raw)
  To: Ron Garret; +Cc: git
In-Reply-To: <ron1-9FA846.14332803022010@news.gmane.org>

On Wed, Feb 3, 2010 at 5:33 PM, Ron Garret <ron1@flownet.com> wrote:
> Here's a realistic case where keeping explicit track of renames could be
> useful.
>
> A and B start with a file named config.  A and B both make edits.  In
> addition, B renames config to be config1 and creates a new, very similar
> file called config2.  B then merges from A with the expectation that B's
> edits to config would end up in config1 and not config2.  It seems to me
> that without tracking renames, it would be luck of the draw which file
> the patch got applied to.

The problem is that this single "realistic case" is not actually very
common, and it's dwarfed by the other realistic cases: developer
forgets to use 'git mv' to rename the file; developer accidentally
deletes a file, commits, and then readds it later; etc.

Have I been bitten by exactly your example?  Yup.  But I've been
bitten by lots of other related things too, and explicit rename
tracking (at least in svn) has quite frequently made the problems
*worse*.  In my personal experience, git screws up less often.  The
fact that it's also elegant is a nice bonus too :)

More about this: http://marc.info/?l=git&m=114123702826251

Have fun,

Avery

^ permalink raw reply

* Re: [gitolite] repo config for delegated projects
From: Teemu Matilainen @ 2010-02-03 22:47 UTC (permalink / raw)
  To: git discussion list; +Cc: Sitaram Chamarty
In-Reply-To: <20100203202249.GA27125@lapse.rw.madduck.net>

On Thu, 04 Feb 2010, martin f krafft wrote:

> src/gl-compile-conf:261 prohibits delegated repositories to make use
> of the functionality to configure config variables of the
> repositories:
> 
>   die "$WARN $fragment attempting to set repo configuration\n"
>     if $fragment ne 'master';
> 
> This is a bit unfortunate and makes me reconsider the use of
> delegations.
> 
> What is the reason for this restriction?

Well, the main reason probably is that I don't personally use delegation
and got tired of even thinking about the security concerns. =)

> Are there settings that are potentially compromising?

I think it depends on the setup and especially hooks.
Can't come up with any real problem, though.

> Would it be worth to consider making it configurable (e.g.
> ~/.gitolite.rc) whether to allow delegated repos to set config
> variables?

That's Sitaram's call. :)


-- 
	- Teemu

^ permalink raw reply

* Re: extra headers in commit objects
From: Shawn O. Pearce @ 2010-02-03 22:48 UTC (permalink / raw)
  To: Scott Chacon; +Cc: git
In-Reply-To: <d411cc4a1002031158k3e50db30l3f7d73d49e3dad23@mail.gmail.com>

Scott Chacon <schacon@gmail.com> wrote:
> On Wed, Feb 3, 2010 at 9:40 AM, Shawn O. Pearce <spearce@spearce.org> wrote:
> > Today I came across this "bug fix" [1,2] in Dulwich, which is
> > claiming to be a pure-Python implementation of Git.
> >
> > I haven't spoken with Jelmer Vernooij directly about it, but after
> > some indirect email through a 3rd party, it seems he might be under
> > the impression that this really is a bug in Dulwich, because "other
> > git implementations do it".
> 
> At the risk of pissing you off for the second time in as many days,
> this is entirely my fault.

Apparently, s**t happens is a good phrase.  One I need to learn.

> I was having a beer with Jelmer in Wellington a few weeks ago

And... beer doesn't promote clear thinking.

All is forgiven.  As is yesterday's remark about not telling me
sooner about a JGit bug.  You really didn't do anything bad, I
just woke up on the wrong side of the bed the past couple of days,
and sort of went off...

Sorry.  :-\

> Anyhow, I was saying that _technically_ you can artificially write
> extra headers into the commit object (though at the time Dulwich
> didn't support reading them because of how it parsed commit objects -
> I believe it would actually explode if it saw something it didn't
> expect).  I said I was still going to keep the metadata in my
> implementation in the message, but he was very interested in hiding
> his in the commit headers.

Yea, everyone wants to hide that extra metadata.  I never get why.
Even in SVN.  Why wouldn't I want to see the bug(s) fixed by
a commit?  Difference of opinion.  I also happen to prefer the
color blue.  Dammit, everyone should prefer blue.

> To my defense, we (you and I, Shawn)
> talked about this at the GitTogether this year and you and a few
> others told me that CGit would not blow up but would just ignore them,
> which is fine for his purposes.  I certainly did not get the
> impression from that short discussion that this was something to be
> absolutely avoided, but rather that it just wasn't really encouraged
> or explicitly supported.

Sorry.  I've held this same opinion as Junio and Nico have expressed
in this thread, that although we ignore extra headers, its only to
leave us an escape hatch in case we add something like "encoding"
in the future.  Adding encoding was almost a nightmare because we
didn't have that escape hatch.

I also hold the opinion that the C implementation is correct,
and everyone else is wrong.  Even JGit.  Unless its a bug in the
C implementation, in which case the bug fix is correct.  :-)

Which in this case means, if the C implementation doesn't give
the user plumbing to do something (aside from using git mkobject),
you really should think twice before doing it.

So I apologize if I gave you the wrong impression at the GitTogether.
I claim stupidity as my only defense.

> Sorry.  So, for future reference, though CGit _can_ handle it, don't?

C Git won't choke if there are extra headers.

But we _really_ don't want them.  And C Git won't be writing any new
headers anytime soon.  I think we're more likely to shift the entire
hashing scheme to SHA-512 or something before we add a new header.

-- 
Shawn.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox