Re: What's cooking in git.git (Apr 2017, #01; Tue, 11)
On Tue, Apr 11, 2017 at 03:47:13PM -0700, Jonathan Nieder wrote:
> Junio C Hamano wrote:
>
> > * jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
> > (merged to 'next' on 2017-02-27 at 7373a1b73d)
> > + setup_git_env: avoid blind fall-back to ".git"
> >
> > This is the endgame of the topic to avoid blindly falling back to
> > ".git" when the setup sequence said we are _not_ in Git repository.
> > A corner case that happens to work right now may be broken by a
> > call to die("BUG").
>
> There's one caller we missed, in "git apply" when you apply a binary
> patch outside any repository.
Good catch. This might also trigger with "apply --3way", but I didn't
check. Your patch would presumably fix that, too.
> diff --git a/sha1_file.c b/sha1_file.c
> index 71063890ff..bf1ff2ef77 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -3481,6 +3481,8 @@ int has_sha1_file_with_flags(const unsigned char *sha1,
> int flags)
> {
> struct pack_entry e;
>
> + if (!startup_info->have_repository)
> + return 0;
I added have_git_dir(), which catches some other cases (e.g., we have a
$GIT_DIR but just haven't entered the repo _yet_). TBH, I am not
entirely sure when it would be needed, and when checking have_repository
is enough. I added it to make some cases with early config-reading work,
but now that we have Dscho's discover-the-real-repo, I think it would
supersede that use anyway.
So...maybe you'd want it here and maybe not?
-Peff
Re: What's cooking in git.git (Apr 2017, #01; Tue, 11)
Junio C Hamano wrote:
> * jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
> (merged to 'next' on 2017-02-27 at 7373a1b73d)
> + setup_git_env: avoid blind fall-back to ".git"
>
> This is the endgame of the topic to avoid blindly falling back to
> ".git" when the setup sequence said we are _not_ in Git repository.
> A corner case that happens to work right now may be broken by a
> call to die("BUG").
There's one caller we missed, in "git apply" when you apply a binary
patch outside any repository.
The patch below fixes it but doesn't have tests. I'll try to send
out a patch with tests later today.
It's probably also worth making the die("BUG") compile-time
configurable so distros have an easy way out if any similar bugs are
lurking undiscovered.
Thanks,
Jonathan
-- >8 --
Subject: has_sha1_file: don't bother if we are not in a repository
Most callers to this function already require that they are in a
git repository, but there is an exception: "git apply" uses
has_sha1_file to avoid work if the result of applying a binary
patch is already present in the repository. When run outside any
repository, this produces an error:
fatal: BUG: setup_git_env called without repository
Signed-off-by: Jonathan Nieder
---
diff --git a/sha1_file.c b/sha1_file.c
index 71063890ff..bf1ff2ef77 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3481,6 +3481,8 @@ int has_sha1_file_with_flags(const unsigned char *sha1,
int flags)
{
struct pack_entry e;
+ if (!startup_info->have_repository)
+ return 0;
if (find_pack_entry(sha1, ))
return 1;
if (has_loose_object(sha1))
Re: What's cooking in git.git (Apr 2017, #01; Tue, 11)
> * cc/split-index-config (2017-03-30) 1 commit > - read-cache: avoid using git_path() in freshen_shared_index() > > The split-index code configuration code used an unsafe git_path() > function without copying its result out. > > Needs to be explained better. > The code looked OK, though. In: http://public-inbox.org/git/xmqq7f367o0s@gitster.mtv.corp.google.com/ it looked like you were ok with the explanation or you actually wanted it to be shorter. So should I send a shorter one? Or should I add explanation about why git_path() is unsafe?
