* [PATCH 2/2] Setup working tree in describe
@ 2019-01-26 10:40 Sebastian Staudt
2019-01-26 11:01 ` Duy Nguyen
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Staudt @ 2019-01-26 10:40 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
This ensures the given working tree is used for --dirty and --broken.
Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
---
builtin/describe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/builtin/describe.c b/builtin/describe.c
index cc118448ee..ba1a0b199b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,
const char *prefix)
if (!hashmap_get_size(&names) && !always)
die(_("No names found, cannot describe anything."));
+ setup_work_tree();
+
if (argc == 0) {
if (broken) {
struct child_process cp = CHILD_PROCESS_INIT;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] Setup working tree in describe
2019-01-26 10:40 [PATCH 2/2] Setup working tree in describe Sebastian Staudt
@ 2019-01-26 11:01 ` Duy Nguyen
2019-01-26 14:07 ` Sebastian Staudt
0 siblings, 1 reply; 4+ messages in thread
From: Duy Nguyen @ 2019-01-26 11:01 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Git Mailing List, Junio C Hamano
On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:
>
> This ensures the given working tree is used for --dirty and --broken.
>
> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> ---
> builtin/describe.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/builtin/describe.c b/builtin/describe.c
> index cc118448ee..ba1a0b199b 100644
> --- a/builtin/describe.c
> +++ b/builtin/describe.c
> @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,
> const char *prefix)
> if (!hashmap_get_size(&names) && !always)
> die(_("No names found, cannot describe anything."));
>
> + setup_work_tree();
This forces worktree's presence in all cases and will die() if
worktree is not available. You need to check if broken or dirty is set
and only call this function in that case.
Though in my opinion it's better to call before we need it in the "if
(broke)" and "else if (dirty)" code blocks. That way you don't even
need to check if it's "dirty" or "broken". Does "broken" really need
this though? If it runs "git diff-index" separately, that command
should handle this setup_work_tree() already, or we may need to fix it
there, not here.
> +
> if (argc == 0) {
> if (broken) {
> struct child_process cp = CHILD_PROCESS_INIT;
> --
> 2.20.1
--
Duy
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] Setup working tree in describe
2019-01-26 11:01 ` Duy Nguyen
@ 2019-01-26 14:07 ` Sebastian Staudt
2019-01-26 14:28 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Staudt @ 2019-01-26 14:07 UTC (permalink / raw)
To: Junio C Hamano, Git Mailing List
Am Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
>
> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures the given working tree is used for --dirty and --broken.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> > builtin/describe.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/builtin/describe.c b/builtin/describe.c
> > index cc118448ee..ba1a0b199b 100644
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,
> > const char *prefix)
> > if (!hashmap_get_size(&names) && !always)
> > die(_("No names found, cannot describe anything."));
> >
> > + setup_work_tree();
>
> This forces worktree's presence in all cases and will die() if
> worktree is not available. You need to check if broken or dirty is set
> and only call this function in that case.
>
> Though in my opinion it's better to call before we need it in the "if
> (broke)" and "else if (dirty)" code blocks. That way you don't even
> need to check if it's "dirty" or "broken". Does "broken" really need
> this though? If it runs "git diff-index" separately, that command
> should handle this setup_work_tree() already, or we may need to fix it
> there, not here.
>
Thanks for your feedback.
Are you sure that it will fail without a working tree?
Is it even possible to have *no* working tree?
I already tested this with some real life examples, e.g.
git --git-dir /some/path/.git describe
From inside and outside of other repositories.
I didn‘t hit any errors so far.
> > +
> > if (argc == 0) {
> > if (broken) {
> > struct child_process cp = CHILD_PROCESS_INIT;
> > --
> > 2.20.1
>
>
>
> --
> Duy
Best regards,
Sebastian
Am Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen <pclouds@gmail.com>:
>
> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures the given working tree is used for --dirty and --broken.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> > builtin/describe.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/builtin/describe.c b/builtin/describe.c
> > index cc118448ee..ba1a0b199b 100644
> > --- a/builtin/describe.c
> > +++ b/builtin/describe.c
> > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,
> > const char *prefix)
> > if (!hashmap_get_size(&names) && !always)
> > die(_("No names found, cannot describe anything."));
> >
> > + setup_work_tree();
>
> This forces worktree's presence in all cases and will die() if
> worktree is not available. You need to check if broken or dirty is set
> and only call this function in that case.
>
> Though in my opinion it's better to call before we need it in the "if
> (broke)" and "else if (dirty)" code blocks. That way you don't even
> need to check if it's "dirty" or "broken". Does "broken" really need
> this though? If it runs "git diff-index" separately, that command
> should handle this setup_work_tree() already, or we may need to fix it
> there, not here.
>
> > +
> > if (argc == 0) {
> > if (broken) {
> > struct child_process cp = CHILD_PROCESS_INIT;
> > --
> > 2.20.1
>
>
>
> --
> Duy
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] Setup working tree in describe
2019-01-26 14:07 ` Sebastian Staudt
@ 2019-01-26 14:28 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2019-01-26 14:28 UTC (permalink / raw)
To: Sebastian Staudt; +Cc: Junio C Hamano, Git Mailing List
On Sat, Jan 26, 2019 at 03:07:54PM +0100, Sebastian Staudt wrote:
> Are you sure that it will fail without a working tree?
> Is it even possible to have *no* working tree?
>
> I already tested this with some real life examples, e.g.
>
> git --git-dir /some/path/.git describe
>
> From inside and outside of other repositories.
> I didn‘t hit any errors so far.
Try:
git clone --bare . bare.git
cd bare.git
git describe
That works with current versions of Git, but yields "fatal: this
operation must be run in a work tree" with your patch.
-Peff
PS Your patches seem whitespace-damaged. You might want to look into
using git-send-email.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-01-26 14:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-26 10:40 [PATCH 2/2] Setup working tree in describe Sebastian Staudt
2019-01-26 11:01 ` Duy Nguyen
2019-01-26 14:07 ` Sebastian Staudt
2019-01-26 14:28 ` Jeff King
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).