From: Jeff Hostetler <git@jeffhostetler.com>
Date: Mon, 29 Apr 2019 19:03:19 GMT
Subject: Re: [PATCH v4 06/10] trace2: use system/global config for default trace2 settings
Message-ID: <465993c8-0eb2-d9fa-5d22-b8b584b3de15@jeffhostetler.com>
In-Reply-To: <20190427134316.GE8695@szeder.dev>



On 4/27/2019 9:43 AM, SZEDER Gábor wrote:
> On Mon, Apr 15, 2019 at 01:39:47PM -0700, Jeff Hostetler via GitGitGadget wrote:
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> Teach git to read the system and global config files for
>> default Trace2 settings.  This allows system-wide Trace2 settings to
>> be installed and inherited to make it easier to manage a collection of
>> systems.
>>
>> The original GIT_TR2* environment variables are loaded afterwards and
>> can be used to override the system settings.
>>
>> Only the system and global config files are used.  Repo and worktree
>> local config files are ignored.  Likewise, the "-c" command line
>> arguments are also ignored.  These limits are for performance reasons.
>>
>> (1) For users not using Trace2, there should be minimal overhead to
>> detect that Trace2 is not enabled.  In particular, Trace2 should not
>> allocate lots of otherwise unused data strucutres.
>>
>> (2) For accurate performance measurements, Trace2 should be initialized
>> as early in the git process as possible, and before most of the normal
>> git process initialization (which involves discovering the .git directory
>> and reading a hierarchy of config files).
> 
> Reading the configuration that early causes unexpected and undesired
> behavior change:
> 
>    $ sudo chmod a-rwx /usr/local/etc/gitconfig
>    $ ./BUILDS/v2.21.0/bin/git
>    usage: git [--version] [--help] [-C <path>] [-c <name>=<value>]
>    <... snip rest of usage ...>
>    $ strace ./BUILDS/v2.21.0/bin/git 2>&1 |grep config -c
>    0
>    $ ./git
>    fatal: unable to access '/usr/local/etc/gitconfig': Permission denied
>    $ ./git --version
>    fatal: unable to access '/usr/local/etc/gitconfig': Permission denied
> 
> I think at least 'git', 'git --help', and 'git --version' should Just
> Work, no matter what.
> 
> 
> This breaks the 32 bit Linux build job on Travis CI, because:
> 
>    - In the 32 bit Docker image we change UID from root to regular user
>      while preserving the environment, including $HOME.
>      
>    - Since $HOME is the default build prefix, Git will look for the
>      system-wide configuration under '/root/etc/gitconfig', which fails
>      as a regular user.
> 
>    - Our test harness checks early (i.e. earlier than setting
>      GIT_CONFIG_NOSYSTEM=1) whether Git has been built successfully by
>      attempting to run '$GIT_BUILD_DIR}/git', which fails because of
>      the inaccessible system-wide config file, and in turn the harness
>      assumes that Git hasn't been built and aborts.
> 
>    https://travis-ci.org/git/git/jobs/524403682#L1258
> 

It appears that config.c:do_git_config_sequence() passes flags = 0
to access_or_die() rather than ACCESS_EACCES_OK as it does for the
other scopes.  This causes the fatal error, rather than ignoring the
problem.

Reasons for this were discussed in:
     4698c8feb1 config: allow inaccessible configuration under $HOME

I'll push a fix shortly.

Thanks
Jeff