All of lore.kernel.org
 help / color / mirror / Atom feed
From: Todd Zullinger <tmz@pobox.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 06/18] version-gen: remove redundant check
Date: Fri, 14 Apr 2023 15:41:55 -0400	[thread overview]
Message-ID: <ZDmsg0njhtv_qM9a@pobox.com> (raw)
In-Reply-To: <6439aa5b2c776_6058729489@chronos.notmuch>

Felipe Contreras wrote:
> Todd Zullinger wrote:
>> If I have a git repo, say ~/fedora/git which contains the
>> fedora packaging (spec file, etc.) and extract a git archive
>> in this directory, the describe will now pick up the data
>> from the parent git directory, won't it?
>> 
>>     $ git -C ~/fedora clone https://src.fedoraproject.org/rpms/git.git
>>     $ cd ~/fedora/git
>>     $ git -C ~/upstream/git archive --format=tar --prefix=git/ HEAD | tar xf -
>>     $ cd git
>>     $ make GIT-VERSION-GEN
>>     $ cat GIT-VERSION-FILE 
>>     GIT_VERSION = 
> 
> I don't think this is a realistic use-case, but supposing it is, what would be
> the desired outcome in this case?
> 
>  GIT_VERSION = 2.40.GIT
> 
> ?

That's what we get before this change.  I don't have an
opinion on whether that could/should change, but if it does,
it should be deliberate I think.

>> The version file in the tarballs prevents this from
>> happening in the most common case, but it still feels like
>> this is loosening things a little more than it should.
> 
> If we care about this, the same behavior can be achieved with GIT_CEILING_DIRECTORIES:
> 
>  GIT_CEILING_DIRECTORIES=$(cd .. && pwd) git describe ...

I only mention it because it seemed like a change in
behavior when the series aimed to not change any behavior.
I'm sure there are plenty of things which could be changed
while making imrpveoments here.  I think we'll be better off
if those changes are all noted and deliberate.

Thanks for working on this stuff, it is good to see
improvements to this sort of plumbing.

-- 
Todd

  reply	other threads:[~2023-04-14 19:42 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-14 12:18 [PATCH 00/18] version-gen: complete revamp/rewrite Felipe Contreras
2023-04-14 12:18 ` [PATCH 01/18] version-gen: reorganize Felipe Contreras
2023-04-14 12:18 ` [PATCH 02/18] version-gen: trivial cleanup Felipe Contreras
2023-04-14 12:18 ` [PATCH 03/18] version-gen: refactor default version Felipe Contreras
2023-04-14 12:18 ` [PATCH 04/18] version-gen: simplify v prefix removal Felipe Contreras
2023-04-14 12:18 ` [PATCH 05/18] version-gen: simplify update check Felipe Contreras
2023-04-14 12:18 ` [PATCH 06/18] version-gen: remove redundant check Felipe Contreras
2023-04-14 15:11   ` Todd Zullinger
2023-04-14 17:47     ` Felipe Contreras
2023-04-14 19:01       ` Todd Zullinger
2023-04-14 19:32         ` Felipe Contreras
2023-04-14 19:41           ` Todd Zullinger [this message]
2023-04-14 12:18 ` [PATCH 07/18] version-gen: simplify `git describe` checks Felipe Contreras
2023-04-14 12:18 ` [PATCH 08/18] version-gen: simplify dirty check Felipe Contreras
2023-04-14 12:18 ` [PATCH 09/18] version-gen: move describe fix into function Felipe Contreras
2023-04-14 12:18 ` [PATCH 10/18] version-gen: describe and sed in one go Felipe Contreras
2023-04-14 12:18 ` [PATCH 11/18] version-gen: refactor describe function Felipe Contreras
2023-04-14 12:18 ` [PATCH 12/18] version-gen: do v fix only when necessary Felipe Contreras
2023-04-14 12:18 ` [PATCH 13/18] version-gen: move v fix into sed Felipe Contreras
2023-04-14 12:18 ` [PATCH 14/18] version-gen: refactor main functionality Felipe Contreras
2023-04-14 12:18 ` [PATCH 15/18] version-gen: remove default version Felipe Contreras
2023-04-14 12:18 ` [PATCH 16/18] version-gen: refactor GIT_VERSION string Felipe Contreras
2023-04-14 12:18 ` [PATCH 17/18] version-gen: get rid of GVF variable Felipe Contreras
2023-04-14 12:18 ` [PATCH 18/18] version-gen: generate proper interim versions Felipe Contreras

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

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

  git send-email \
    --in-reply-to=ZDmsg0njhtv_qM9a@pobox.com \
    --to=tmz@pobox.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.