git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Norbert Nemec <Norbert.Nemec@microsoft.com>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: RE: fast-import on existing branches
Date: Tue, 12 Mar 2019 09:18:42 +0000	[thread overview]
Message-ID: <VI1PR8303MB008034B99238403D0BCEBBACFD490@VI1PR8303MB0080.EURPRD83.prod.outlook.com> (raw)
In-Reply-To: <CABPp-BGs4E48bBQ0e94jxhoXv6t9nzwoNEEnb37tUBpTsi_mCw@mail.gmail.com>

Hi Elijah,

thanks for explaining the motivation behind the current solution!

I still believe the situation could be improved without breaking compatibility:
* in the documentation the paragraph about "Omitting the from command" should change "existing branches" into something like "existing branches within the cache of the current fast-import stream". The current phrasing is simply wrong.
* the documentation of the "from branch^0" variant currently looks like a solution for a rare situation that is easily overlooked. Maybe it could be integrated with the paragraph about "Omitting" since it is very closely related.
* in update_branch the warning could hint at the possible solution (explicitly supply a from parent_branch^0 argument)
* I'm still not sure why it would hurt to change fatal error in parse_from about creating a branch from itself to simply fall back to the ^0 behavior?

A much better solution in my view would be to aim for real statelessness and make the caching of branch pointers completely transparent. For all I can see, fast-import nearly follows this paradigm but violates it in this subtle point. Once way to achieve this would be to offer an explicit way to state that a commit should have no parent and deprecate the variant without "from" argument, issuing a warning. The great practical advantage of that would be that interrupting and continuing the fast-import stream would be guaranteed to deliver the same result. Far more important is the conceptual simplicity: A developer could completely forget about caching when it comes to correctness and only think of it when it comes to performance optimization.

Anyway: these are all improvements for future developers. Personally, I am satisfied with everything I have.

Greetings,
Norbert


-----Original Message-----
From: Elijah Newren <newren@gmail.com> 
Sent: Monday, March 11, 2019 8:46 PM
To: Norbert Nemec <Norbert.Nemec@microsoft.com>
Cc: git@vger.kernel.org
Subject: Re: fast-import on existing branches

Hi Norbert,

On Fri, Mar 8, 2019 at 9:38 AM Norbert Nemec <Norbert.Nemec@microsoft.com> wrote:
>
> Thanks, Elijah, I had indeed missed that block about the ^0 handling.
>
> I still don't get why this awkward workaround is required. Why isn't that lookup done by default? Performance can't be the reason, since the same lookup is done lateron anyway, just as correctness check. The way I read the documentation, providing no "from" should continue committing to a branch in any case. I would never have seen the continuation of an incremental import a "special case". There is a number of tools around that sync a git repo from some other source and would regularly need to continue an existing branch.
>
> Greetings,
> Norbert

If this "awkward workaround", as you put it, were removed it would make it impossible to create a commit with no parent without using a different branch name.  I really like being able to export, modify, and re-import history, using something of the form:

   git fast-export --all | <edit the stream somehow> | git fast-import --force

which would no longer work if fast-import automatically assumed a parent for every from-less commit in the input based on the reference name.  Personally, I'm more on the side of not understanding why "from" isn't required whenever you want your commit to have a parent; users can specify either a sha or a mark-id easily enough; I don't see what it saves to allow omitting it, and it inevitably leads to other confusion like yours.  But I'm well over a decade too late to advocate for that.


Hope that helps,
Elijah

      reply	other threads:[~2019-03-12  9:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-08 10:50 fast-import on existing branches Norbert Nemec
2019-03-08 15:40 ` Elijah Newren
2019-03-08 17:33   ` Norbert Nemec
2019-03-11 19:46     ` Elijah Newren
2019-03-12  9:18       ` Norbert Nemec [this message]

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=VI1PR8303MB008034B99238403D0BCEBBACFD490@VI1PR8303MB0080.EURPRD83.prod.outlook.com \
    --to=norbert.nemec@microsoft.com \
    --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).