git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: index-pack outside of repository?
Date: Thu, 15 Dec 2016 15:40:00 -0500	[thread overview]
Message-ID: <20161215204000.avlcfaqjwstkptu2@sigill.intra.peff.net> (raw)

Running git on 'next', you can trigger a BUG:

  $ cd /some/repo
  $ git pack-objects --all --stdout </dev/null >/tmp/foo.pack
  $ cd /not-a-git-repo
  $ git index-pack --stdin </tmp/foo.pack
  fatal: BUG: setup_git_env called without repository

This obviously comes from my b1ef400eec (setup_git_env: avoid blind
fall-back to ".git", 2016-10-20). What's going on is that index-pack
uses RUN_SETUP_GENTLY, but never actually handles the out-of-repo case.
When we use the internal git_dir to make "objects/pack/pack-xxx.pack",
it barfs.

In older versions of git will just blindly write into
".git/objects/pack", even though there's no repository there.

So I think complaining to the user is the right thing to do here. I
started to write a patch to have index-pack notice when it needs a repo
and doesn't have one, but the logic is actually a bit unclear.  Do we
need to complain early _just_ when --stdin is specified, or does that
miss somes cases?  Likewise, are there cases where --stdin can operate
without a repo? I couldn't think of any.

I'm actually wondering if the way it calls die() in 'next' is a pretty
reasonable way for things to work in general. It happens when we lazily
try to ask for the repository directory. So we don't have to replicate
logic to say "are we going to need a repo"; at the moment we need it, we
notice we don't have it and die. The only problem is that it says "BUG"
and not "this operation must be run in a git repository".

That strategy _might_ be a problem for some programs, which would want
to notice the issue early before doing work. But it seems like a
reasonable outcome for index-pack. Thoughts?

-Peff

             reply	other threads:[~2016-12-15 20:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-15 20:40 Jeff King [this message]
2016-12-16  0:13 ` index-pack outside of repository? Junio C Hamano
2016-12-16  1:37   ` Jeff King
2016-12-16  2:29     ` Jeff King
2016-12-16  2:30       ` [PATCH 1/3] t5000: extract nongit function to test-lib-functions.sh Jeff King
2016-12-16  2:30       ` [PATCH 2/3] index-pack: complain when --stdin is used outside of a repo Jeff King
2016-12-16  2:31       ` [PATCH 3/3] t: use nongit() function where applicable Jeff King
2016-12-16 17:52       ` index-pack outside of repository? Junio C Hamano
2016-12-16 17:59         ` Junio C Hamano
2016-12-16 18:01           ` Junio C Hamano
2016-12-16 21:43             ` Jeff King
2016-12-16 21:55               ` Junio C Hamano
2016-12-16 18:54   ` Junio C Hamano
2016-12-16 21:44     ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161215204000.avlcfaqjwstkptu2@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).