Allow setting, archiver, C compiler flags and linker flags from commandline

[awvwgk]

• Read Fortran compiler from FPM_FC or --compiler (deprecate FPM_COMPILER)
• Read Fortran compiler options from FPM_FFLAGS or --flag
• Read C compiler from FPM_CC or --c-compiler (deprecate FPM_C_COMPILER)
• Read C compiler options from FPM_CFLAGS or --c-flag
• Read archiver from FPM_AR or --archiver
• Read linker options from FPM_LDFLAGS or --link-flag

Closes #529

[urbanjost]

Well, lets ask for general feedback now that there has been a lot more use of fpm(1) things might have changed since the last go-around(?)

Not sure I am impartial enough to review this, as I had previous PRs to
add --flag and use of environment variables, such as #235 :> .

So I think some of the voices against doing this in the past should be
heard from once again to see if stances have changed.

On a more specific note do you really need separate functions to pull the
environment variable values? The new functions are essentially a single
line with a nested get_env(3f) call; but it might allow for further
processing to handle things like when FC has FFLAG options or is a full
pathname, so not strongly opposed to it, but a few comments specifying
why that choice was made and for use by Ford(1) would be useful. Really
hoping we make a habit of having !> comments in all new procedures.

Previous discussions included the pros and cons of the use of environment
variables when switching compilers, which for base cases fpm(1) does
very simply but environment variables complicate very significantly,
as compilers vary. Perhaps an --ignoreenv flag might be useful in this
regard, or the default could be to disregard the non-FPM_* specific
variables unless an -env flag was added.

In a previous variant the variables were only used for an unknown
compiler.

The general preference seemed to be the use of unique FPM_* names instead
of common FC, FFLAGS ... (but not standardized) variables as they are
used by other build tools in different ways. Make and CMake and other
tools do or do not allow FC to contain compiler options, to use full
pathnames, etc. fpm(1) uses the compiler name assuming it is just a leaf
name to identify profiles, etc. So instead of removing the C compiler
variable, in this vein you would actually make sure their was an FPM_
variable for each general variable instead of removing the C compiler
one; and that FPM_* variables would always override, not concatenate
with the old make(1)-derived ones.

The thought earlier was that the FPM_* names could be multiple
values in the syntax of the manifest file so they could contain
things like a compiler name to associate to the flags, and having
a unique prefix prevents them from conflicting with other flags.
Since the manifest format was not settled at the time, some syntax like
FPM_FLAG='gfortran -O2:ifort -O3:...' was discussed, somewhat like $PATH
and $LD_LIBRARY_PATH.

Some compilers already make use of the same variable names and/or have
other options like config files to set options, so it was not clear what
possibly duplicating the values might have.

Doing this through the manifest file encourages the proper arguments
are packaged for use by other packages, and can be compiler-specific,
avoiding a lot of the issues mentioned here.

Another solution is/was the use of response files, so you can have
compiler and platform-specific switches in a file that you can invoke
like "fpm build @timing", that is available via the M_CLI2 command-line
parser but never seemed to garner much use (outside of me, perhaps!).

Since one of the stated goals of fpm(1) is to easily generate a
reproducable build, and the use of the environment variables means
introducing external complexities the previous PRs were withdrawn.

All that being said everything is working so far. Assuming the main
advantage gained is not having to specify the variables on the various
fpm(1) commands (which a manifest file would address on a per-package
basis but not as nicely when going in-between packages unless "global"
profiles are supported, which breaks the "package" paradigm) and that
the use and meaning of these variable names is in no way standardized,
just common I think they should be ignored unless a switch or other
variable is present (such as $FPM_USE_ENV=YES?).

I assume you are commonly using the same variables with Cmake(1) or some
other package? There were several issues/PRs about this, I will try to
find the numbers.

That being said, I will continue reviewing this as a go-forward proposal
and leave it up to a general discussion otherwise, as at least at one
time I thought it was a good idea (see #235) , and I believe matched the Haskell
version behavior as well. Will be interesting to see if the concensus
has changed, as --flag was added after being originally rejected.

All the testing so far works. Would be interested in a discussion of
your general use case. Pretty sure there were some other issues/PRs/discussions
surrounding this so anyone feel free to add those.

[urbanjost]

PS;
Every time I pull and build a PR I keep forgetting the last way I did it; but I like to do it from the CLI so I complicate it for myself I suppose. Maybe we should add instructions in the Discussion section for things like doing an initial build from scratch and several of the ways to pull and build
a PR, especially for newcomers? Right after I typed in something that worked after two failures I remembered I had an alias for this in my .gitconfig that did it a different way :>

This time I used

git clone https://github.com/fortran-lang/fpm
cd fpm
git fetch origin pull/549/head:pull_549
git checkout pull_549
fpm build # I already have an fpm(1) on this machine
git diff master

[awvwgk]

Every time I pull and build a PR I keep forgetting the last way I did it; but I like to do it from the CLI so I complicate it for myself I suppose. Maybe we should add instructions in the Discussion section for things like doing an initial build from scratch and several of the ways to pull and build

There used to be a pop-up in the code button at the top right with the git instructions, but those have been removed after GitHub CLI was released.

I use

git fetch [email protected]:fortran-lang/fpm pull/549/head
git checkout FETCH_HEAD

[urbanjost]

Not sure if there is an advantage to one or the other right off the cuff, but missed "gh" being released. Installed it. Interesting. First thing that looks appealing is the issues command for listing and entering issues without having to type in these GUIs. Thanks. All the tests I have run so far are working, wondering about FFLAGS versus F90FLAGS, as FFLAGS is often associated with pre-F90 code; also what about LDFLAGS and CPPFLAGS, probably use more frequently than AR. What should be the criteria for which flags are supported?

Out of curiosity I can a scan and out of several million lines of files related to Make and CMake these were the closest to "standard" names I could find in Fortran-related files (far more containing C-related names than I imaged at > 30/100, which seems out of sorts, but many Fortran projects are private, I suspect):

AR         ARCH     ARFLAGS  CC        CCFLAGS   CCOPTS  CF77        CF77FLAGS
CF90FLAGS  CFLAGS   COPT     COPTIONS  CPPFLAGS  CXX     CXX11FLAGS  CXXFLAGS
CXXOPTS    FC       CF90     F77OPT    FOPTIONS  FPP     FPPFLAGS    INCLUDES
INSTALL    LDFLAGS  LFLAGS   LIB       PREFIX    TMPDIR

with the vast majority using similar names with "project" prefixes (often lowercase), which was also interesting. What it did show was there is a lot of variability in what names are used.

[awvwgk]

I'm currently looking into meson and cmake to figure out which environment variables they support and conda-build for a number of environment variables which are usually set from a packager for a build system.

also what about LDFLAGS and CPPFLAGS, probably use more frequently than AR. What should be the criteria for which flags are supported?

CFLAGS and LDFLAGS are two variables which we should support as well, I'll look into those.

[awvwgk]

All that being said everything is working so far. Assuming the main
advantage gained is not having to specify the variables on the various
fpm(1) commands (which a manifest file would address on a per-package
basis but not as nicely when going in-between packages unless "global"
profiles are supported, which breaks the "package" paradigm) and that
the use and meaning of these variable names is in no way standardized,
just common I think they should be ignored unless a switch or other
variable is present (such as $FPM_USE_ENV=YES?).

I see the main purpose of the flag options to add package independent options, like building against a custom sysroot when building relocatable packages.

I think we could have a ~/.fpmconfig or ~/.config/fpm/config.toml file where one could disable such behaviour globally. So far I didn't saw a good use-case for adding support for a global config file, but maybe we should look into this in the near future.

[milancurcic]

I don't recommend relying on these generic but common environment variables. One relevant discussion about this is in #444.

I do think there is value in having a way to control fpm behavior independent of the package. And I like using environment variables.

I would support this if we namespaced any fpm variable with FPM_, so I think FPM_FC, FPM_CC, etc.or similar would be fine.

This is a big design decision so I will request review from a few more people.

[milancurcic]

If we use env variables, I think we should not use the generic ones used by other tools.

[certik]

My view is expressed in #444, I think any environment variables should be prefixed with FPM_. Using FPM_FC instead of FPM_COMPILER is fine. Using FPM_CC instead of FPM_C_COMPILER is fine. And so on. I explained in #444 why, but I am happy to answer any questions.

My next comment is that if such an environment variable changes, say:

fpm build
export FPM_FC=ifort
fpm build

Then the second fpm build should rebuild everything, because the FPM_FC now points to a different compiler. Does this currently happen? If not, we need to fix things so that it does happen.

It looks like we might want to meet over video to brainstorm this.

[awvwgk]

I think there is merit in supporting the most common environment variables in fpm, rather than rolling our own. My motivation for this change is coming from my experience with packaging fpm. Fpm could have the capability to work just as fpm install --prefix "$PREFIX" for quite a few package ecosystems. But right now it is just tedious, because the packager has to remember that fpm works differently than other build systems and fixup the environment first export FPM_COMPILER="${FC}" export FPM_C_COMPILER="${CC}" fpm install --prefix "$PREFIX" --flag "${LDFLAGS} ${FFLAGS}" Without fixing up the environment first, the build might be broken. If you look into conda-build the Fortran environment variables are already quite a few, I really would prefer to not add more to the list with fpm.

[certik]

That's a valid use case. We could fix it with:

fpm install --prefix "$PREFIX" --use-env-variables

I am really worried about this design if we enable it by default. In order to move forward, let's enable this behavior with an option. That way in the Conda build file the build will be trivial, and that way we are not stopping progress. However, we need to meet and design this properly.

[urbanjost]

In the previous discussions the one that got the most likes was

 program demo_M_CLI2
use M_CLI2,  only : set_args, sget, specified
use M_CLI2,  only : filenames=>unnamed
character(len=:),allocatable :: env
call set_args(' -env " "')
env=sget('env')
if(specified('env'))write(*,*)'env is specified'
write(*,*)'env is ['//env//']', len(sget('env'))
end program demo_M_CLI2
! if -env not present environment variables are not used
! if -env present with no value use FPM_*
! if -env with NULL string use names CC, FC, LDFLAGS, FFLAGS,AR
! if -env with a value use that, allowing for individual sets per compiler, per ...

That is, the fpm command did not use environment variables (that would be a change, as FPM_COMPILER has been around for a while now) by default, but if the -env flag were present it would use names with the FPM_ prefix if no value was given, else if any other value was given it would be used as a suffix with an underscore between the prefix and the supported names, unless the value was a null string, in which case the plain names would be used. If activiated, the specific one would be looked for and if not found the FPM_* name would be looked for.

This would allow you to set up defaults for a specific compiler like IFORT_FC=ifort, IFORT_FFLAGS='....' or for a specific type of compile like PROFILE_FFLAGS='-pg -O0' and then enter fpm build -env PROFILE, or fpm build -env IFORT. There was debate on whether if a name was specified FPM_ should be the backup or not. So for the packaging option it would just be

fpm install --prefix "$PREFIX" --env

fpm build -env  # would be equivalent to "fpm build -env FPM"
fpm build -env IFORT  # would use IFORT_* instead of FPM_* names

and the FC, FFLAGS, AR, LDFLAGS, ... values would be used. This works with the command parser because strings have to be declared as at least one space in SETARGS(3f) but when used a null string is allowed; also I also just use a default value no one would ever use, normally "#N#" and if the value is set to that but specified(3f) is .TRUE. I know the keyword was specified but no value.

As has come up before about making sure the values are used to create a unique build this is already an issue with several compilers (or a feature, depending on what you want to do). The Intel compiler lets you specify prefix and suffix environment variables for the compile line, and allows for pointing to a config file. We used to use the config file extensively to set the defaults for the site to a specific set of parameters we found gave us highly reproducable values and good performance across multiple platforms, and also made static libraries the default. Very handy, (the white paper about which parameters to use is unfortunately not public but was worth it and I recommend others do that, as an aside). But it did cause problems with some COTS products that compiled Fortran (particularly ABAQUS) so you had to use modules or wrappers to turn it off in some cases. That is a bit of an aside, but it is worth noting several compilers have features that bypass the fpm feature of creating a unique build directory for each set of compiler options, but file-specific profiles will do that too; but I have thought about whether fpm needed a feature to unset such variables. In the previous discussions it was hoped that profiles would eliminate the need for environment variables to a large extent.

A major appeal of fpm(1) is that a new user can start building projects virtually "out of the box". If we complicate usage with too many options to satisfy a small populations' needs such as making it easier to package we risk having some of the same complaints made against other packaging utilities from make(1) and cmake(1) on, that the learning curve for learning how to use the packager for developers is way to high.

[awvwgk]

One option would be to allow specification entirely over the command line, without any environment variable usage fpm install --flag "$FFLAGS" --c-flag "$CFLAGS" \ --link-flag "$LDFLAGS" --compiler "$FC" --c-compiler "$CC" \ --archiver "$AR" This might become a bit unwieldy in practice. But I don't see how we can make this setup automatic in fpm, i.e. I don't think there is a way to know whether the C compiler should be clang or gcc together with gfortran. Another option would be to make it opt-in to use environment variables: fpm config toolchain.use-env=true which could be saved in `~/.config/fpm/config.toml` or similar and maps to [toolchain] use-env = true Regarding changes in environment variables, changing the Fortran compiler will trigger a rebuild. We currently don't track which compiler is used for C, therefore changing FPM_C_COMPILER doesn't trigger rebuilds right now (this can be considered a bug). A possible fix would be to create a hash from our entire toolchain, instead of just using the basename of the Fortran compiler and the hash of the Fortran flags.

[everythingfunctional]

I would have suggested that we support the standard environment variables (FFLAGS, CFLAGS, LDFLAGS, etc.), with FPM_* counterparts taking precedence, as a way of making things feel more natural to new users, but @urbanjost list makes me think that won't necessarily help, since that list doesn't even have FFLAGS or CFLAGS.

I've never been much of a fan of environment variables (hidden global state that affects the behavior of various commands), so I do think any options should be specifiable via the command line. I'm ok with having environment variables for those who like them. I'm not sure how much value putting these kinds of options into a global config file would have. I would think they would be more project specific than that.

I did notice that this PR adds the --c-flag and --ld-flag options. (At least I think it does). If you also add --c-compiler and --archiver (I'm not attached to that last name) options, I think I'd be in favor of this.

[certik]

@awvwgk, would you be against only supporting the FPM_* variables by default, but adding an option --use-env-variables (perhaps better named) that would allow to use the FC / FFLAGS and other variables optionally?

Then we can merge this PR, move on, you get what you want (almost, just adding one single option), I get what I want, and we can use it and see if we want to make the --use-env-variables the default. And if we decide to do --use-env-variables by default, that's a super simple PR that we can submit anytime.

The issue with making --use-env-variables the default right away is that if we later decide that it was a mistake and want to take it away, it will break people's builds. And that is mainly the experience that I want to avoid. I am not against adding optional features to test things out, and if they don't work out, we can deprecate and remove, without really breaking people's builds.

[awvwgk]

There is an issue with the order of evaluation when using a option like --use-env-variables, which would require some more restructuring of the code. Right now we are using that we can provide defaults to M_CLI2 and populate those from the environment variables. This way the values are correctly overwritten when the option is provided by the user. In this setup we would only know after the fact that we are allowed to use the environment variables as default. Brad's suggestion was easy to try out, therefore I went ahead and implemented it. For a command line switch I need a bit more time to get the implementation done properly. I'm at a conference the whole week, therefore I won't get my head free to work concentrated on a bigger coding project, but I'll pick it up for sure once I'm back. Also, my preference for this would actually be a global configuration file where every user can opt-in once. This saves the work of rewriting the logic in the CLI as well. And I kind of like the way the hierachy between command line and environment variables just works naturally with M_CLI2 right now. My suggestion to move forward would be: 1. I remove the non-prefixed environment variables from this patch 2. We discuss and review this patch 3. I implement a global setting file and a git-config like CLI 4. We add the non-prefixed environment variables as opt-in (maybe even allow to opt-out of environment variables alltogether) I guess we should organize a call for next week with at least everybody participating in this thread to discuss further, by then I will have the implementations ready for comparison.

[certik]

@awvwgk your plan works with me. A global settings file I think is fine. It's not there by default, but you can put it there and enable the non-prefixed environment variables to work.

[awvwgk]

@certik @milancurcic would you like to organize a call to discuss this topic?

Requested changes addressed

[awvwgk]

As discussed today, this PR will be limited to filling in the missing options for customizing the linker and C compiler flags as well as selecting a custom archiver. It also proposes environment variables for all those options which are prefixed with FPM_, we can remove those environment variables if they should be discussed separately.

I want to keep this PR lean and focus on this single feature, once we agree on it I will build the other points we discussed here on top and open a separate PR.

[certik]

Related to this PR, I want to point attention to this issue: conda-forge/fpm-feedstock#7

I strongly suspect the reason the fpm on Apple M1 segfaults is because we do not follow the Conda's special flags (https://github.com/conda-forge/fpm-feedstock/blob/efd5174e1f87e12f99f0b3b7e24dc0784ea7f125/recipe/build.sh#L4). It is cross-compiled on an Intel macOS, so we have to follow the exact flags, otherwise it gets miscompiled.

[awvwgk]

Yes, this issue might indeed be related to the fact that we currently have no way to pass C flags to the compiler. Maybe this patch is able to fix the cross-compiled MacOS/Arm64 binary on conda-forge.

@certik Since you have a Mac, could you give this a try? I think you can run the MacOS/Arm64 cross-compiler with rosetta on your machine and directly verify the result. Not sure how easy it is to reproduce the conda-build setup this way, but most of the environment setup should be in the build log. If not we have to wait for the next release to see whether this was the cause.

[certik]

The easiest way to try the patch is to start a PR at the fpm-feedstock (and just not merge it). It's quite hard to reproduce the build locally.

[LKedward]

Following the discussion yesterday, I'm happy for this PR to move forward as it is.
The implementation looks good to me, thank you Sebastian @awvwgk.

[certik]

Same here. Looks good to me as is.

[awvwgk]

Thanks for reviewing. I'll go ahead and merge this patch.