Git development
 help / color / mirror / Atom feed
* Re: [PATCH] git-svn: Destroy the cache when we fail to read it
From: Eric Wong @ 2011-08-23  8:15 UTC (permalink / raw)
  To: Jason Gross; +Cc: git, Jonathan Nieder
In-Reply-To: <1313979422-21286-1-git-send-email-jgross@mit.edu>

Jason Gross <jgross@MIT.EDU> wrote:
>  		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";

Can we make the caches sharable by encoding variables like
$Config{use64bitint} and $Storable::VERSION into $cache_path?

Something like this (untested):

	use Config;

	my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
	$cache_path .= "$Config{use64bitint}.$Storable::VERSION/";

We'll blow everybody's cache away once during the git-svn upgrade, but
in the future people will be able to share the same working tree between
different versions of perl/Storable/whatnot without needing extra code
to detect croaks, different build options, and nuking each other's
caches.

-- 
Eric Wong

^ permalink raw reply

* Re: How to check out the repository at a particular point in time
From: R. Diez @ 2011-08-23  7:41 UTC (permalink / raw)
  To: PJ Weisberg; +Cc: Thomas Rast, git@vger.kernel.org, in-git-vger@baka.org


> "master" is nothing more than a pointer to a
> particular commit.  The commit has references
> to its parent(s), from which you can build a
> whole history.  In general, Git doesn't
> know the name of the branch a developer had
> checked out when a commit was created.
> In fact, in this example, both branches
> could have been called "master" if they
> lived in different developers' workspaces.

I understand. However, let's see if git can cope with this pretty common scenario:

Say I'm a developer with too many projects and little time. I don't really want to find the commit ID at each release and manually make a note on the project's web site. In fact, I have no "official" releases. The "contract" with my users (or co-developers in my team) is simple: check out HEAD, it should always work. If it doesn't, last week it worked well, check out at that point in time and wait until I fix the HEAD.

Now you're saying I cannot reliably checkout last week's versions because yesterday I did a merge from an older branch? You mean that git stores everything with clean graphs and numeric pointers, so it cannot know what this repository looked like last week?

As the developer, I have full control, I can decide what the branches are called and how the public repository is updated/pushed/whatever. I can control the clock so there are no time skews.

What do I have to do in order to be able to reliably checkout last week's versions without too much administrative work? I just want to get the same result today as if I had done a checkout last week from the public repository and had made a back-up copy of the working directory then.

With say CVS and Subversion, that's piece of cake, they already work that way, I don't need to manually keep track of revision IDs or visually inspect the merge tree.

Thanks for your answers,
  R. Diez

^ permalink raw reply

* What's the difference between `git show branch:file | diff -u - file` vs `git diff branch file`?
From: Marat Radchenko @ 2011-08-23  7:25 UTC (permalink / raw)
  To: git

$ time git show branch:file | diff -u - file > /dev/null 

real    0m0.003s
user    0m0.000s
sys     0m0.000s

$ time git diff branch -- file > /dev/null 

real    0m31.442s
user    0m31.040s
sys     0m0.380s

What does git diff do so it takes that much time? And is there any flag to git 
diff so that it will work as fast as show + diff? I thought these two are 
equivalent but from run time it is obvious that they aren't.

gprof output: http://slonopotamus.org/git-diff/git-diff-branch.gprof.txt

^ permalink raw reply

* Re: [PATCH] Mark http-fetch without -a as deprecated
From: Matthieu Moy @ 2011-08-23  7:02 UTC (permalink / raw)
  To: Ben Walton; +Cc: git, gitster
In-Reply-To: <1314060703-24265-1-git-send-email-bwalton@artsci.utoronto.ca>

Ben Walton <bwalton@artsci.utoronto.ca> writes:

> +	if (get_all == 0)
> +	  warning("http-fetch: use without -a is deprecated; -a will become the default in a future release");

Bad indentation. Git indents with tabs, not spaces. Also, you can break
the line to make both the error message and the source code fit on 80
characters.

No opinion on whether the deprecation is sensible, though. If it is, add
something in the documentation, not just in the code.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: git svn --stdlayout 's little quirk.
From: Andreas Krey @ 2011-08-23  6:09 UTC (permalink / raw)
  To: Michael J Gruber; +Cc: htl10, Eric Wong, madduck, git
In-Reply-To: <4E52759E.3020005@drmicha.warpmail.net>

On Mon, 22 Aug 2011 17:28:30 +0000, Michael J Gruber wrote:
...
> You *could* have this layout:
> 
> foo/trunk/trunk
> foo/trunk/tags/v1
> foo/trunk/tags/v2

We *do*. Ok, it's more like dev/trunk/mod/submod/trunk (and sometimes with
even more trunks). Stupid historical accident.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

^ permalink raw reply

* Re: [PATCH] git-svn: Destroy the cache when we fail to read it
From: Jason Gross @ 2011-08-23  5:51 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, Eric Wong
In-Reply-To: <20110823022717.GA4623@elie.gateway.2wire.net>

1) Sure.  I'm not sure whether I should make $cache_path toplevel, or
a parameter of do_memoization.  I'm thinking the latter, just in case
$ENV{GIT_DIR} changes between calls to
memoize_svn_mergeinfo_functions?

2) I'm not sure if there's a clean/robust way to detect cache
corruption; if it's completely corrupted, Storable complains about not
finding the right magic number, but I could imagine things being
corrupted in more subtle ways that give rise to other Storable errors.
As for your worry: I don't know the code well enough to have git-svn
recover from some other error in memoization; if there's an error,
that means that it can't read the cache correctly (either in tieing
the hashes, or in memoizing the functions).  The other alternative
that I see is to die.  Is this preferable to repeatedly discarding the
cache?

3) Will fix.

4) Will do.

I'll send a new patch in once I hear back from you about 1 and 2.

-Jason

On Mon, Aug 22, 2011 at 10:27 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> (+cc: Eric Wong)
> Hi Jason,
>
> Jason Gross wrote:
>
> [This patch teaches "git svn" to invalidate caches when they
>  fail to load, for example because the endianness or size of
>  some type changed, which is common in the perl 5.6 -> 5.8
>  upgrade.]
>
> > http://lists.debian.org/debian-perl/2011/05/msg00023.html and
> > http://lists.debian.org/debian-perl/2011/05/msg00026.html).
> [...]
> > --- a/git-svn.perl
> > +++ b/git-svn.perl
> > @@ -1680,7 +1680,7 @@ use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
> >              $_use_svnsync_props $no_reuse_existing $_minimize_url
> >           $_use_log_author $_add_author_from $_localtime/;
> >  use Carp qw/croak/;
> > -use File::Path qw/mkpath/;
> > +use File::Path qw/mkpath rmtree/;
> >  use File::Copy qw/copy/;
> >  use IPC::Open3;
> >  use Memoize;  # core since 5.8.0, Jul 2002
> > @@ -3198,28 +3198,41 @@ sub has_no_changes {
> >               $memoized = 1;
> >
> >               my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
> > -             mkpath([$cache_path]) unless -d $cache_path;
> > -
> > -             tie my %lookup_svn_merge_cache => 'Memoize::Storable',
> [...]
> > -             ;
> > +             my $do_memoization = sub {
> > +                     mkpath([$cache_path]) unless -d $cache_path;
> [...]
> > +                     ;
> > +             };
> > +
> > +             if (not eval {
> > +                     $do_memoization->();
> > +                     1;
> > +             }) {
> > +                     my $err = $@ || "Zombie error"; # "Zombie error" to catch clobbered $@ in buggy destructors
> > +                     die $err unless -d $cache_path;
> > +                     print STDERR "Discarding cache and trying again ($@)\n";
> > +                     rmtree([$cache_path]);
> > +                     $do_memoization->();
> > +             }
>
> Thank you thank you thank you.
>
> Okay, time for nitpicks:
>
>  1) Would it be possible to lift this do_memoization() to a toplevel sub?
>    I suspect that could make the code a little easier to read.
>
>  2) Is it important to discard the cache for _all_ errors, instead
>    of just corruption and "is not compatible" errors?  Rebuilding the
>    cache is not cheap, and I am afraid of effects like repeatedly
>    discarding the cache only to rebuild it again due to a typo in
>    git-svn.perl or an out-of-memory condition.
>
>  3) The line with "Zombie error" is very long --- I guess putting the
>    comment on the line before would help.
>
>  4) The series would be clearer imho as a single patch that includes
>    both the fix and tests.
>
> Eric, what do you think?
>
> Thanks again, :)
> Jonathan

^ permalink raw reply

* Re: [PATCH 2/6] Access reference caches only through new function get_cached_refs().
From: Michael Haggerty @ 2011-08-23  4:21 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
	Josh Triplett
In-Reply-To: <7vzkjblk22.fsf@alter.siamese.dyndns.org>

On 08/15/2011 12:12 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> +	if (! submodule)
> 
> (style) lose the SP before "submodule".

Will be fixed in re-roll.

>> -	if (!refs->did_packed || submodule) {
>> +	if (!refs->did_packed) {
>> +		const char *packed_refs_file;
>> +		if (submodule)
>> +			packed_refs_file = git_path_submodule(submodule, "packed-refs");
>> +		else
>> +			packed_refs_file = git_path("packed-refs");
>>  		FILE *f = fopen(packed_refs_file, "r");
> 
> decl-after-statement.

Will be fixed.

On 08/15/2011 12:21 AM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> +struct cached_refs *create_cached_refs()
>
> struct cached_refs *create_cached_refs(void)

Will be fixed.

Thanks for your feedback.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply

* Re: Re* [PATCH 1/2] clone: allow to clone from .git file
From: Nguyen Thai Ngoc Duy @ 2011-08-23  4:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7v8vqku6le.fsf@alter.siamese.dyndns.org>

On Tue, Aug 23, 2011 at 10:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> But I ended up deciding not to add "gently" variant for now, as nobody
> needs it, and if we were to have "gently", we should also rethink the
> earlier checks in the function if they should give diagnosis. Please check
> what is queued near the tip of tonight's 'pu'.

Looks OK.
-- 
Duy

^ permalink raw reply

* Re: Re* [PATCH 1/2] clone: allow to clone from .git file
From: Junio C Hamano @ 2011-08-23  3:53 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8ABBB69V3pFjGohJwAQbZ_MwGqv=p27tUHF8HAGRMLpQw@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Set *status here too? I assume we need valid *status whenever NULL is returned.

You are absolutely correct.

But I ended up deciding not to add "gently" variant for now, as nobody
needs it, and if we were to have "gently", we should also rethink the
earlier checks in the function if they should give diagnosis. Please check
what is queued near the tip of tonight's 'pu'.

Thanks.

^ permalink raw reply

* Re: [PATCH 0/2] clone-local fixup
From: Hui Wang @ 2011-08-23  3:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Hui Wang
In-Reply-To: <1314061516-24964-1-git-send-email-gitster@pobox.com>

This patch series solved the problem from the root cause. :-)

But there is a little problem in the second patch, i provide an 
incremental patch basing on your second patch, if it is fine to you, it 
is OK to squash this patch to your second patch.


commit 47e890818fdac5f6493cd0bbaf9da350785a2bca
Author: Hui Wang <jason77.wang@gmail.com>
Date:   Tue Aug 23 11:32:26 2011 +0800

    clone: don't change absolute path in the copy_alternates
   
    The source alternates may include empty lines, comment lines, relative
    paths and absolute paths. Absolute paths don't need to be changed
    before added to the destination alternates.
   
    Signed-off-by: Hui Wang <jason77.wang@gmail.com>

diff --git a/builtin/clone.c b/builtin/clone.c
index 2842707..284e325 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -258,6 +258,10 @@ static void copy_alternates(struct strbuf *src, 
struct strbuf *dst,
                char *abs_path, abs_buf[PATH_MAX];
                if (!line.len || line.buf[0] == '#')
                        continue;
+               if (is_absolute_path(line.buf)) {
+                       add_to_alternates_file(line.buf);
+                       continue;
+               }
                abs_path = mkpath("%s/objects/%s", src_repo, line.buf);
                normalize_path_copy(abs_buf, abs_path);
                add_to_alternates_file(abs_buf);


Regards,
Hui.

Junio C Hamano wrote:
> So I ended up tackling this myself. I didn't bother moving the
> add_to_alternates_file(), though.
>
> Junio C Hamano (2):
>   clone: allow more than one --reference
>   clone: clone from a repository with relative alternates
>
>  builtin/clone.c  |   82 +++++++++++++++++++++++++++++++++++++++++------------
>  sha1_file.c      |    2 +-
>  t/t5601-clone.sh |   23 +++++++++++++++
>  3 files changed, 87 insertions(+), 20 deletions(-)
>
>   

^ permalink raw reply related

* Re: Restoring timestamps (Re: Branches & directories)
From: Hilco Wijbenga @ 2011-08-23  3:20 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Kyle Moffett, Michael Witten, Junio C Hamano, Evan Shelhamer,
	Git Mailing List
In-Reply-To: <CAE1pOi1J=TWUmJKZorotBsDoz3wozXsioN7fVO=7JBxdMD7Zqg@mail.gmail.com>

On 22 August 2011 20:06, Hilco Wijbenga <hilco.wijbenga@gmail.com> wrote:
> On 22 August 2011 16:21, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Hilco Wijbenga wrote:
>>
>>> You mean an extra dotfile per file in the commit?
>>
>> Let me step back a moment.
>>
>> When you mentioned mtime, a switch went off and reminded me of the
>> problem of metadata in general, especially owner and permissions. That
>> problem is important for people keeping track of /etc or $HOME in
>> version control (always a little dangerous because of the effect of a
>> stray "git reset --hard", but never mind). And the last time it came
>> up, I convinced myself that a hook script setting up entries in
>> .gitattributes, .gitmetadata, or .etckeepr with information like "all
>> files are owned by root:root unless otherwise specified" could be a
>> good and not too invasive way to deal with that.
>>
>> Now that you remind me that the mtime of every file is likely to
>> differ from every other file, it is harder to imagine what situation
>> would make this meaningful information that should be stored in the
>> repository and shared with other people. It seems more like something
>> associated to the worktree, which fits more closely with what you are
>> trying to do, anyway.
>
> Yes, indeed.
>
>> Regarding the problem "eclipse metadata is not carried over from one
>> worktree to another", isn't that going to be a problem regardless? In
>> your proposed setup, each time you stash everything and start work on
>> a different branch, the eclipse metadata before would be stashed along
>> with everything else, which doesn't make anything any easier (unless
>> disk space is very scarce or metadata stores the absolute path to the
>> cwd).
>
> Eclipse is a wonderful IDE except for how it makes sharing workspaces
> practically impossible (where "share" means "put in SCM", not "used my
> several developers at the same time"). It's mostly (AFAICT) binary
> data with lots of absolute paths. So it's a pain but it doesn't make
> my scenario all that much more complicated.
>
> The hard part is creating a new branch. Somehow I would need to
> duplicate the state in the parent branch in the child branch. It needs
> to be duplicated because I need to be able to do git stash in the
> child branch *and* the parent branch. Is it possible to do git stash
> pop without losing the stash? Or to "copy" a stash? Otherwise, it's
> probably easier to simply write a separate script to do it. That
> should not be very hard. I would not use git stash at all.
>
> For a new branch, the script would
> 1. in "parent", move workspace (i.e. the root working dir, where .git
> is)  into .git/branches/parent/
> 2. create and jump to "child"
> 3. in "child", copy .git/branches/parent into workspace (so creating a
> new branch is no longer a cheap operation)
>
> For an existing branch, the script would
> 1. in "current-branch", move workspace into .git/branches/current-branch
> 2. jump to "other-branch"
> 3. in "other-branch", move .git/branches/other-branch into workspace
>
> The moves (assuming everything is on the same partition) should make
> switching branches relatively cheap. Not very elegant (quite brute
> force in fact) but it's simple and I think it would work. Or did I
> overlook something? Better/other ideas are certainly welcome. :-)

P.S. I do not mean to imply that I have discounted simply using
different clones or git-new-workdir. Those are still valid options.
The script is probably a bit faster, though.

^ permalink raw reply

* Re: Restoring timestamps (Re: Branches & directories)
From: Hilco Wijbenga @ 2011-08-23  3:06 UTC (permalink / raw)
  To: Jonathan Nieder
  Cc: Kyle Moffett, Michael Witten, Junio C Hamano, Evan Shelhamer,
	Git Mailing List
In-Reply-To: <CAFzf2Xw6=BFsKauYTG-4cw0D_LzLSNb_wqz8dQJ83wJHNQXbdg@mail.gmail.com>

On 22 August 2011 16:21, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hilco Wijbenga wrote:
>
>> You mean an extra dotfile per file in the commit?
>
> Let me step back a moment.
>
> When you mentioned mtime, a switch went off and reminded me of the
> problem of metadata in general, especially owner and permissions. That
> problem is important for people keeping track of /etc or $HOME in
> version control (always a little dangerous because of the effect of a
> stray "git reset --hard", but never mind). And the last time it came
> up, I convinced myself that a hook script setting up entries in
> .gitattributes, .gitmetadata, or .etckeepr with information like "all
> files are owned by root:root unless otherwise specified" could be a
> good and not too invasive way to deal with that.
>
> Now that you remind me that the mtime of every file is likely to
> differ from every other file, it is harder to imagine what situation
> would make this meaningful information that should be stored in the
> repository and shared with other people. It seems more like something
> associated to the worktree, which fits more closely with what you are
> trying to do, anyway.

Yes, indeed.

> Regarding the problem "eclipse metadata is not carried over from one
> worktree to another", isn't that going to be a problem regardless? In
> your proposed setup, each time you stash everything and start work on
> a different branch, the eclipse metadata before would be stashed along
> with everything else, which doesn't make anything any easier (unless
> disk space is very scarce or metadata stores the absolute path to the
> cwd).

Eclipse is a wonderful IDE except for how it makes sharing workspaces
practically impossible (where "share" means "put in SCM", not "used my
several developers at the same time"). It's mostly (AFAICT) binary
data with lots of absolute paths. So it's a pain but it doesn't make
my scenario all that much more complicated.

The hard part is creating a new branch. Somehow I would need to
duplicate the state in the parent branch in the child branch. It needs
to be duplicated because I need to be able to do git stash in the
child branch *and* the parent branch. Is it possible to do git stash
pop without losing the stash? Or to "copy" a stash? Otherwise, it's
probably easier to simply write a separate script to do it. That
should not be very hard. I would not use git stash at all.

For a new branch, the script would
1. in "parent", move workspace (i.e. the root working dir, where .git
is)  into .git/branches/parent/
2. create and jump to "child"
3. in "child", copy .git/branches/parent into workspace (so creating a
new branch is no longer a cheap operation)

For an existing branch, the script would
1. in "current-branch", move workspace into .git/branches/current-branch
2. jump to "other-branch"
3. in "other-branch", move .git/branches/other-branch into workspace

The moves (assuming everything is on the same partition) should make
switching branches relatively cheap. Not very elegant (quite brute
force in fact) but it's simple and I think it would work. Or did I
overlook something? Better/other ideas are certainly welcome. :-)

^ permalink raw reply

* Re: [PATCH] git-svn: Destroy the cache when we fail to read it
From: Jonathan Nieder @ 2011-08-23  2:36 UTC (permalink / raw)
  To: Jason Gross; +Cc: git, Eric Wong
In-Reply-To: <20110823022717.GA4623@elie.gateway.2wire.net>

Jonathan Nieder wrote:

> [This patch teaches "git svn" to invalidate caches when they
>  fail to load, for example because the endianness or size of
>  some type changed, which is common in the perl 5.6 -> 5.8
>  upgrade.]

Erm, actually it showed up in the perl 5.10 -> 5.12 upgrade in Debian,
due to use of -Duse64bitint when compiling the latter and not the
former[1].

To summarize:

 - "$Storable::interwork_56_64bit = 1;" can be used to work around
   that particular brand of breakage;

 - The interwork_* option does not take care of similar cases in which
   a git-svn repository is generated on one machine and read by
   another[2], though.

Hence my happiness at the arrival of Jason's patch.

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=32;bug=618875
[2] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=20;bug=587650

^ permalink raw reply

* Re: [PATCH] git-svn: Destroy the cache when we fail to read it
From: Jonathan Nieder @ 2011-08-23  2:27 UTC (permalink / raw)
  To: Jason Gross; +Cc: git, Eric Wong
In-Reply-To: <1313979422-21286-1-git-send-email-jgross@mit.edu>

(+cc: Eric Wong)
Hi Jason,

Jason Gross wrote:

[This patch teaches "git svn" to invalidate caches when they
 fail to load, for example because the endianness or size of
 some type changed, which is common in the perl 5.6 -> 5.8
 upgrade.]

> http://lists.debian.org/debian-perl/2011/05/msg00023.html and
> http://lists.debian.org/debian-perl/2011/05/msg00026.html).
[...]
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1680,7 +1680,7 @@ use vars qw/$default_repo_id $default_ref_id $_no_metadata $_follow_parent
>              $_use_svnsync_props $no_reuse_existing $_minimize_url
>  	    $_use_log_author $_add_author_from $_localtime/;
>  use Carp qw/croak/;
> -use File::Path qw/mkpath/;
> +use File::Path qw/mkpath rmtree/;
>  use File::Copy qw/copy/;
>  use IPC::Open3;
>  use Memoize;  # core since 5.8.0, Jul 2002
> @@ -3198,28 +3198,41 @@ sub has_no_changes {
>  		$memoized = 1;
>  
>  		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
> -		mkpath([$cache_path]) unless -d $cache_path;
> -
> -		tie my %lookup_svn_merge_cache => 'Memoize::Storable',
[...]
> -		;
> +		my $do_memoization = sub {
> +			mkpath([$cache_path]) unless -d $cache_path;
[...]
> +			;
> +		};
> +
> +		if (not eval {
> +			$do_memoization->();
> +			1;
> +		}) {
> +			my $err = $@ || "Zombie error"; # "Zombie error" to catch clobbered $@ in buggy destructors
> +			die $err unless -d $cache_path;
> +			print STDERR "Discarding cache and trying again ($@)\n";
> +			rmtree([$cache_path]);
> +			$do_memoization->();
> +		}

Thank you thank you thank you.

Okay, time for nitpicks:

 1) Would it be possible to lift this do_memoization() to a toplevel sub?
    I suspect that could make the code a little easier to read.

 2) Is it important to discard the cache for _all_ errors, instead
    of just corruption and "is not compatible" errors?  Rebuilding the
    cache is not cheap, and I am afraid of effects like repeatedly
    discarding the cache only to rebuild it again due to a typo in
    git-svn.perl or an out-of-memory condition.

 3) The line with "Zombie error" is very long --- I guess putting the
    comment on the line before would help.

 4) The series would be clearer imho as a single patch that includes
    both the fix and tests.

Eric, what do you think?

Thanks again, :)
Jonathan

^ permalink raw reply

* Re: [PATCH/RFC 2/2] git-p4: Add complex test case for branch import
From: Pete Wyckoff @ 2011-08-23  2:27 UTC (permalink / raw)
  To: Vitor Antunes; +Cc: git, Tor Arvid Lund
In-Reply-To: <20110822011319.0a013117@fenix.utopia.dhis.org>

vitor.hda@gmail.com wrote on Mon, 22 Aug 2011 01:13 +0100:
> I still need some help on the code part of this RFC patch. I really do
> not like the way it is implemented. Do you see other possible ways of
> achieving the same algorithm?

I think I understand the problem now, and have a small test case
to see what's going wrong.  Your solution is definitely
sub-optimal.  :)  Is there anything that can be done with
--import-marks and --export-marks?

Could you guess at the branch point using p4?  I.e. for each file
in the new branch, grab the change that caused its most recent
revision (the #xx part).  Pick the max of those changes.  In git,
find the commit whith "change = ..." that you just discovered.
That's the parent.

Even this won't be perfect, and I'm not even sure if it's less
icky than diff-tree.

		-- Pete

^ permalink raw reply

* Re: Re* [PATCH 1/2] clone: allow to clone from .git file
From: Nguyen Thai Ngoc Duy @ 2011-08-23  1:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vty99upn9.fsf_-_@alter.siamese.dyndns.org>

On Tue, Aug 23, 2011 at 4:01 AM, Junio C Hamano <gitster@pobox.com> wrote:
> -const char *read_gitfile_gently(const char *path)
> +const char *read_gitfile_gently(const char *path, int *status)
>  {
>        char *buf;
>        char *dir;
> @@ -389,20 +391,39 @@ const char *read_gitfile_gently(const char *path)
>        if (!S_ISREG(st.st_mode))
>                return NULL;

Set *status here too? I assume we need valid *status whenever NULL is returned.

>        fd = open(path, O_RDONLY);
> -       if (fd < 0)
> -               die_errno("Error opening '%s'", path);
> +       if (fd < 0) {
> +               if (!status)
> +                       die_errno("Error opening '%s'", path);
> +               *status = ERROR_READ_GITFILE_CANTOPEN;
> +               return NULL;
> +       }
>        buf = xmalloc(st.st_size + 1);
>        len = read_in_full(fd, buf, st.st_size);
>        close(fd);
-- 
Duy

^ permalink raw reply

* [PATCH 2/2] clone: clone from a repository with relative alternates
From: Junio C Hamano @ 2011-08-23  1:05 UTC (permalink / raw)
  To: git; +Cc: Hui Wang
In-Reply-To: <1314061516-24964-1-git-send-email-gitster@pobox.com>

Cloning from a local repository blindly copies or hardlinks all the files
under objects/ hierarchy. This results in two issues:

 - If the repository cloned has an "objects/info/alternates" file, and the
   command line of clone specifies --reference, the ones specified on the
   command line get overwritten by the copy from the original repository.

 - An entry in a "objects/info/alternates" file can specify the object
   stores it borrows objects from as a path relative to the "objects/"
   directory. When cloning a repository with such an alternates file, if
   the new repository is not sitting next to the original repository, such
   relative paths needs to be adjusted so that they can be used in the new
   repository.

This updates add_to_alternates_file() to take the path to the alternate
object store, including the "/objects" part at the end (earlier, it was
taking the path to $GIT_DIR and was adding "/objects" itself), as it is
technically possible to specify in objects/info/alternates file the path
of a directory whose name does not end with "/objects".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/clone.c  |   52 ++++++++++++++++++++++++++++++++++++++++------------
 sha1_file.c      |    2 +-
 t/t5601-clone.sh |   10 +++++++++-
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index ee5d651..2842707 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -210,8 +210,7 @@ static void strip_trailing_slashes(char *dir)
 static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
 	const char *ref_git;
-	char *ref_git_copy;
-
+	struct strbuf alternate = STRBUF_INIT;
 	struct remote *remote;
 	struct transport *transport;
 	const struct ref *extra;
@@ -224,19 +223,17 @@ static int add_one_reference(struct string_list_item *item, void *cb_data)
 		die(_("reference repository '%s' is not a local directory."),
 		    item->string);
 
-	ref_git_copy = xstrdup(ref_git);
-
-	add_to_alternates_file(ref_git_copy);
+	strbuf_addf(&alternate, "%s/objects", ref_git);
+	add_to_alternates_file(alternate.buf);
+	strbuf_release(&alternate);
 
-	remote = remote_get(ref_git_copy);
-	transport = transport_get(remote, ref_git_copy);
+	remote = remote_get(ref_git);
+	transport = transport_get(remote, ref_git);
 	for (extra = transport_get_remote_refs(transport); extra;
 	     extra = extra->next)
 		add_extra_ref(extra->name, extra->old_sha1, 0);
 
 	transport_disconnect(transport);
-
-	free(ref_git_copy);
 	return 0;
 }
 
@@ -245,7 +242,32 @@ static void setup_reference(void)
 	for_each_string_list(&option_reference, add_one_reference, NULL);
 }
 
-static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
+static void copy_alternates(struct strbuf *src, struct strbuf *dst,
+			    const char *src_repo)
+{
+	/*
+	 * Read from the source objects/info/alternates file
+	 * and copy the entries to corresponding file in the
+	 * destination repository with add_to_alternates_file().
+	 * Both src and dst have "$path/objects/info/alternates".
+	 */
+	FILE *in = fopen(src->buf, "r");
+	struct strbuf line = STRBUF_INIT;
+
+	while (strbuf_getline(&line, in, '\n') != EOF) {
+		char *abs_path, abs_buf[PATH_MAX];
+		if (!line.len || line.buf[0] == '#')
+			continue;
+		abs_path = mkpath("%s/objects/%s", src_repo, line.buf);
+		normalize_path_copy(abs_buf, abs_path);
+		add_to_alternates_file(abs_buf);
+	}
+	strbuf_release(&line);
+	fclose(in);
+}
+
+static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
+				   const char *src_repo, int src_baselen)
 {
 	struct dirent *de;
 	struct stat buf;
@@ -281,7 +303,13 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
 		}
 		if (S_ISDIR(buf.st_mode)) {
 			if (de->d_name[0] != '.')
-				copy_or_link_directory(src, dest);
+				copy_or_link_directory(src, dest,
+						       src_repo, src_baselen);
+			continue;
+		}
+
+		if (!strcmp(src->buf + src_baselen, "/info/alternates")) {
+			copy_alternates(src, dest, src_repo);
 			continue;
 		}
 
@@ -314,7 +342,7 @@ static const struct ref *clone_local(const char *src_repo,
 	else {
 		strbuf_addf(&src, "%s/objects", src_repo);
 		strbuf_addf(&dest, "%s/objects", dest_repo);
-		copy_or_link_directory(&src, &dest);
+		copy_or_link_directory(&src, &dest, src_repo, src.len);
 		strbuf_release(&src);
 		strbuf_release(&dest);
 	}
diff --git a/sha1_file.c b/sha1_file.c
index 064a330..f7c3408 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -380,7 +380,7 @@ void add_to_alternates_file(const char *reference)
 {
 	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
 	int fd = hold_lock_file_for_append(lock, git_path("objects/info/alternates"), LOCK_DIE_ON_ERROR);
-	char *alt = mkpath("%s/objects\n", reference);
+	char *alt = mkpath("%s\n", reference);
 	write_or_die(fd, alt, strlen(alt));
 	if (commit_lock_file(lock))
 		die("could not close alternates file");
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0163ad1..d87214c 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -207,7 +207,7 @@ test_expect_success 'clone separate gitdir where target already exists' '
 	test_must_fail git clone --separate-git-dir realgitdir src dst
 '
 
-test_expect_failure 'clone --reference from original' '
+test_expect_success 'clone --reference from original' '
 	git clone --shared --bare src src-1 &&
 	git clone --bare src src-2 &&
 	git clone --reference=src-2 --bare src-1 target-8 &&
@@ -222,4 +222,12 @@ test_expect_success 'clone with more than one --reference' '
 	grep /src-4/ target-9/.git/objects/info/alternates
 '
 
+test_expect_success 'clone from original with relative alternate' '
+	mkdir nest &&
+	git clone --bare src nest/src-5 &&
+	echo ../../../src/.git/objects >nest/src-5/objects/info/alternates &&
+	git clone --bare nest/src-5 target-10 &&
+	grep /src/\\.git/objects target-10/objects/info/alternates
+'
+
 test_done
-- 
1.7.6.557.gcee42

^ permalink raw reply related

* [PATCH 1/2] clone: allow more than one --reference
From: Junio C Hamano @ 2011-08-23  1:05 UTC (permalink / raw)
  To: git; +Cc: Hui Wang
In-Reply-To: <1314061516-24964-1-git-send-email-gitster@pobox.com>

Also add a test to expose a long-standing bug that is triggered when
cloning with --reference option from a local repository that has its own
alternates. The alternate object stores specified on the command line
are lost, and only alternates copied from the source repository remain.
The bug will be fixed in the next patch.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/clone.c  |   32 ++++++++++++++++++++++++--------
 t/t5601-clone.sh |   15 +++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index f579794..ee5d651 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -39,13 +39,23 @@ static const char * const builtin_clone_usage[] = {
 
 static int option_no_checkout, option_bare, option_mirror;
 static int option_local, option_no_hardlinks, option_shared, option_recursive;
-static char *option_template, *option_reference, *option_depth;
+static char *option_template, *option_depth;
 static char *option_origin = NULL;
 static char *option_branch = NULL;
 static const char *real_git_dir;
 static char *option_upload_pack = "git-upload-pack";
 static int option_verbosity;
 static int option_progress;
+static struct string_list option_reference;
+
+static int opt_parse_reference(const struct option *opt, const char *arg, int unset)
+{
+	struct string_list *option_reference = opt->value;
+	if (!arg)
+		return -1;
+	string_list_append(option_reference, arg);
+	return 0;
+}
 
 static struct option builtin_clone_options[] = {
 	OPT__VERBOSITY(&option_verbosity),
@@ -71,8 +81,8 @@ static struct option builtin_clone_options[] = {
 		    "initialize submodules in the clone"),
 	OPT_STRING(0, "template", &option_template, "template-directory",
 		   "directory from which templates will be used"),
-	OPT_STRING(0, "reference", &option_reference, "repo",
-		   "reference repository"),
+	OPT_CALLBACK(0 , "reference", &option_reference, "repo",
+		     "reference repository", &opt_parse_reference),
 	OPT_STRING('o', "origin", &option_origin, "branch",
 		   "use <branch> instead of 'origin' to track upstream"),
 	OPT_STRING('b', "branch", &option_branch, "branch",
@@ -197,7 +207,7 @@ static void strip_trailing_slashes(char *dir)
 	*end = '\0';
 }
 
-static void setup_reference(const char *repo)
+static int add_one_reference(struct string_list_item *item, void *cb_data)
 {
 	const char *ref_git;
 	char *ref_git_copy;
@@ -206,13 +216,13 @@ static void setup_reference(const char *repo)
 	struct transport *transport;
 	const struct ref *extra;
 
-	ref_git = real_path(option_reference);
+	ref_git = real_path(item->string);
 
 	if (is_directory(mkpath("%s/.git/objects", ref_git)))
 		ref_git = mkpath("%s/.git", ref_git);
 	else if (!is_directory(mkpath("%s/objects", ref_git)))
 		die(_("reference repository '%s' is not a local directory."),
-		    option_reference);
+		    item->string);
 
 	ref_git_copy = xstrdup(ref_git);
 
@@ -227,6 +237,12 @@ static void setup_reference(const char *repo)
 	transport_disconnect(transport);
 
 	free(ref_git_copy);
+	return 0;
+}
+
+static void setup_reference(void)
+{
+	for_each_string_list(&option_reference, add_one_reference, NULL);
 }
 
 static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest)
@@ -521,8 +537,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	git_config_set(key.buf, repo);
 	strbuf_reset(&key);
 
-	if (option_reference)
-		setup_reference(git_dir);
+	if (option_reference.nr)
+		setup_reference();
 
 	fetch_pattern = value.buf;
 	refspec = parse_fetch_refspec(1, &fetch_pattern);
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 151ea53..0163ad1 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -207,4 +207,19 @@ test_expect_success 'clone separate gitdir where target already exists' '
 	test_must_fail git clone --separate-git-dir realgitdir src dst
 '
 
+test_expect_failure 'clone --reference from original' '
+	git clone --shared --bare src src-1 &&
+	git clone --bare src src-2 &&
+	git clone --reference=src-2 --bare src-1 target-8 &&
+	grep /src-2/ target-8/objects/info/alternates
+'
+
+test_expect_success 'clone with more than one --reference' '
+	git clone --bare src src-3 &&
+	git clone --bare src src-4 &&
+	git clone --reference=src-3 --reference=src-4 src target-9 &&
+	grep /src-3/ target-9/.git/objects/info/alternates &&
+	grep /src-4/ target-9/.git/objects/info/alternates
+'
+
 test_done
-- 
1.7.6.557.gcee42

^ permalink raw reply related

* [PATCH 0/2] clone-local fixup
From: Junio C Hamano @ 2011-08-23  1:05 UTC (permalink / raw)
  To: git; +Cc: Hui Wang
In-Reply-To: <7vliulun2a.fsf@alter.siamese.dyndns.org>

So I ended up tackling this myself. I didn't bother moving the
add_to_alternates_file(), though.

Junio C Hamano (2):
  clone: allow more than one --reference
  clone: clone from a repository with relative alternates

 builtin/clone.c  |   82 +++++++++++++++++++++++++++++++++++++++++------------
 sha1_file.c      |    2 +-
 t/t5601-clone.sh |   23 +++++++++++++++
 3 files changed, 87 insertions(+), 20 deletions(-)

-- 
1.7.6.557.gcee42

^ permalink raw reply

* [PATCH] Mark http-fetch without -a as deprecated
From: Ben Walton @ 2011-08-23  0:51 UTC (permalink / raw)
  To: git, gitster; +Cc: Ben Walton

As the use of http-fetch without -a can create an object store that is
invalid to the point where it cannot even be fsck'd, mark it as
deprecated.  A future release should change the default and then
remove the option entirely.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 http-fetch.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/http-fetch.c b/http-fetch.c
index 3af4c71..003ddb7 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -56,6 +56,9 @@ int main(int argc, const char **argv)
 		commits = 1;
 	}
 
+	if (get_all == 0)
+	  warning("http-fetch: use without -a is deprecated; -a will become the default in a future release");
+
 	if (argv[arg])
 		str_end_url_with_slash(argv[arg], &url);
 
-- 
1.7.4.1

^ permalink raw reply related

* [PATCH] Documentation: Grammar correction, wording fixes and cleanup
From: Ben Walton @ 2011-08-23  0:48 UTC (permalink / raw)
  To: gitster; +Cc: git, Ben Walton
In-Reply-To: <7vk4a5wc67.fsf@alter.siamese.dyndns.org>

Correct a few grammar issues in gitrepository-layout.txt and also
rewords a few sections for clarity.

Remove references to using http-fetch without -a to create a broken
repository.

Mark a few areas of the repository structure as legacy.

Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
---
 Documentation/gitrepository-layout.txt |   60 +++++++++++++++-----------------
 1 files changed, 28 insertions(+), 32 deletions(-)

diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index eb3d040..5023ea9 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -23,32 +23,25 @@ objects::
 	Object store associated with this repository.  Usually
 	an object store is self sufficient (i.e. all the objects
 	that are referred to by an object found in it are also
-	found in it), but there are couple of ways to violate
-	it.
+	found in it), but there are a few ways to violate it.
 +
-. You could populate the repository by running a commit walker
-without `-a` option.  Depending on which options are given, you
-could have only commit objects without associated blobs and
-trees this way, for example.  A repository with this kind of
-incomplete object store is not suitable to be published to the
-outside world but sometimes useful for private repository.
-. You also could have an incomplete but locally usable repository
-by cloning shallowly.  See linkgit:git-clone[1].
-. You can be using `objects/info/alternates` mechanism, or
-`$GIT_ALTERNATE_OBJECT_DIRECTORIES` mechanism to 'borrow'
+. You could have an incomplete but locally usable repository
+by creating a shallow clone.  See linkgit:git-clone[1].
+. You could be using the `objects/info/alternates` or
+`$GIT_ALTERNATE_OBJECT_DIRECTORIES` mechanisms to 'borrow'
 objects from other object stores.  A repository with this kind
 of incomplete object store is not suitable to be published for
-use with dumb transports but otherwise is OK as long as
-`objects/info/alternates` points at the right object stores
-it borrows from.
+use with dumb transports but is otherwise OK as long as
+`objects/info/alternates` points to object stores containing the
+missing objects.
 
 objects/[0-9a-f][0-9a-f]::
-	Traditionally, each object is stored in its own file.
-	They are split into 256 subdirectories using the first
-	two letters from its object name to keep the number of
-	directory entries `objects` directory itself needs to
-	hold.  Objects found here are often called 'unpacked'
-	(or 'loose') objects.
+	A newly created object is stored in its own file.
+	The objects are splayed over 256 subdirectories using
+	the first two characters of the sha1 object name to
+	keep the number of directory entries in `objects`
+	itself to a manageable number. Objects found
+	here are often called 'unpacked' (or 'loose') objects.
 
 objects/pack::
 	Packs (files that store many object in compressed form,
@@ -85,7 +78,7 @@ objects/info/http-alternates::
 
 refs::
 	References are stored in subdirectories of this
-	directory.  The 'git prune' command knows to keep
+	directory.  The 'git prune' command knows to preserve
 	objects reachable from refs found in this directory and
 	its subdirectories.
 
@@ -119,16 +112,17 @@ HEAD::
 +
 HEAD can also record a specific commit directly, instead of
 being a symref to point at the current branch.  Such a state
-is often called 'detached HEAD', and almost all commands work
-identically as normal.  See linkgit:git-checkout[1] for
-details.
+is often called 'detached HEAD.'  See linkgit:git-checkout[1]
+for details.
 
 branches::
 	A slightly deprecated way to store shorthands to be used
-	to specify URL to 'git fetch', 'git pull' and 'git push'
-	commands is to store a file in `branches/<name>` and
-	give 'name' to these commands in place of 'repository'
-	argument.
+	to specify a URL to 'git fetch', 'git pull' and 'git push'.
+	A file can be stored as `branches/<name>` and then
+	'name' can be given to these commands in place of
+	'repository' argument.  See the REMOTES section in
+	linkgit:git-fetch[1] for details.  This mechanism is legacy
+	and not likely to be found in modern repositories.
 
 hooks::
 	Hooks are customization scripts used by various git
@@ -173,9 +167,11 @@ info/exclude::
 	at it.  See also: linkgit:gitignore[5].
 
 remotes::
-	Stores shorthands to be used to give URL and default
-	refnames to interact with remote repository to
-	'git fetch', 'git pull' and 'git push' commands.
+	Stores shorthands for URL and default refnames for use
+	when interacting with remote repositories via 'git fetch',
+	'git pull' and 'git push' commands.  See the REMOTES section
+	in linkgit:git-fetch[1] for details.  This mechanism is legacy
+	and not likely to be found in modern repositories.
 
 logs::
 	Records of changes made to refs are stored in this
-- 
1.7.4.1

^ permalink raw reply related

* Re: [PATCH] Grammar and wording fixes in gitrepository-layout
From: Ben Walton @ 2011-08-23  0:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vk4a5wc67.fsf@alter.siamese.dyndns.org>

Excerpts from Junio C Hamano's message of Mon Aug 22 14:09:36 -0400 2011:

Hi Junio,

> Perhaps we should deprecate http-fetch without -a and drop this item
> from the list?

I don't know much about http-fetch as I've never touched it directly.
It doesn't sound like a good idea to allow creating a broken object
store though.  I'll send a patch shortly that adds the deprecation
warning if -a isn't given.

> > +. You could be using the `objects/info/alternates` or
> > +`$GIT_ALTERNATE_OBJECT_DIRECTORIES` mechanisms to 'borrow'
> >  objects from other object stores.  A repository with this kind
> >  of incomplete object store is not suitable to be published for
> > -use with dumb transports but otherwise is OK as long as
> > -`objects/info/alternates` points at the right object stores
> > -it borrows from.
> > +use with dumb transports but is otherwise OK as long as
> > +`objects/info/alternates` points at the right object stores.
> 
> The last three words in the original are meant to clarify and define
> what "the right object stores" are. Was there a compelling reason to
> drop them?

The only reason I dropped them is that I didn't feel they were helping
to clarify the meaning.  The wording was a bit awkward and I thought
'it borrows from' was part the problem.  I see what you're saying
though and think that the following wording is better:

use with dumb transports but is otherwise OK as long as
`objects/info/alternates` points to object stores containing the
missing objects.

> >  objects/[0-9a-f][0-9a-f]::
> >      Traditionally, each object is stored in its own file.
> 
> I would suggest further rewording this to something like:
> 
>     A newly created object is stored in its own file.

Agreed.  This is much better.

> > @@ -120,15 +118,15 @@ HEAD::
> >  HEAD can also record a specific commit directly, instead of
> >  being a symref to point at the current branch.  Such a state
> >  is often called 'detached HEAD', and almost all commands work
> > -identically as normal.  See linkgit:git-checkout[1] for
> > +as they normally would.  See linkgit:git-checkout[1] for
> >  details.
> 
> We may want to reword the sentence that begins with "almost all commands"
> further. In the early days after detached HEAD support was introduced, we
> may have left cases where the result was _undefined_ for commands that
> would not make sense unless you are on a branch, but by now what we have
> should behave sensibly by either erroring out when the operation does not
> make sense unless you are on a real branch, or doing something
> useful.

Ok, how about a full stop after 'detached HEAD' and then the link to
git-checkout?

> >  branches::
> >      A slightly deprecated way to store shorthands to be used
> > -    to specify URL to 'git fetch', 'git pull' and 'git push'
> > -    commands is to store a file in `branches/<name>` and
> > -    give 'name' to these commands in place of 'repository'
> > -    argument.
> > +    to specify a URL to 'git fetch', 'git pull' and 'git push'.
> > +    A file can be stored as `branches/<name>` and then
> > +    'name' can be givent to these commands in place of
> 
> s/givent to/given to/
> 
> > +    'repository' argument.
> 
> We would at least need "See linkgit:..." to say what is expected in this
> file and how it is used (the information is in urls-remotes.txt but that
> is not a top-level file, so it needs to refer to git-fetch and git-push
> instead).

Noted.  The updated patch will address this.

> > @@ -173,9 +171,9 @@ info/exclude::
> >      at it.  See also: linkgit:gitignore[5].
> >  
> >  remotes::
> > -    Stores shorthands to be used to give URL and default
> > -    refnames to interact with remote repository to
> > -    'git fetch', 'git pull' and 'git push' commands.
> > +    Stores shorthands for URL and default refnames for use
> > +    when interacting with remote repositories via 'git fetch',
> > +    'git pull' and 'git push' commands.
> 
> Likewise.
> 
> Also I would personally consider "branches" and "remotes" both "slightly
> deprecated". "git init", "git clone", and "git remote" stopped populating
> these long time ago.

Ok, I've added the note about this being legacy to both of the
sections in question.

Updated patch to follow shortly.

Thanks
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302

^ permalink raw reply

* [RFH] lifetime rule for url parameter to transport_get()?
From: Junio C Hamano @ 2011-08-23  0:34 UTC (permalink / raw)
  To: Daniel Barkalow, Shawn O. Pearce; +Cc: git

Does anybody remember why we use a copied string of "ref_git_copy" in
builtin/clone.c::setup_reference()?

	ref_git = real_path(option_reference);
	...
	ref_git_copy = xstrdup(ref_git);
	add_to_alternates_file(ref_git_copy);
	remote = remote_get(ref_git_copy);
	transport = transport_get(remote, ref_git_copy);
	for (extra = transport_get_remote_refs(transport); extra;
	     extra = extra->next)
		add_extra_ref(extra->name, extra->old_sha1, 0);
	transport_disconnect(transport);
	free(ref_git_copy);

The three functions add_to_alternates_file(), remote_get(), and
transport_get() all get "const char *" so I do not think the copy was done
to avoid "option_reference" from getting clobbered by these functions. The
only thing I can think of is that transport_get() does this:

    struct transport *transport_get(struct remote *remote, const char *url)
    {
            const char *helper;
            struct transport *ret = xcalloc(1, sizeof(*ret));

            ...
            if (!url && remote->url)
                    url = remote->url[0];
            ret->url = url;
	    ...
	    return ret;
    }

holding onto "url" without making a copy for its own use. But then freeing
that copy by the caller after calling transport_disconnect() does not make
much sense to me---we could have just gave it the original option_reference,
have transport use it while it runs ls-remote equivalent, and then called
transport_disconnect(), without using any extra copy.

What I am missing?

^ permalink raw reply

* Re: [PATCH] gitweb: highlight: strip non-printable characters via col(1)
From: Junio C Hamano @ 2011-08-22 23:21 UTC (permalink / raw)
  To: Christopher M. Fuhrman; +Cc: git, jnareb, cwilson, sylvain
In-Reply-To: <1314053923-13122-1-git-send-email-cfuhrman@panix.com>

"Christopher M. Fuhrman" <cfuhrman@panix.com> writes:

> Strip non-printable control-characters by piping the output produced
> by git-cat-file(1) to col(1) as follows:
>
>   git cat-file blob deadbeef314159 | col -bx | highlight <args>
>
> Note usage of the '-x' option which tells col(1) to output multiple
> spaces instead of tabs.

Are all implementations of col known to correctly handle bytes with their
highest bit on, without mistaking them with unknown control sequences?
Has the code updated by your patch been tested with non-ASCII payload, at
least with UTF-8 outside US-ASCII?

In what locale does the code updated by your patch run under, and would
the use of "col" affected by the choice of the locale in a negative way?

For example, here is what I get on my box:

    $ LANG=C LC_ALL=C col -bx <t/t3902-quoted.sh ; echo $?
    col: Invalid or incomplete multibyte or wide character
    1

that makes me ask you these questions.

> I haven't gotten any responses to my patch for a while, so I am now
> submitting this for general inclusion into git.

Unfortunately, no news is not good news around here, and that is why I
am asking you the above questions.

Thanks.

^ permalink raw reply

* Re: Restoring timestamps (Re: Branches & directories)
From: Jonathan Nieder @ 2011-08-22 23:21 UTC (permalink / raw)
  To: Hilco Wijbenga
  Cc: Kyle Moffett, Michael Witten, Junio C Hamano, Evan Shelhamer,
	Git Mailing List
In-Reply-To: <CAE1pOi1+nnpnHAuhYsXcfFNUroW0JcDQKLu6D7YNrUwJg0tXPw@mail.gmail.com>

Hilco Wijbenga wrote:

> You mean an extra dotfile per file in the commit?

Let me step back a moment.

When you mentioned mtime, a switch went off and reminded me of the
problem of metadata in general, especially owner and permissions. That
problem is important for people keeping track of /etc or $HOME in
version control (always a little dangerous because of the effect of a
stray "git reset --hard", but never mind). And the last time it came
up, I convinced myself that a hook script setting up entries in
.gitattributes, .gitmetadata, or .etckeepr with information like "all
files are owned by root:root unless otherwise specified" could be a
good and not too invasive way to deal with that.

Now that you remind me that the mtime of every file is likely to
differ from every other file, it is harder to imagine what situation
would make this meaningful information that should be stored in the
repository and shared with other people. It seems more like something
associated to the worktree, which fits more closely with what you are
trying to do, anyway.

Regarding the problem "eclipse metadata is not carried over from one
worktree to another", isn't that going to be a problem regardless? In
your proposed setup, each time you stash everything and start work on
a different branch, the eclipse metadata before would be stashed along
with everything else, which doesn't make anything any easier (unless
disk space is very scarce or metadata stores the absolute path to the
cwd).

Sorry for the lack of clarity.
Jonathan

^ 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