| Message ID | 20181209200449.16342-4-t.gummerer@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce no-overlay and cached mode in git checkout | expand |
On Sun, Dec 9, 2018 at 9:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > 'checkout_entry()' currently only supports creating new entries in the > working tree, but not deleting them. Add the ability to remove > entries at the same time if the entry is marked with the CE_WT_REMOVE > flag. > > Currently this doesn't have any effect, as the CE_WT_REMOVE flag is > only used in unpack-tree, however we will make use of this in a > subsequent step in the series. > > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> > --- > entry.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/entry.c b/entry.c > index 3ec148ceee..cd1c6601b6 100644 > --- a/entry.c > +++ b/entry.c > @@ -441,6 +441,13 @@ int checkout_entry(struct cache_entry *ce, > static struct strbuf path = STRBUF_INIT; > struct stat st; > > + if (ce->ce_flags & CE_WT_REMOVE) { > + if (topath) > + BUG("Can't remove entry to a path"); > + unlink_entry(ce); > + return 0; > + } This makes the path counting in nd/checkout-noisy less accurate. But it's not your fault of course. Junio, do you still want to merge that series down to 'next' or drop it? If it will be merged down, I'll keep a note and fix it once this one lands too. > + > if (topath) > return write_entry(ce, topath, state, 1); > > -- > 2.20.0.405.gbc1bbc6f85 >
On Sun, Dec 9, 2018 at 12:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > 'checkout_entry()' currently only supports creating new entries in the > working tree, but not deleting them. Add the ability to remove > entries at the same time if the entry is marked with the CE_WT_REMOVE > flag. > > Currently this doesn't have any effect, as the CE_WT_REMOVE flag is > only used in unpack-tree, however we will make use of this in a > subsequent step in the series. > > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> > --- > entry.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/entry.c b/entry.c > index 3ec148ceee..cd1c6601b6 100644 > --- a/entry.c > +++ b/entry.c > @@ -441,6 +441,13 @@ int checkout_entry(struct cache_entry *ce, > static struct strbuf path = STRBUF_INIT; > struct stat st; > > + if (ce->ce_flags & CE_WT_REMOVE) { > + if (topath) > + BUG("Can't remove entry to a path"); Minor nit: This error message is kinda hard to parse, for someone not that familiar with all the *_entry functions, like myself. Maybe add a comment before this line: /* No content and thus no path to create, so we have no pathname to return */ or reword the error slightly? Or maybe it's fine and I was just confused from lack of code familiarity, but I'll throw it out there since I stumbled on it a bit. > + unlink_entry(ce); > + return 0; > + } > + > if (topath) > return write_entry(ce, topath, state, 1); > > -- > 2.20.0.405.gbc1bbc6f85
Duy Nguyen <pclouds@gmail.com> writes: >> + if (ce->ce_flags & CE_WT_REMOVE) { >> + if (topath) >> + BUG("Can't remove entry to a path"); >> + unlink_entry(ce); >> + return 0; >> + } > > This makes the path counting in nd/checkout-noisy less accurate. But > it's not your fault of course. When we check out absense of one path, how do we want to count it? Do we say "one path checked out?" when we remove one path? > Junio, do you still want to merge that series down to 'next' or drop > it? If it will be merged down, I'll keep a note and fix it once this > one lands too. Sure, I still agree with you that "git checkout" that reports what it did when given a "<branch>", but does not report what it did when given a "<pathspec>", is being inconsistent. If it makes it easier to manage, I can kick nd/checkout-noisy out of 'next' to be rebased on whatever more appropriate when rewinding its tip. Thanks.
On 12/10, Elijah Newren wrote: > On Sun, Dec 9, 2018 at 12:05 PM Thomas Gummerer <t.gummerer@gmail.com> wrote: > > > > 'checkout_entry()' currently only supports creating new entries in the > > working tree, but not deleting them. Add the ability to remove > > entries at the same time if the entry is marked with the CE_WT_REMOVE > > flag. > > > > Currently this doesn't have any effect, as the CE_WT_REMOVE flag is > > only used in unpack-tree, however we will make use of this in a > > subsequent step in the series. > > > > Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> > > --- > > entry.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/entry.c b/entry.c > > index 3ec148ceee..cd1c6601b6 100644 > > --- a/entry.c > > +++ b/entry.c > > @@ -441,6 +441,13 @@ int checkout_entry(struct cache_entry *ce, > > static struct strbuf path = STRBUF_INIT; > > struct stat st; > > > > + if (ce->ce_flags & CE_WT_REMOVE) { > > + if (topath) > > + BUG("Can't remove entry to a path"); > > Minor nit: This error message is kinda hard to parse, for someone not > that familiar with all the *_entry functions, like myself. Maybe add > a comment before this line: > /* No content and thus no path to create, so we have no pathname > to return */ > or reword the error slightly? Or maybe it's fine and I was just > confused from lack of code familiarity, but I'll throw it out there > since I stumbled on it a bit. I'll try to make it more clear in the new round, thanks! > > + unlink_entry(ce); > > + return 0; > > + } > > + > > if (topath) > > return write_entry(ce, topath, state, 1); > > > > -- > > 2.20.0.405.gbc1bbc6f85
On Tue, Dec 11, 2018 at 3:28 AM Junio C Hamano <gitster@pobox.com> wrote: > > Duy Nguyen <pclouds@gmail.com> writes: > > >> + if (ce->ce_flags & CE_WT_REMOVE) { > >> + if (topath) > >> + BUG("Can't remove entry to a path"); > >> + unlink_entry(ce); > >> + return 0; > >> + } > > > > This makes the path counting in nd/checkout-noisy less accurate. But > > it's not your fault of course. > > When we check out absense of one path, how do we want to count it? > Do we say "one path checked out?" when we remove one path? It is still "checked out" according to this non-overlay concept. Although we could make it clear by saying "5 paths updated, 2 deleted" (but that may make us say "3 paths added" as well, hmm). Or maybe just "%d paths updated" where updates include file creation and deletion.
Duy Nguyen <pclouds@gmail.com> writes: > Although we could make it clear by saying "5 paths updated, 2 deleted" > (but that may make us say "3 paths added" as well, hmm). Or maybe just > "%d paths updated" where updates include file creation and deletion. Yeah, the last one is the simplest and good.
diff --git a/entry.c b/entry.c index 3ec148ceee..cd1c6601b6 100644 --- a/entry.c +++ b/entry.c @@ -441,6 +441,13 @@ int checkout_entry(struct cache_entry *ce, static struct strbuf path = STRBUF_INIT; struct stat st; + if (ce->ce_flags & CE_WT_REMOVE) { + if (topath) + BUG("Can't remove entry to a path"); + unlink_entry(ce); + return 0; + } + if (topath) return write_entry(ce, topath, state, 1);
'checkout_entry()' currently only supports creating new entries in the working tree, but not deleting them. Add the ability to remove entries at the same time if the entry is marked with the CE_WT_REMOVE flag. Currently this doesn't have any effect, as the CE_WT_REMOVE flag is only used in unpack-tree, however we will make use of this in a subsequent step in the series. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> --- entry.c | 7 +++++++ 1 file changed, 7 insertions(+)