configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables #1231

alejandro-colomar · 2025-02-24T22:35:59Z

Closes: #1229
Reported-by: @zeha
Cc: @thesamesam

Is this okay?

Revisions:

v2

Remove definitions of $prefix and $exec_prefix. [@thesamesam ]

$ git range-diff master gh/ep ep 
1:  8840d0e3 ! 1:  6f4e5d61 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
    @@ Metadata
      ## Commit message ##
         configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
     
    +    According to Sam, if we just remove the definitions, autotools will
    +    provide the right values.
    +
    +    Note that this changes the default from /usr to /usr/local, which is the
    +    right value according to the GNU conding standards (and also what the
    +    BSDs prefer, according to one conversation I had with Ingo Schwarze from
    +    OpenBSD).
    +
         Closes: <https://github.com/shadow-maint/shadow/issues/1229>
    +    Link: <https://www.gnu.org/software/autoconf/manual/autoconf-2.72/html_node/Default-Prefix.html>
         Reported-by: Chris Hofstaedtler <[email protected]>
         Cc: Sam James <[email protected]>
         Signed-off-by: Alejandro Colomar <[email protected]>
    @@ src/Makefile.am
      
     -ubindir = ${prefix}/bin
     -usbindir = ${prefix}/sbin
    -+prefix = /usr/local
    -+exec_prefix = ${prefix}
     +ubindir = ${exec_prefix}/bin
     +usbindir = ${exec_prefix}/sbin
      suidperms = 4755

v3

Replace ${} by $() in surrounding code. [@thesamesam ]

$ git range-diff master gh/ep ep 
1:  6f4e5d61 = 1:  6f4e5d61 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
-:  -------- > 2:  0fada16b src/Makefile.am: Use $() for consistency

v3b

Rebase

$ git range-diff master..gh/ep shadow/master..ep 
1:  6f4e5d61 = 1:  983e3308 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  0fada16b = 2:  8d0eb980 src/Makefile.am: Use $() for consistency

v3c

Rebase

$ git rd
1:  983e3308 = 1:  3d09f4f9 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  8d0eb980 = 2:  51b0e9f9 src/Makefile.am: Use $() for consistency

v3d

Rebase

$ git range-diff db/master..gh/ep shadow/master..ep 
1:  3d09f4f9 = 1:  d19fdb29 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  51b0e9f9 = 2:  33142670 src/Makefile.am: Use $() for consistency

v3e

Rebase

$ git rd
1:  d19fdb29 = 1:  c499e815 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  33142670 = 2:  8738df50 src/Makefile.am: Use $() for consistency

v3f

Rebase

$ git rd
1:  c499e815 = 1:  905759e6 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  8738df50 = 2:  ba3c8f90 src/Makefile.am: Use $() for consistency

v3g

Rebase

$ git rd 
1:  905759e6 = 1:  657b01f1 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  ba3c8f90 = 2:  9dff75be src/Makefile.am: Use $() for consistency

v3h

Rebase

$ git rd
1:  657b01f1 = 1:  ca0daff9 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  9dff75be = 2:  3f86cfbb src/Makefile.am: Use $() for consistency

v3i

Rebase

$ git rd
1:  ca0daff9 = 1:  f793375e configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  3f86cfbb = 2:  cc9e79c7 src/Makefile.am: Use $() for consistency

v3j

Rebase

$ git rd
1:  f793375e = 1:  4309e187 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  cc9e79c7 = 2:  b28fb398 src/Makefile.am: Use $() for consistency

v3k

Rebase

$ git rd 
1:  4309e187 = 1:  e9fc9e1b configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  b28fb398 = 2:  7ab69930 src/Makefile.am: Use $() for consistency

v3l

Rebase

$ git rd 
1:  e9fc9e1b = 1:  0eeb7f4c configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  7ab69930 = 2:  e471ea0d src/Makefile.am: Use $() for consistency

v3m

Rebase

$ git rd 
1:  0eeb7f4c = 1:  389a4058 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  e471ea0d = 2:  a7585cb7 src/Makefile.am: Use $() for consistency

v3n

Rebase

$ git rd 
1:  389a4058 = 1:  71ea3767 configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables
2:  a7585cb7 = 2:  4f6598bb src/Makefile.am: Use $() for consistency

zeha · 2025-02-24T22:46:41Z

#1227 relies on exec_prefix already being set in configure.ac; with both combined, i get this in config.h:

#define PASSWD_PROGRAM "NONE/bin/passwd"

but maybe you can just push a fix to #1227.

alejandro-colomar · 2025-02-24T22:54:06Z

#1227 relies on exec_prefix already being set in configure.ac; with both combined, i get this in config.h:
#define PASSWD_PROGRAM "NONE/bin/passwd"
but maybe you can just push a fix to #1227.

I bail out; too much autotools for me. @thesamesam please help. :-)

thesamesam · 2025-02-25T04:07:50Z

I'll look but probably tomorrow.

src/Makefile.am

thesamesam

OK with that one change for {} vs ().

src/Makefile.am

lslebodn · 2025-03-04T12:45:49Z

src/Makefile.am

@@ -2,13 +2,13 @@
 EXTRA_DIST = \
 	.indent.pro

-ubindir = ${prefix}/bin
-usbindir = ${prefix}/sbin


With the current state of MR, the same exec_prefix will be used for ubindir && bindir

https://www.gnu.org/software/automake/manual/html_node/Standard-Directory-Variables.html
So with the current state of MR ubindir == bindir and usbindir == sbindir.

Do we keepubindir and usbindir in such case?
Or they are intentionally separate in Makefile.am so one can use make variables to have different values for ubindir and bindir e.g.

make install ubindir=/tmp/bin usbindir=/tmp/sbin

Otherwise LGTM and +1 for removing hacks :-)

I want to remove them, but in a separate PR. I'd say even in a separate release, just to make sure we don't break anything at some point.

alejandro-colomar · 2025-03-05T08:47:52Z

#1227 relies on exec_prefix already being set in configure.ac; with both combined, i get this in config.h:
#define PASSWD_PROGRAM "NONE/bin/passwd"
but maybe you can just push a fix to #1227.

Has this been fixed?

Cc: @zeha

ikerexxe · 2025-03-26T15:37:20Z

As it is now, the patch does not break anything in the fedora packaging.

alejandro-colomar · 2025-03-26T18:21:32Z

#1227 relies on exec_prefix already being set in configure.ac; with both combined, i get this in config.h:
#define PASSWD_PROGRAM "NONE/bin/passwd"
but maybe you can just push a fix to #1227.
Has this been fixed?

Cc: @zeha

Self answer: I still get this.

@thesamesam , do you know why?

alejandro-colomar · 2025-03-30T09:39:08Z

#1227 relies on exec_prefix already being set in configure.ac; with both combined, i get this in config.h:
#define PASSWD_PROGRAM "NONE/bin/passwd"
but maybe you can just push a fix to #1227.
Has this been fixed?
Cc: @zeha
Self answer: I still get this.

@thesamesam , do you know why?

It's due to this line:

shadow/configure.ac

Line 106 in b4beda8

AC_DEFINE_UNQUOTED(PASSWD_PROGRAM, "$exec_prefix/bin/passwd",

But is that line bad? How should one use $exec_prefix within configure.ac properly?

…directory variables According to Sam, if we just remove the definitions, autotools will provide the right values. Note that this changes the default from /usr to /usr/local, which is the right value according to the GNU conding standards (and also what the BSDs prefer, according to one conversation I had with Ingo Schwarze from OpenBSD). Closes: <shadow-maint#1229> Link: <https://www.gnu.org/software/autoconf/manual/autoconf-2.72/html_node/Default-Prefix.html> Reported-by: Chris Hofstaedtler <[email protected]> Cc: Sam James <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

Some code was using $() and other ${}. Use the same everywhere. Reported-by: Sam James <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

ikerexxe reviewed Feb 25, 2025

View reviewed changes

src/Makefile.am Outdated Show resolved Hide resolved

thesamesam reviewed Feb 25, 2025

View reviewed changes

src/Makefile.am Outdated Show resolved Hide resolved

alejandro-colomar force-pushed the ep branch from 8840d0e to 6f4e5d6 Compare February 26, 2025 07:01

thesamesam reviewed Feb 27, 2025

View reviewed changes

src/Makefile.am Outdated Show resolved Hide resolved

src/Makefile.am Show resolved Hide resolved

alejandro-colomar requested a review from thesamesam February 28, 2025 21:03

alejandro-colomar force-pushed the ep branch 2 times, most recently from 8d0eb98 to 51b0e9f Compare March 4, 2025 12:03

lslebodn reviewed Mar 4, 2025

View reviewed changes

alejandro-colomar force-pushed the ep branch from 51b0e9f to 3314267 Compare March 26, 2025 18:19

alejandro-colomar force-pushed the ep branch from 3314267 to 8738df5 Compare May 3, 2025 15:23

alejandro-colomar force-pushed the ep branch 5 times, most recently from cc9e79c to b28fb39 Compare June 2, 2025 09:28

alejandro-colomar force-pushed the ep branch 3 times, most recently from e471ea0 to a7585cb Compare June 4, 2025 08:22

alejandro-colomar added 2 commits June 7, 2025 20:01

src/Makefile.am: Use $() for consistency

4f6598b

Some code was using $() and other ${}. Use the same everywhere. Reported-by: Sam James <[email protected]> Signed-off-by: Alejandro Colomar <[email protected]>

alejandro-colomar force-pushed the ep branch from a7585cb to 4f6598b Compare June 7, 2025 18:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables #1231

configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables #1231

Uh oh!

alejandro-colomar commented Feb 24, 2025 •

edited

Loading

Uh oh!

zeha commented Feb 24, 2025

Uh oh!

alejandro-colomar commented Feb 24, 2025 •

edited

Loading

Uh oh!

thesamesam commented Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

thesamesam left a comment

Uh oh!

Uh oh!

Uh oh!

lslebodn Mar 4, 2025

Uh oh!

alejandro-colomar Mar 5, 2025

Uh oh!

alejandro-colomar commented Mar 5, 2025 •

edited

Loading

Uh oh!

ikerexxe commented Mar 26, 2025

Uh oh!

alejandro-colomar commented Mar 26, 2025 •

edited

Loading

Uh oh!

alejandro-colomar commented Mar 30, 2025

Uh oh!

Uh oh!

configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables #1231

Are you sure you want to change the base?

configure.ac, src/Makefile.am: Follow GNU coding standards regarding directory variables #1231

Uh oh!

Conversation

alejandro-colomar commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zeha commented Feb 24, 2025

Uh oh!

alejandro-colomar commented Feb 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thesamesam commented Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

thesamesam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

lslebodn Mar 4, 2025

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

alejandro-colomar commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ikerexxe commented Mar 26, 2025

Uh oh!

alejandro-colomar commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alejandro-colomar commented Mar 30, 2025

Uh oh!

Uh oh!

alejandro-colomar commented Feb 24, 2025 •

edited

Loading

alejandro-colomar commented Feb 24, 2025 •

edited

Loading

alejandro-colomar commented Mar 5, 2025 •

edited

Loading

alejandro-colomar commented Mar 26, 2025 •

edited

Loading