Support -C option

[illiliti]

This option is non-standard, but widely available. Current implementation is compatible with gmake, bmake and smake.

---

TODO:

• [x] Should this be guarded with ENABLE_FEATURE_MAKE_EXTENSIONS?

[E5ten]

I'm not sure what the policy for the project is in regards to what gets guarded by ENABLE_FEATURE_MAKE_EXTENSIONS, but I believe that additional options to utilities not specified by POSIX are inherently undefined, and it is still completely POSIX conformant if the utility accepts that option. So that might be a reason not to guard it.

[rmyorston]

Both GNU make and bmake say:

If multiple -C options are specified, each is interpreted relative to the previous one: -C / -C etc is equivalent to -C /etc.

What's implemented here is that only the last -C option is applied.

[rmyorston]

I suppose the closest thing to a policy is what I wrote on my website:

In writing this version I've tried to stick to the POSIX standard. Where the standard leaves things undefined or implementation dependent I've mostly chosen the smallest, simplest, laziest option. The default build from source should result in a utility that does the bare minimum required of a POSIX make.

Features that aren't explicitly specified in the standard probably shouldn't be in the default build. Otherwise pdpmake ends up just being another make with a selection of incompatible, non-standard features. There are plenty of those already.

It's possible I've been too permissive in the default build. Maybe it shouldn't allow more than one pathname in an include line. I don't know.

[illiliti]

If multiple -C options are specified, each is interpreted relative to the previous one: -C / -C etc is equivalent to -C /etc.

Fixed, thanks for pointing that out.

[E5ten]

I don't know if this is important or if there is any problem with the current method (I can't think of one, I just think this difference is worth noting), but GNU make doesn't do chdir() until after the options have been parsed. Like I said, I think this also works just as well, but the difference should be known in case some problem does pop up somehow.

[illiliti]

Updated commit message to mention implementation details as suggested by @E5ten

[rmyorston]

GNU make puts the directories given in -C options into a list. It seems to need this to implement its 'makefiles can be rebuilt' extension. When makefiles have been rebuilt, make is restarted and the chdir() calls are rerun.

Since they have the list of directories anyway it does no harm to use it during a normal start. I don't think it makes much difference (apart from making the binary a bit bigger).

I see that bmake does the chdir() calls while the options are being processed.

One thing to consider is how the -C option affects PR #10. The absolute path to the make binary is relative to the original directory, not where we end up after applying -C options.

[illiliti]

The absolute path to the make binary is relative to the original directory, not where we end up after applying -C options.

Yeah, this is pretty interesting. I wonder if we should use realpath() to fix this like bmake did. I tested gmake and smake since both use getcwd. It seems that gmake does some smart logic under the hood to make sure that MAKE is valid, while smake don't care and simply prepends $PWD to argv0(like we do right now in #10). Should this be considered as a bug?

[rmyorston]

Having an invalid value for MAKE would certainly be a bug. A successful call to realpath() would avoid that possibility. It'll also result in a tidier pathname in many cases.

[illiliti]

Reworked PR #10 to use realpath(). It is no longer affected by this PR.

[rmyorston]

I've been thinking about the consequences of treating non-standard command line options as extensions. Here's a proposed patch to follow on from this PR: 0001-Treat-non-standard-options-as-extensions.txt.

Non-standard options won't be present in a default build with extensions disabled. In a build with extensions enabled it should be possible to turn off non-standard options. The current mechanism for this sort of thing, the .POSIX special target, won't work for the command line. Instead I propose a -P command line option.

From the commit message:

[PATCH] Treat non-standard options as extensions

The -C option isn't specified by POSIX, so let's treat it as an extension.

Add a -P option to request POSIX-conformance from the command line. If the -P option is specified any attempt to use a non-standard option, like -C, will be treated as an error.

The possibility of a -P option was considered by the authors of the POSIX standard but rejected. Hence, it too is a non-standard option. Though, obviously, its presence won't be treated as an error.

The -P option must be the first on the command line. It doesn't have to be the first in MAKEFLAGS as none of the options allowed there has side effects (at present). More than one -P option is permitted.

The value of the MAKE macro has to be determined before any options have been processed. If it turns out that a -P option was present the MAKE macro has to revert to the value required by the current POSIX standard.

[illiliti]

I disagree. The -P option has absolutely different purpose in smake/illumos make. In order to prevent incompatibility, i propose POSIXLY_CORRECT environment variable, well-known variable designed to enforce POSIX behavior.

[rmyorston]

I did consider using POSIXLY_CORRECT but its effects aren't limited to make. It will be passed to any commands invoked by make which may have unexpected consequences.

I also considered a long option, such as --posix, but that can't be passed via the letter string format of MAKEFLAGS.

A single-character option is the best solution. I'm not too concerned about compatibility with SunPro make and would prefer to use -P for its mnemonic value.

(As an aside, I note that BSD makes based on pmake have, in the past, had a -P option. But {Free,Net,Open}BSD, at least, no longer do.)

[E5ten]

imo picking an option used by an existing make implementation, even if it's not as widely used as GNU or BSD makes, isn't the best way to do this. I think breaking compatibility should be avoided when possible (which I think it definitely is here).

[illiliti]

It will be passed to any commands invoked by make which may have unexpected consequences.

Good point.

A single-character option is the best solution

As a last chance i propose PDPMAKE_ENABLE_FEATURE_MAKE_EXTENSIONS or PDPMAKE_POSIXLY_CORRECT.

I'm not too concerned about compatibility with SunPro make

This sounds not good at all. I guess if you don't care about sunpro make, you don't care about other niche make's as well...That's pretty sad and unfortunate.

[rmyorston]

Here's what it looks like using PDPMAKE_POSIXLY_CORRECT: 0001-Use-environment-variable-to-turn-off-non-POSIX-featu.txt.

This one also turns off the special treatment of the MAKE macro.

Another thought: if pdpmake with extensions enabled is asked to process a makefile with the .POSIX special target, should it turn off non-POSIX features for further invocations?

[illiliti]

should it turn off non-POSIX features for further invocations?

If you meant pdpmake -f file_with_posix -f non_posix, i think yes because bmake and smake disable them.

[E5ten]

Might also be describing a situation where $(MAKE) is called from a Makefile with .POSIX?

[rmyorston]

Might also be describing a situation where $(MAKE) is called from a Makefile with .POSIX?

Yes, that's what I meant. Suppose pdpmake is called with extensions enabled on a makefile containing:

.POSIX
target:
    $(MAKE) -C /somewhere/else

By virtue of the .POSIX it will warn about non-POSIX constructs in the makefile, but it won't warn about the non-POSIX -C option.

It could be made to do so if the presence of the .POSIX caused it to turn off extensions in future invocations of pdpmake.

[E5ten]

My opinion is that .POSIX should only refer to the syntax of the Makefile itself, and shouldn't have an effect on invocations of $(MAKE).

[rmyorston]

OK, here's another one: 0001-Allow-non-POSIX-features-to-be-turned-off-at-runtime.txt.

[PATCH] Allow non-POSIX features to be turned off at runtime

The -C option (GitHub PR #8) and enhanced treatment of the MAKE macro (GitHub PR #10) should be turned off in POSIX mode.

The .POSIX special target doesn't work for these features as they're processed before any makefiles are read. Instead:

• Use the --posix command line option, which must be the first given.

• Set the environment variable PDPMAKE_POSIXLY_CORRECT.

In addition, if pdpmake is run with extensions enabled and a valid .POSIX special target is present, then PDPMAKE_POSIXLY_CORRECT is set in the environment. Thus, recursive invocations of pdpmake will have non-POSIX features disabled.

[aabacchus]

OK, here's another one: 0001-Allow-non-POSIX-features-to-be-turned-off-at-runtime.txt.

In addition, if pdpmake is run with extensions enabled and a valid .POSIX special target is present, then PDPMAKE_POSIXLY_CORRECT is set in the environment. Thus, recursive invocations of pdpmake will have non-POSIX features disabled.

This sounds like sensible behaviour.

posix = getenv("PDPMAKE_POSIXLY_CORRECT") != NULL;

I'm not sure if most tools using POSIXLY_CORRECT just check if it's set or not, but it may be useful to check for a particular value. For example, if I have set export PDPMAKE_POSIXLY_CORRECT=1 in my session but I want to run pdpmake once without this option set, PDPMAKE_POSIXLY_CORRECT= pdpmake doesn't unset it. This is an aesthetic change though.

[rmyorston]

I followed what GNU tools do. They only check whether POSIXLY_CORRECT is set, the value doesn't matter.

In coreutils source:

$ git grep POSIXLY | grep getenv
src/cp.c:  x->open_dangling_dest_symlink = getenv ("POSIXLY_CORRECT") != NULL;
src/dd.c:  bool catch_siginfo = ! (SIGINFO == SIGUSR1 && getenv ("POSIXLY_CORRECT"));
src/df.c:          output_block_size = (getenv ("POSIXLY_CORRECT") ? 512 : 1024);
src/echo.c:  bool posixly_correct = !!getenv ("POSIXLY_CORRECT");
src/id.c:          || (default_format && ! getenv ("POSIXLY_CORRECT"))))
src/nohup.c:  exit_internal_failure = (getenv ("POSIXLY_CORRECT")
src/pr.c:    date_format = (getenv ("POSIXLY_CORRECT") && !hard_locale (LC_TIME)
src/printf.c:  posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);
src/pwd.c:  bool logical = (getenv ("POSIXLY_CORRECT") != NULL);
src/sort.c:  bool posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);
src/touch.c:      if (! getenv ("POSIXLY_CORRECT"))
src/uniq.c:  bool posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);
src/wc.c:  posixly_correct = (getenv ("POSIXLY_CORRECT") != NULL);

[illiliti]

Speaking of this code:

if (argv[1] && strcmp(argv[1], "--posix") == 0) {
    argv[1] = argv[0];
    ...

Should we also handle a case when argc == 0?

[rmyorston]

Even if we get beyond that if (we probably will) there's also:

    if (argv[0][0] != '/' && strchr(argv[0], '/')) {

further down which is likely to segfault, since argc==0 implies argv[0] == NULL.

   if (argc == 0)
        return EXIT_FAILURE;

right at the top of main() would make the problem go away.

[illiliti]

OK, here's another one: 0001-Allow-non-POSIX-features-to-be-turned-off-at-runtime.txt.

Patch applied with extra check for argc == 0

[rmyorston]

Patch applied. Thank you!