mbox series

[00/10] fast export and import fixes and features

Message ID 20181111062312.16342-1-newren@gmail.com (mailing list archive)
Headers show
Series fast export and import fixes and features | expand

Message

Elijah Newren Nov. 11, 2018, 6:23 a.m. UTC
This is a series of ten patches representing two doc corrections, one
pedantic fix, three real bug fixes, one micro code refactor, and three
new features.  Each of these ten changes is relatively small in size.
These changes predominantly affect fast-export, but there's a couple
small changes for fast-import as well.

I could potentially split these patches up, but I'd just end up
chaining them sequentially since otherwise there'd be lots of
conflicts; having 10 different single patch series with lots of
dependencies sounded like a bigger pain to me, but let me know if you
would prefer I split them up and how you suggest doing so.

These patches were driven by the needs of git-repo-filter[1], but most
if not all of them should be independently useful.

Elijah Newren (10):
  git-fast-import.txt: fix documentation for --quiet option
  git-fast-export.txt: clarify misleading documentation about rev-list
    args
  fast-export: use value from correct enum
  fast-export: avoid dying when filtering by paths and old tags exist
  fast-export: move commit rewriting logic into a function for reuse
  fast-export: when using paths, avoid corrupt stream with non-existent
    mark
  fast-export: ensure we export requested refs
  fast-export: add --reference-excluded-parents option
  fast-export: add a --show-original-ids option to show original names
  fast-export: add --always-show-modify-after-rename

 Documentation/git-fast-export.txt |  33 ++++++-
 Documentation/git-fast-import.txt |   7 +-
 builtin/fast-export.c             | 156 +++++++++++++++++++++++-------
 fast-import.c                     |  17 ++++
 t/t9350-fast-export.sh            | 124 +++++++++++++++++++++++-
 5 files changed, 293 insertions(+), 44 deletions(-)

[1] https://github.com/newren/git-repo-filter if you're really
curious, but ***** IT HAS SEVERAL SHARP EDGES *****.  It isn't really
ready for review/testing/usage/announcing/etc; in fact, it's not quite
WIP/RFC ready.  (Further, it's not clear if it should somehow become
part of core git, should go into contrib, or just remain separate
indefinitely.)  Anyway, please do not attempt to use it for anything
real yet.  I'll send out an email when I think it's closer to ready.

Comments

Jeff King Nov. 11, 2018, 7:27 a.m. UTC | #1
On Sat, Nov 10, 2018 at 10:23:02PM -0800, Elijah Newren wrote:

> This is a series of ten patches representing two doc corrections, one
> pedantic fix, three real bug fixes, one micro code refactor, and three
> new features.  Each of these ten changes is relatively small in size.
> These changes predominantly affect fast-export, but there's a couple
> small changes for fast-import as well.
> 
> I could potentially split these patches up, but I'd just end up
> chaining them sequentially since otherwise there'd be lots of
> conflicts; having 10 different single patch series with lots of
> dependencies sounded like a bigger pain to me, but let me know if you
> would prefer I split them up and how you suggest doing so.

I think it's fine to put them in sequence when there's a textual
dependency.  If it turns out that one of them needs more discussion and
we don't want it to hold later patches hostage, we can always re-roll at
that point.

(I also think it's fine to lump together thematically similar patches
even when they aren't strictly dependent, even textually. It's less work
for the maintainer to consider 1 group of 10 than 10 groups of 1).

> These patches were driven by the needs of git-repo-filter[1], but most
> if not all of them should be independently useful.

I left lots of comments. Some of the earlier ones may just be showing my
confusion about fast-export works (some of which was cleared up by your
later patches). But I like the overall direction for sure.

-Peff
Elijah Newren Nov. 11, 2018, 8:44 a.m. UTC | #2
On Sat, Nov 10, 2018 at 11:27 PM Jeff King <peff@peff.net> wrote:
>
> On Sat, Nov 10, 2018 at 10:23:02PM -0800, Elijah Newren wrote:
>
> > This is a series of ten patches representing two doc corrections, one
> > pedantic fix, three real bug fixes, one micro code refactor, and three
> > new features.  Each of these ten changes is relatively small in size.
> > These changes predominantly affect fast-export, but there's a couple
> > small changes for fast-import as well.
> >
> > I could potentially split these patches up, but I'd just end up
> > chaining them sequentially since otherwise there'd be lots of
> > conflicts; having 10 different single patch series with lots of
> > dependencies sounded like a bigger pain to me, but let me know if you
> > would prefer I split them up and how you suggest doing so.
>
> I think it's fine to put them in sequence when there's a textual
> dependency.  If it turns out that one of them needs more discussion and
> we don't want it to hold later patches hostage, we can always re-roll at
> that point.
>
> (I also think it's fine to lump together thematically similar patches
> even when they aren't strictly dependent, even textually. It's less work
> for the maintainer to consider 1 group of 10 than 10 groups of 1).
>
> > These patches were driven by the needs of git-repo-filter[1], but most
> > if not all of them should be independently useful.
>
> I left lots of comments. Some of the earlier ones may just be showing my
> confusion about fast-export works (some of which was cleared up by your
> later patches). But I like the overall direction for sure.

Thanks for taking the time to read over the series and providing lots
of feedback!  And, whoops, looks like it's gotten kinda late, so I'll
check any further feedback on Monday.
Jeff King Nov. 12, 2018, 1 p.m. UTC | #3
On Sun, Nov 11, 2018 at 12:44:47AM -0800, Elijah Newren wrote:

> > > These patches were driven by the needs of git-repo-filter[1], but most
> > > if not all of them should be independently useful.
> >
> > I left lots of comments. Some of the earlier ones may just be showing my
> > confusion about fast-export works (some of which was cleared up by your
> > later patches). But I like the overall direction for sure.
> 
> Thanks for taking the time to read over the series and providing lots
> of feedback!  And, whoops, looks like it's gotten kinda late, so I'll
> check any further feedback on Monday.

Thank you for your patience with my sometimes-confused responses. :)

Overall it makes more sense to me now (and everything seems like a good
direction), with the exception that I'm still a bit confused about patch
10.

-Peff