Re: [PATCH 2/3] var: add attributes files locations
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Jun 22, 2023, 21:18 UTC
- Message-ID
- <CAPig+cQ=B7M6nPYx-+gqtsJeSuiqKeQhCfiWmyfjiGXsVnFbTw@mail.gmail.com>
- In-Reply-To
- <20230622195059.320593-3-sandals@crustytoothpaste.net>
On Thu, Jun 22, 2023 at 4:06 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:
Show 33 quoted lines
> Currently, there are some programs which would like to read and parse > the gitattributes files at the global or system levels. However, it's > not always obvious where these files live, especially for the system > file, which may have been hard-coded at compile time or computed > dynamically based on the runtime prefix. > > It's not reasonable to expect all callers of Git to intuitively know > where the Git distributor or user has configured these locations to > be, so add some entries to allow us to determine their location. Honor > the GIT_ATTR_NOSYSTEM environment variable if one is specified. Expose > the accessor functions in a way that we can reuse them from within the > var code. > > In order to make our paths consistent on Windows and also use the same > form as paths use in "git rev-parse", let's normalize the path before we > return it. This results in Windows-style paths that use slashes, which > is convenient for making our tests function in a consistent way across > platforms. Note that this requires that some of our values be freed, so > let's add a flag about whether the value needs to be freed and use it > accordingly. > > Signed-off-by: brian m. carlson <bk2204@github.com> > --- > diff --git a/t/t0007-git-var.sh b/t/t0007-git-var.sh > @@ -159,6 +159,26 @@ test_expect_success MINGW 'GIT_SHELL_PATH points to a suitable shell' ' > +test_expect_success 'GIT_ATTR_SYSTEM points to the correct location' ' > + test_must_fail env GIT_ATTR_NOSYSTEM=1 git var GIT_ATTR_SYSTEM && > + ( > + sane_unset GIT_ATTR_NOSYSTEM && > + git var GIT_ATTR_SYSTEM >path && > + test "$(cat path)" != "" > + ) > +'
Same observation as in patch [1/3]: no need for a temporary file:
p=$(git var GIT_ATTR_SYSTEM) &&
test -n "$p"> +test_expect_success 'GIT_ATTR_GLOBAL points to the correct location' ' > + TRASHDIR="$(test-tool path-utils normalize_path_copy "$(pwd)")" &&
The reference to $(pwd) is unnecessary, thus potentially confusing. Simpler:
TRASHDIR="$(test-tool path-utils normalize_path_copy .)" &&
> + XDG_CONFIG_HOME="$TRASHDIR/.config" git var GIT_ATTR_GLOBAL >path && > + test "$(cat path)" = "$TRASHDIR/.config/git/attributes" &&
Same observation about unnecessary temporary file:
p=$(XDG_CONFIG_HOME="$TRASHDIR/.config" git var GIT_ATTR_GLOBAL) &&
test "$p" = "$TRASHDIR/.config/git/attributes" &&Show 5 quoted lines
> + ( > + sane_unset XDG_CONFIG_HOME && > + HOME="$TRASHDIR" git var GIT_ATTR_GLOBAL >path && > + test "$(cat path)" = "$TRASHDIR/.config/git/attributes" > + )
And here too.
> +'