{"thread":{"id":"61501","subject":"[PATCH] scalar: make enlistment delete to work on all POSIX platforms","startedAt":"2024-05-17T14:43:02Z","lastAt":"2024-05-30T21:17:46Z","messageCount":5,"participants":["Marcel Telka","Junio C Hamano","Johannes Schindelin","Josh Steadmon"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"494974","messageId":"Zkds81OB7C5bTCl_@telcontar","threadId":"61501","inReplyTo":null,"subject":"[PATCH] scalar: make enlistment delete to work on all POSIX platforms","fromName":"Marcel Telka","fromEmail":"marcel@telka.sk","sentAt":"2024-05-17T14:42:59Z","receivedAt":"2024-05-17T14:43:02Z","isPatch":true,"sender":{"key":"marcel@telka.sk","avatar":"https://avatars.githubusercontent.com/u/3000173?v=4"},"body":"The ability to remove the current working directory is not guaranteed by\nPOSIX so it is better to go out of the directory we want to delete on\nall platforms unconditionally.\n\nSigned-off-by: Marcel Telka \n---\n scalar.c | 4 ----\n 1 file changed, 4 deletions(-)\n\ndiff --git a/scalar.c b/scalar.c\nindex 7234049a1b..331b91dbdb 100644\n--- a/scalar.c\n+++ b/scalar.c\n@@ -361,16 +361,13 @@ static char *remote_default_branch(const char *url)\n \n static int delete_enlistment(struct strbuf *enlistment)\n {\n-#ifdef WIN32\n \tstruct strbuf parent = STRBUF_INIT;\n \tsize_t offset;\n \tchar *path_sep;\n-#endif\n \n \tif (unregister_dir())\n \t\treturn error(_(\"failed to unregister repository\"));\n \n-#ifdef WIN32\n \t/*\n \t * Change the current directory to one outside of the enlistment so\n \t * that we may delete everything underneath it.\n@@ -385,7 +382,6 @@ static int delete_enlistment(struct strbuf *enlistment)\n \t\treturn res;\n \t}\n \tstrbuf_release(&parent);\n-#endif\n \n \tif (have_fsmonitor_support() && stop_fsmonitor_daemon())\n \t\treturn error(_(\"failed to stop the FSMonitor daemon\"));\n"},{"id":"495000","messageId":"xmqqbk54go51.fsf@gitster.g","threadId":"61501","inReplyTo":"Zkds81OB7C5bTCl_@telcontar","subject":"Re: [PATCH] scalar: make enlistment delete to work on all POSIX platforms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-17T20:49:14Z","receivedAt":"2024-05-17T20:49:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marcel Telka writes:\n\n> The ability to remove the current working directory is not guaranteed by\n> POSIX so it is better to go out of the directory we want to delete on\n> all platforms unconditionally.\n>\n> Signed-off-by: Marcel Telka \n> ---\n> scalar.c | 4 ----\n> 1 file changed, 4 deletions(-)\n\nLet's CC a few folks that had their hands in the delete_enlistment()\nfunction over the years for their opinions on this change.\n\n> diff --git a/scalar.c b/scalar.c\n> index 7234049a1b..331b91dbdb 100644\n> --- a/scalar.c\n> +++ b/scalar.c\n> @@ -361,16 +361,13 @@ static char *remote_default_branch(const char *url)\n> \n> static int delete_enlistment(struct strbuf *enlistment)\n> {\n> -#ifdef WIN32\n> \tstruct strbuf parent = STRBUF_INIT;\n> \tsize_t offset;\n> \tchar *path_sep;\n> -#endif\n> \n> \tif (unregister_dir())\n> \t\treturn error(_(\"failed to unregister repository\"));\n> \n> -#ifdef WIN32\n> \t/*\n> \t * Change the current directory to one outside of the enlistment so\n> \t * that we may delete everything underneath it.\n> @@ -385,7 +382,6 @@ static int delete_enlistment(struct strbuf *enlistment)\n> \t\treturn res;\n> \t}\n> \tstrbuf_release(&parent);\n> -#endif\n> \n> \tif (have_fsmonitor_support() && stop_fsmonitor_daemon())\n> \t\treturn error(_(\"failed to stop the FSMonitor daemon\"));\n"},{"id":"495045","messageId":"fb20ebe4-e643-211e-29c8-2cbfb4b49abf@gmx.de","threadId":"61501","inReplyTo":"xmqqbk54go51.fsf@gitster.g","subject":"Re: [PATCH] scalar: make enlistment delete to work on all POSIX platforms","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2024-05-18T19:45:23Z","receivedAt":"2024-05-18T19:45:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 17 May 2024, Junio C Hamano wrote:\n\n> Marcel Telka writes:\n>\n> > The ability to remove the current working directory is not guaranteed by\n> > POSIX so it is better to go out of the directory we want to delete on\n> > all platforms unconditionally.\n> >\n> > Signed-off-by: Marcel Telka \n> > ---\n> > scalar.c | 4 ----\n> > 1 file changed, 4 deletions(-)\n>\n> Let's CC a few folks that had their hands in the delete_enlistment()\n> function over the years for their opinions on this change.\n>\n> > diff --git a/scalar.c b/scalar.c\n> > index 7234049a1b..331b91dbdb 100644\n> > --- a/scalar.c\n> > +++ b/scalar.c\n> > @@ -361,16 +361,13 @@ static char *remote_default_branch(const char *url)\n> >\n> > static int delete_enlistment(struct strbuf *enlistment)\n> > {\n> > -#ifdef WIN32\n> > \tstruct strbuf parent = STRBUF_INIT;\n> > \tsize_t offset;\n> > \tchar *path_sep;\n> > -#endif\n> >\n> > \tif (unregister_dir())\n> > \t\treturn error(_(\"failed to unregister repository\"));\n> >\n> > -#ifdef WIN32\n> > \t/*\n> > \t * Change the current directory to one outside of the enlistment so\n> > \t * that we may delete everything underneath it.\n> > @@ -385,7 +382,6 @@ static int delete_enlistment(struct strbuf *enlistment)\n> > \t\treturn res;\n> > \t}\n> > \tstrbuf_release(&parent);\n> > -#endif\n\nBasically, this turns the previously Windows-only logic to `chdir(\"..\")`\ninto the now-universal logic.\n\nI like it!\n\nThank you,\nJohannes\n\n> >\n> > \tif (have_fsmonitor_support() && stop_fsmonitor_daemon())\n> > \t\treturn error(_(\"failed to stop the FSMonitor daemon\"));\n>\n"},{"id":"495970","messageId":"v6q5pahyzcoa7c6tp2x63awvtc6xvjjpi4kl5okgk5qi4god7f@yg55zwwmcuzt","threadId":"61501","inReplyTo":"Zkds81OB7C5bTCl_@telcontar","subject":"Re: [PATCH] scalar: make enlistment delete to work on all POSIX platforms","fromName":"Josh Steadmon","fromEmail":"steadmon@google.com","sentAt":"2024-05-30T20:57:13Z","receivedAt":"2024-05-30T20:57:19Z","isPatch":true,"sender":{"key":"steadmon@google.com","avatar":"https://avatars.githubusercontent.com/u/2654920?v=4"},"body":"On 2024.05.17 16:42, Marcel Telka wrote:\n> The ability to remove the current working directory is not guaranteed by\n> POSIX so it is better to go out of the directory we want to delete on\n> all platforms unconditionally.\n> \n> Signed-off-by: Marcel Telka \n> ---\n> scalar.c | 4 ----\n> 1 file changed, 4 deletions(-)\n> \n> diff --git a/scalar.c b/scalar.c\n> index 7234049a1b..331b91dbdb 100644\n> --- a/scalar.c\n> +++ b/scalar.c\n> @@ -361,16 +361,13 @@ static char *remote_default_branch(const char *url)\n> \n> static int delete_enlistment(struct strbuf *enlistment)\n> {\n> -#ifdef WIN32\n> \tstruct strbuf parent = STRBUF_INIT;\n> \tsize_t offset;\n> \tchar *path_sep;\n> -#endif\n> \n> \tif (unregister_dir())\n> \t\treturn error(_(\"failed to unregister repository\"));\n> \n> -#ifdef WIN32\n> \t/*\n> \t * Change the current directory to one outside of the enlistment so\n> \t * that we may delete everything underneath it.\n> @@ -385,7 +382,6 @@ static int delete_enlistment(struct strbuf *enlistment)\n> \t\treturn res;\n> \t}\n> \tstrbuf_release(&parent);\n> -#endif\n> \n> \tif (have_fsmonitor_support() && stop_fsmonitor_daemon())\n> \t\treturn error(_(\"failed to stop the FSMonitor daemon\"));\n> \n\nThis looks like a straightforward change; none of the formerly protected\nlogic uses anything specific to Windows, and tests still pass on Linux,\nso this looks good to me.\n\nReviewed-by: Josh Steadmon \n"},{"id":"495971","messageId":"xmqqwmnbngni.fsf@gitster.g","threadId":"61501","inReplyTo":"v6q5pahyzcoa7c6tp2x63awvtc6xvjjpi4kl5okgk5qi4god7f@yg55zwwmcuzt","subject":"Re: [PATCH] scalar: make enlistment delete to work on all POSIX platforms","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-05-30T21:17:37Z","receivedAt":"2024-05-30T21:17:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh Steadmon writes:\n\n> This looks like a straightforward change; none of the formerly protected\n> logic uses anything specific to Windows, and tests still pass on Linux,\n> so this looks good to me.\n>\n> Reviewed-by: Josh Steadmon \n\nThanks.\n\nWith Dscho's Ack we saw earlier, I should have marked the topic for\n'next' already, but I was placing a lot of things on back burner. A\ngentle nudge like this is greatly appreciated.\n\nWill merge to 'next'. Thanks.\n"}]}