feat: Use shell values for defaults #4
Conversation
|
@APN-Pucky this is ready for review. It is mainly to make it easier for conda-forge packaging so that I don't need to do |
Makefile
Outdated
| FC := $(shell echo $$FC) | ||
| ifndef FC | ||
| FC := gfortran | ||
| endif |
There was a problem hiding this comment.
What is the benefit of this over FC ?= gfortran? I have some alternative modifications in mind to shrink the makefile a bit more.
There was a problem hiding this comment.
You can also just run make FC=flang or make FC=${FC} as it is right now.
There was a problem hiding this comment.
Yeah, FC ?= gfortran should work, so I can just do that.
Makefile
Outdated
| FC := gfortran | ||
| endif | ||
|
|
||
| FFLAGS := $(shell echo $$FFLAGS) -fno-automatic -O2 -funroll-all-loops -std=legacy -fPIC |
There was a problem hiding this comment.
That won't allow you to go to -O3 if you like, so I'd rather put it behind.
Again with
FFLAGS = -fno-automatic -O2 -funroll-all-loops -std=legacy -fPIC $(FFLAGS)
You could call it with make FFLAGS=${FFLAGS}. To my understanding that is the default way Makefiles behave/are designed.
There was a problem hiding this comment.
For reference I found https://www.gnu.org/software/make/manual/html_node/Complex-Makefile.html where also CC and CFLAGS are directly set, but since we set some defaults lets prepend our default.
There was a problem hiding this comment.
Is there a reason to not just do
FFLAGS ?= -fno-automatic -O2 -funroll-all-loops -std=legacy -fPICinstead? (I'm asking here as I don't deeply know Make syntax beyond the basics.)
There was a problem hiding this comment.
My though process was to aim for as little modifactions necessary on the gentoo side, which automatically sets FFLAGS. By default e.g. -std=legacy is not set and then it would have to be set/appended again on the ebuild/conda side. So the question is are the FFLAGS on our side mandatory for the program to work (can't be overwritten) or optional/defaults (can be overwritten). cernlib for example has some weird flags that are necessary and does only work with -O1.
There was a problem hiding this comment.
By default e.g.
-std=legacyis not set and then it would have to be set/appended again on the ebuild/conda side. So the question is are the FFLAGS on our side mandatory for the program to work (can't be overwritten) or optional/defaults (can be overwritten).
Good question RE: the FFLAGS which I don't know the answer to (which is part of the reason I opened Issue #2).
Do you know that for Gentoo all of
Line 12 in d00bfe0
are explicitly required? In the example below I'm using
# echo $FC
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran
# echo $FFLAGS
-march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/includeand everything builds fine, as far as I can tell.
(I'm using a python Docker container only because I know it has curl installed already and so I don't have to apt install anything.)
$ docker run --rm -ti python:3.12 bash
root@7178225de27f:/# curl -fsSL https://pixi.sh/install.sh | bash && . /root/.bashrc
root@7178225de27f:/# mkdir -p example && cd $_
root@7178225de27f:/example# pixi init
✔ Created /example/pixi.toml
root@7178225de27f:/example# pixi add fortran-compiler make git
✔ Added fortran-compiler >=1.8.0,<2
✔ Added make >=4.4.1,<5
✔ Added git >=2.47.1,<3
root@7178225de27f:/example# pixi shell
(example) root@7178225de27f:/example# git clone https://github.com/matthewfeickert/mpfun90.git && cd mpfun90
(example) root@7178225de27f:/example/mpfun90# git checkout origin/feat/use-shell-information-for-defaults -b feat/use-shell-information-for-defaults
(example) root@7178225de27f:/example/mpfun90# echo $FC
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran
(example) root@7178225de27f:/example/mpfun90# echo $FFLAGS
-march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include
(example) root@7178225de27f:/example/mpfun90# make dynamic
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpfun90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpfun90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar: creating libmpfun90.a
a - mpfun90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmod90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmod90.o
a - mpmod90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmodm90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmodm90.o
a - mpmodm90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmodx90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmodx90.o
a - mpmodx90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -x libmpfun90.a
g++ -shared *.o -o libmpfun90.so
(example) root@7178225de27f:/example/mpfun90# ls libmpfun90.*
libmpfun90.a libmpfun90.so
(example) root@7178225de27f:/example/mpfun90#We can also set your flags, and things build as well
(example) root@7178225de27f:/example/mpfun90# rm -f *.a *.so *.o *.mod
(example) root@7178225de27f:/example/mpfun90# make dynamic FFLAGS="-fno-automatic -funroll-all-loops -std=legacy $FFLAGS"
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -fno-automatic -funroll-all-loops -std=legacy -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpfun90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpfun90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar: creating libmpfun90.a
a - mpfun90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -fno-automatic -funroll-all-loops -std=legacy -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmod90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmod90.o
a - mpmod90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -fno-automatic -funroll-all-loops -std=legacy -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmodm90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmodm90.o
a - mpmodm90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-gfortran -fno-automatic -funroll-all-loops -std=legacy -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /example/.pixi/envs/default/include -I/example/.pixi/envs/default/include -c mpmodx90.f90
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -rv libmpfun90.a mpmodx90.o
a - mpmodx90.o
/example/.pixi/envs/default/bin/x86_64-conda-linux-gnu-ar -x libmpfun90.a
g++ -shared *.o -o libmpfun90.so
(example) root@7178225de27f:/example/mpfun90# ls libmpfun90.*
libmpfun90.a libmpfun90.so
(example) root@7178225de27f:/example/mpfun90#There was a problem hiding this comment.
I think I took the -fno-automatic -O2 -funroll-all-loops -std=legacy -fPIC from the cuttools code, where they set these flags in the top most makefile. Also using FC = instead of FC ?= comes from there.
But since cuttools applies them to all the code, not just mpfun90, it might not be necessary.
There was a problem hiding this comment.
I'd be in favor of dropping all but -fPIC (pre-/apended) so that we are consistent with mpfun2020's (very simple) compile script.
There was a problem hiding this comment.
My thought process was to aim for as little modifications necessary on the gentoo side
Does Gentoo build from hashes? Or would any modification to this repository immediately get picked up and break builds?
It would be ideal in my mind if we could harcode the minimum set of required flags in the Makefile and have the rest be passed by the user to make things explicit about the build and not have people guess at flag choice. Though for conda-forge/staged-recipes#28415 I'll just start doing
make dynamic FC="$FC" FFLAGS="$FFLAGS"There was a problem hiding this comment.
Gentoo uses the tagged release (I set the date YYYY.MM.DD as release version). The repo can be modified freely without breaking stuff.
There was a problem hiding this comment.
I modified the repo directly now only one QA complaint remains:
* QA Notice: Missing soname symlink(s):
*
* usr/lib64/mpfun90.so.0 -> libmpfun90.so
*
should be easy enough to fix (Gentoo ideally want shared libraries to have a soname and since we are already modifying the Makefile here).
* Use the '?=' Make syntax to default to the environment variables for FC and FFLAGS, but provide defaults if they are not set.
matthewfeickert
force-pushed
the
feat/use-shell-information-for-defaults
branch
from
e8fed5a to
de16f63
Compare
|
@APN-Pucky As it seems that you're taking care of things on |
?=Make syntax to default to the environment variables forFCandFFLAGS, but provide defaults if they are not set.