Specify GIT_DIR when running git-config in a mirror.

Specify GIT_DIR when running git-config in a mirror.

BUG=497894
R=hinoka@chromium.org,agable@chromium.org

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=295587

[agable, 2015-06-08 19:52:33]

Doesn't GIT_DIR need to point directly at a .git directory, not at a checkout
containing one?

[hinoka, 2015-06-08 19:54:04]

I think this removes all other environment variables ($HOME, $PATH, etc) from
the execution environment.

[szager1, 2015-06-08 20:26:43]

On 2015/06/08 19:54:04, hinoka wrote:
> I think this removes all other environment variables ($HOME, $PATH, etc) from
> the execution environment.

If you look at the definition of RunGit(), it already adds an env parameter. 
This just adds one more entry to that.

[szager1, 2015-06-08 20:27:22]

On 2015/06/08 19:52:33, agable wrote:
> Doesn't GIT_DIR need to point directly at a .git directory, not at a checkout
> containing one?

Either a .git directory, or the root of a bare repo.

[szager1, 2015-06-08 20:28:37]

On 2015/06/08 20:27:22, szager1 wrote:
> On 2015/06/08 19:52:33, agable wrote:
> > Doesn't GIT_DIR need to point directly at a .git directory, not at a
checkout
> > containing one?
> 
> Either a .git directory, or the root of a bare repo.

To clarify: config() is only ever called to set up a mirror, which is a bare git
repo.  So in every case, GIT_DIR=cwd is correct.

[chromium-reviews, 2015-06-08 20:34:13]

RunGit() does:
env = kwargs.get('env') or kwargs.setdefault('env', os.environ.copy())

if you don't specify an env in kwargs it's set to os.environ.copy()

If you do specify an env then it takes it directly from kwargs and
os.environ.copy() never gets called.

On Mon, Jun 8, 2015 at 1:28 PM, <szager@chromium.org> wrote:

> On 2015/06/08 20:27:22, szager1 wrote:
>
>> On 2015/06/08 19:52:33, agable wrote:
>> > Doesn't GIT_DIR need to point directly at a .git directory, not at a
>>
> checkout
>
>> > containing one?
>>
>
>  Either a .git directory, or the root of a bare repo.
>>
>
> To clarify: config() is only ever called to set up a mirror, which is a
> bare git
> repo.  So in every case, GIT_DIR=cwd is correct.
>
> https://codereview.chromium.org/1167193002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[szager1, 2015-06-08 20:43:44]

On 2015/06/08 20:34:13, chromium-reviews wrote:
> RunGit() does:
> env = kwargs.get('env') or kwargs.setdefault('env', os.environ.copy())
> 
> if you don't specify an env in kwargs it's set to os.environ.copy()
> 
> If you do specify an env then it takes it directly from kwargs and
> os.environ.copy() never gets called.
> 
> On Mon, Jun 8, 2015 at 1:28 PM, <mailto:szager@chromium.org> wrote:
> 
> > On 2015/06/08 20:27:22, szager1 wrote:
> >
> >> On 2015/06/08 19:52:33, agable wrote:
> >> > Doesn't GIT_DIR need to point directly at a .git directory, not at a
> >>
> > checkout
> >
> >> > containing one?
> >>
> >
> >  Either a .git directory, or the root of a bare repo.
> >>
> >
> > To clarify: config() is only ever called to set up a mirror, which is a
> > bare git
> > repo.  So in every case, GIT_DIR=cwd is correct.
> >
> > https://codereview.chromium.org/1167193002/
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Ah, indeed; fixed now, thanks.

[szager1, 2015-06-09 16:59:31]

ping

[hinoka, 2015-06-09 18:10:35]

lgtm + nit

https://codereview.chromium.org/1167193002/diff/40001/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1167193002/diff/40001/git_cache.py#newcode241
git_cache.py:241: cwd=cwd, env=env)
that looks like a tab

[szager1, 2015-06-09 20:32:52]

https://codereview.chromium.org/1167193002/diff/40001/git_cache.py
File git_cache.py (right):

https://codereview.chromium.org/1167193002/diff/40001/git_cache.py#newcode241
git_cache.py:241: cwd=cwd, env=env)
On 2015/06/09 18:10:34, hinoka wrote:
> that looks like a tab

Done.

[szager1, 2015-06-09 20:32:56]

The CQ bit was checked by szager@chromium.org

[szager1, 2015-06-09 20:32:56]

The patchset sent to the CQ was uploaded after l-g-t-m from hinoka@chromium.org
Link to the patchset: https://codereview.chromium.org/1167193002/#ps60001
(title: "fix whitespace")

[commit-bot: I haz the power, 2015-06-09 20:33:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1167193002/60001

[commit-bot: I haz the power, 2015-06-09 20:35:50]

Committed patchset #4 (id:60001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=295587

[hinoka, 2015-06-10 19:18:44]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1179593002/ by hinoka@chromium.org.

The reason for reverting is: Potentially broke the NaCl toolchain builders:
crbug.com/498942

Speculative revert..