threads / patch / 57920

patchFix wrong info in `INSTALL`

Subject: [PATCH] Fix wrong info in `INSTALL`

## tl;dr

6 messages between May 24, 2022 and May 25, 2022. Diffs are folded; open one to read it.

replies: 5people: 5as markdown or json

Shao-Ce SUN via GitGitGadget· May 24, 2022, 20:43 UTC · lore

From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>

The user prompt should be `$` instead of `#`.

Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
---
    Fix wrong info in INSTALL
    
    The user prompt should be $ instead of #.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1241%2Fsunshaoce%2Finstall-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1241/sunshaoce/install-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1241

 INSTALL | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Show changes to INSTALL +6 −6

diff --git a/INSTALL b/INSTALL
index 4140a3f5c8b..7bb3f48311d 100644
--- a/INSTALL
+++ b/INSTALL
@@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
 will install the git programs in your own ~/bin/ directory.  If you want
 to do a global install, you can do
 
-	$ make prefix=/usr all doc info ;# as yourself
-	# make prefix=/usr install install-doc install-html install-info ;# as root
+	$ make prefix=/usr all doc info ; $ as yourself
+	# make prefix=/usr install install-doc install-html install-info ; # as root
 
 (or prefix=/usr/local, of course).  Just like any program suite
 that uses $prefix, the built results have some paths encoded,
@@ -20,10 +20,10 @@ config.mak file.
 Alternatively you can use autoconf generated ./configure script to
 set up install paths (via config.mak.autogen), so you can write instead
 
-	$ make configure ;# as yourself
-	$ ./configure --prefix=/usr ;# as yourself
-	$ make all doc ;# as yourself
-	# make install install-doc install-html;# as root
+	$ make configure ; $ as yourself
+	$ ./configure --prefix=/usr ; $ as yourself
+	$ make all doc ; $ as yourself
+	# make install install-doc install-html; # as root
 
 If you're willing to trade off (much) longer build time for a later
 faster git you can also do a profile feedback build with

base-commit: 7a3eb286977746bc09a5de7682df0e5a7085e17c

-- 
gitgitgadget

Ævar Arnfjörð Bjarmason· May 24, 2022, 21:39 UTC · re: Shao-Ce SUN via GitGitGadget · lore

Re: [PATCH] Fix wrong info in `INSTALL`

On Tue, May 24 2022, Shao-Ce SUN via GitGitGadget wrote:

Show 49 quoted lines

> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>
> The user prompt should be `$` instead of `#`.
>
> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
> ---
>     Fix wrong info in INSTALL
>     
>     The user prompt should be $ instead of #.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1241%2Fsunshaoce%2Finstall-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1241/sunshaoce/install-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1241
>
>  INSTALL | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/INSTALL b/INSTALL
> index 4140a3f5c8b..7bb3f48311d 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>  will install the git programs in your own ~/bin/ directory.  If you want
>  to do a global install, you can do
>  
> -	$ make prefix=/usr all doc info ;# as yourself
> -	# make prefix=/usr install install-doc install-html install-info ;# as root
> +	$ make prefix=/usr all doc info ; $ as yourself
> +	# make prefix=/usr install install-doc install-html install-info ; # as root
>  
>  (or prefix=/usr/local, of course).  Just like any program suite
>  that uses $prefix, the built results have some paths encoded,
> @@ -20,10 +20,10 @@ config.mak file.
>  Alternatively you can use autoconf generated ./configure script to
>  set up install paths (via config.mak.autogen), so you can write instead
>  
> -	$ make configure ;# as yourself
> -	$ ./configure --prefix=/usr ;# as yourself
> -	$ make all doc ;# as yourself
> -	# make install install-doc install-html;# as root
> +	$ make configure ; $ as yourself
> +	$ ./configure --prefix=/usr ; $ as yourself
> +	$ make all doc ; $ as yourself
> +	# make install install-doc install-html; # as root
>  
>  If you're willing to trade off (much) longer build time for a later
>  faster git you can also do a profile feedback build with
>
> base-commit: 7a3eb286977746bc09a5de7682df0e5a7085e17c

This looks good to me, FWIW I dug into this slightly and didn't know that POSIX had this to say about it:

        This variable is used for interactive prompts. Historically, the
        "superuser" has had a prompt of '#'. Since privileges are not
        required to be monolithic, it is difficult to define which
        privileges should cause the alternate prompt. However, a
        sufficiently powerful user should be reminded of that power by
        having an alternate prompt.

See https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html#tag_23_02_05_03

The one suggestion I have here is that the $subject should be clearer, e.g.:

    INSTALL: use '#', not '$' for root-run command prompt

Which in this case would both be better in --oneline output, and be enough to get rid of the commit message body entirely (unless it wished to say something more on the subject).

Philippe Blain· May 24, 2022, 22:03 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [PATCH] Fix wrong info in `INSTALL`

Hi Shao-Ce,

Le 2022-05-24 à 17:39, Ævar Arnfjörð Bjarmason a écrit :

Show 32 quoted lines

> 
> On Tue, May 24 2022, Shao-Ce SUN via GitGitGadget wrote:
> 
>> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>>
>> The user prompt should be `$` instead of `#`.
>>
>> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>> ---
>>     Fix wrong info in INSTALL
>>     
>>     The user prompt should be $ instead of #.
>>
>> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1241%2Fsunshaoce%2Finstall-v1
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1241/sunshaoce/install-v1
>> Pull-Request: https://github.com/gitgitgadget/git/pull/1241
>>
>>  INSTALL | 12 ++++++------
>>  1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/INSTALL b/INSTALL
>> index 4140a3f5c8b..7bb3f48311d 100644
>> --- a/INSTALL
>> +++ b/INSTALL
>> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>>  will install the git programs in your own ~/bin/ directory.  If you want
>>  to do a global install, you can do
>>  
>> -	$ make prefix=/usr all doc info ;# as yourself
>> -	# make prefix=/usr install install-doc install-html install-info ;# as root
>> +	$ make prefix=/usr all doc info ; $ as yourself
>> +	# make prefix=/usr install install-doc install-html install-info ; # as root

The prompt for the first invocation is already '$', what you are changing in the first line is the '# as yourself' comment which is a shell comment and thus uses '#'. This allows the whole line to be pasted as-is.

The second line is supposed to be ran as root and already has '#' as prompt, and also has '# as root' as a comment, which you simply prefix with a space.

I don't think there is anything to "fix" here...

Show 15 quoted lines

>>  
>>  (or prefix=/usr/local, of course).  Just like any program suite
>>  that uses $prefix, the built results have some paths encoded,
>> @@ -20,10 +20,10 @@ config.mak file.
>>  Alternatively you can use autoconf generated ./configure script to
>>  set up install paths (via config.mak.autogen), so you can write instead
>>  
>> -	$ make configure ;# as yourself
>> -	$ ./configure --prefix=/usr ;# as yourself
>> -	$ make all doc ;# as yourself
>> -	# make install install-doc install-html;# as root
>> +	$ make configure ; $ as yourself
>> +	$ ./configure --prefix=/usr ; $ as yourself
>> +	$ make all doc ; $ as yourself
>> +	# make install install-doc install-html; # as root

Same here.

Junio C Hamano· May 24, 2022, 22:21 UTC · re: Shao-Ce SUN via GitGitGadget · lore

Re: [PATCH] Fix wrong info in `INSTALL`

"Shao-Ce SUN via GitGitGadget" <gitgitgadget@gmail.com> writes:

> -	$ make prefix=/usr all doc info ;# as yourself
> -	# make prefix=/usr install install-doc install-html install-info ;# as root
> +	$ make prefix=/usr all doc info ; $ as yourself

This and all the changes in the patch is wrong, but it is not entirely your fault. You need to know the shell syntax.

The semicolon in ";#" is an end of one command (i.e. "make"), and the hash is "start of comment--ignore the rest of the line.

If you replace # with $, then "$ as yourself" would be fed to the shell because $ is not a comment introducer. The command line no longer can be copied and pasted, starting from "make" to the end of the line.

Thanks.

Christian Couder· May 25, 2022, 05:29 UTC · re: Shao-Ce SUN via GitGitGadget · lore

Re: [PATCH] Fix wrong info in `INSTALL`

On Wed, May 25, 2022 at 6:35 AM Shao-Ce SUN via GitGitGadget <gitgitgadget@gmail.com> wrote:

>
> From: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
>
> The user prompt should be `$` instead of `#`.

On the following lines:

$ make prefix=/usr all doc info ;# as yourself # make prefix=/usr install install-doc install-html install-info ;# as root

the '$' or '#' before "make" is indeed a prompt, but the '#' before "as" starts a single line comment. So there is no need to change it into a '$'.

See for example:

https://tldp.org/LDP/abs/html/abs-guide.html#SPECIAL-CHARS

One issue with the above lines could be that there is no space character before the '#' that starts the comments. The ';' is a command separator character though, and I think that when a new command starts with '#' it's considered a comment.

So we could perhaps get rid of the ';', but on the other hand ";#" should make it extra clear that the command before it ended and that the rest of the line is a comment.

> Signed-off-by: Shao-Ce SUN <sunshaoce@iscas.ac.cn>
> ---
>     Fix wrong info in INSTALL

Instead of "info" you could say something more specific like "prompt character".

Also we start commit subjects with the area of the code, so maybe "INSTALL: fix wrong prompt character".

Show 11 quoted lines

> diff --git a/INSTALL b/INSTALL
> index 4140a3f5c8b..7bb3f48311d 100644
> --- a/INSTALL
> +++ b/INSTALL
> @@ -5,8 +5,8 @@ Normally you can just do "make" followed by "make install", and that
>  will install the git programs in your own ~/bin/ directory.  If you want
>  to do a global install, you can do
>
> -       $ make prefix=/usr all doc info ;# as yourself
> -       # make prefix=/usr install install-doc install-html install-info ;# as root
> +       $ make prefix=/usr all doc info ; $ as yourself

Here a '#' is changed into a '$' but also a space character is inserted before the '$' without any explanation.

> +       # make prefix=/usr install install-doc install-html install-info ; # as root

Here only a space character is inserted before the second '#' without any explanation.

Junio C Hamano· May 25, 2022, 16:05 UTC · re: Christian Couder · lore

Re: [PATCH] Fix wrong info in `INSTALL`

Christian Couder <christian.couder@gmail.com> writes:

> So we could perhaps get rid of the ';', but on the other hand ";#"
> should make it extra clear that the command before it ended and that
> the rest of the line is a comment.

True. In modern shells, ";" in the ";#" sequence may not be necessary, but consistently writing ";#" as if it were a single token would probably be a good idea in the documentation.

Of course, if we can declare that the usefulness of the monospace plain text document does not matter, we could do without ";#" and typeset the comment (and the prompt) in different font and/or color. It would make it easier to see which part of the line is what is to be typed verbatim.

Thanks.