Re: [StGIT PATCH 2/2] Don't touch ref files manually

From: Karl Hasselström <kha@treskal.com>
Date: Aug 21, 2007, 15:58 UTC
Message-ID: <20070821155845.GA16221@diana.vm.bytemark.co.uk>
In-Reply-To: <b0943d9e0708210623h112faa42p97bba06bc9fab774@mail.gmail.com>

On 2007-08-21 14:23:29 +0100, Catalin Marinas wrote:

Show 21 quoted lines

> On 10/08/07, Karl Hasselström <kha@treskal.com> wrote:
>
> > Messing with files manually doesn't work if the refs are packed.
> > The officially preferred way is to use git-update-ref,
> > git-show-ref, et.al. So do that.
>
> Thanks for this patch. I'll apply it but I have some questions
> below.
>
> > @@ -294,14 +301,16 @@ def set_head_file(ref):
> >      # head cache flushing is needed since we might have a different value
> >      # in the new head
> >      __clear_head_cache()
> > -    if __run('git-symbolic-ref HEAD',
> > -             [os.path.join('refs', 'heads', ref)]) != 0:
> > +    if __run('git-symbolic-ref HEAD', ['refs/heads/%s' % ref]) != 0:
> >          raise GitException, 'Could not set head to "%s"' % ref
>
> Why replacing os.path.join with a single string? I think the former
> is more portable if, in the future, someone will try to run StGIT on
> Windows (and GIT would be compiled directly, without cygwin).

Yes, that's the case iff it is a pathname in the file system. But I think forward slashes are used even on Windows in this case -- otherwise too mmuch configuration would have to be changed. This is what I _think_, however; I haven't actually checked.

Show 13 quoted lines

> > diff --git a/t/t1000-branch-create.sh b/t/t1000-branch-create.sh
> > index cca5504..e920e93 100755
> > --- a/t/t1000-branch-create.sh
> > +++ b/t/t1000-branch-create.sh
> > @@ -13,26 +13,9 @@ Exercises the "stg branch" commands.
> >  stg init
> >
> >  test_expect_success \
> > -    'Create a spurious refs/patches/ entry' '
> > -    find .git -name foo | xargs rm -rf &&
> > -    touch .git/refs/patches/foo
>
> I haven't tried this test but is there any problem if we keep them?

Yes.

This test started to fail after my change. I don't remember exactly, but I think it was git-symbolic-ref actually reading the file, finding no ref there, and ignoring it. So this empty file didn't mess up the machinery anymore, and since this isn't a common usecase anyway, and since this change is to rely on git for all the low-level ref manipulation, I just deleted the test.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle