conda / conda-build

Commands and tools for building conda packages
https://docs.conda.io/projects/conda-build/
Other
380 stars 421 forks source link

Exported CFLAGS & FFLAGS set -I to non-existent include directory #3008

Closed maddenp closed 6 years ago

maddenp commented 6 years ago

Actual Behavior

In a build environment, conda-build exports CFLAGS and FFLAGS variables that point to a non-existent directory, eliciting a compiler warning from gfortran. Specifically, the values of the -I switches in those variables point to the equivalent of $PREFIX/include, which does not exist (conda-build does not create it, even though it creates $PREFIX/bin, $PREFIX/lib and $PREFIX/share).

Example from a build:

/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_build_env/bin/x86_64-conda_cos6-linux-gnu-gfortran -fopenmp -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -I/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/include -fdebug-prefix-map=/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/work=/usr/local/src/conda/foo-0.0.1 -fdebug-prefix-map=/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl=/usr/local/src/conda-prefix -o /lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/bin/p p.f90
f951: Warning: Nonexistent include directory '/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/include' [-Wmissing-include-dirs]
/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_build_env/bin/x86_64-conda_cos6-linux-gnu-cc -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -pipe -I/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/include -fdebug-prefix-map=/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/work=/usr/local/src/conda/foo-0.0.1 -fdebug-prefix-map=/lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl=/usr/local/src/conda-prefix -o /lscratch/pmadden/miniconda/conda-bld/foo_1531399027142/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl/bin/q q.c

Expected Behavior

Ideally, conda-build would create the $PREFIX/include directory so that the -I switches are correct and so that this standard directory would be available to the build without manually creating it. For example, include is a common place to store Fortran .mod files that need to be packaged with a library.

In any case, conda-build should not set switches that will certainly cause compiler warnings, which should only reflect end-user coding problems.

Steps to Reproduce

% cat meta.yaml
package:
  name: foo
  version: 0.0.1

source:
  path: .

requirements:
  build:
    - {{ compiler('c') }}
    - {{ compiler('fortran') }}
    - make 4.2.1

build:
  script: make

% cat Makefile 
all: $(PREFIX)/bin/p $(PREFIX)/bin/q

$(PREFIX)/bin/p: p.f90
    $(FC) $(FFLAGS) -o $@ $<

$(PREFIX)/bin/q: q.c
    $(CC) $(CFLAGS) -o $@ $<

% cat p.f90 
program p
  print *,'p'
end program p

% cat q.c 
#include <stdio.h>

void main() {
  printf("q\n");
}

% conda build .
Output of conda info
     active environment : None
       user config file : /home/spire/pmadden/.condarc
 populated config files : 
          conda version : 4.5.5
    conda-build version : 3.10.9
         python version : 3.6.6.final.0
       base environment : /lscratch/pmadden/miniconda  (writable)
           channel URLs : https://repo.anaconda.com/pkgs/main/linux-64
                          https://repo.anaconda.com/pkgs/main/noarch
                          https://repo.anaconda.com/pkgs/free/linux-64
                          https://repo.anaconda.com/pkgs/free/noarch
                          https://repo.anaconda.com/pkgs/r/linux-64
                          https://repo.anaconda.com/pkgs/r/noarch
                          https://repo.anaconda.com/pkgs/pro/linux-64
                          https://repo.anaconda.com/pkgs/pro/noarch
          package cache : /lscratch/pmadden/miniconda/pkgs
                          /home/spire/pmadden/.conda/pkgs
       envs directories : /lscratch/pmadden/miniconda/envs
                          /home/spire/pmadden/.conda/envs
               platform : linux-64
             user-agent : conda/4.5.5 requests/2.19.1 CPython/3.6.6 Linux/3.10.0-327.3.1.el7_lustre.x86_64 rhel/7.2 glibc/2.17
                UID:GID : 5090:5018
             netrc file : None
           offline mode : False
mingwandroid commented 6 years ago

I disagree. Conda-build had no place assuming any particular layout. Create it as you need it.

maddenp commented 6 years ago

That doesn't make sense: If conda-build has no place assuming any particular layout, why does it create bin, lib and include? There's either a principle in play here or there isn't.

Also, are you saying that conda-build should set a compiler option that causes a compiler warning? That's a terrible idea: Warnings are there to be heeded, and sorting the difference between warnings caused by code the user is responsible for and warnings that the build tool is erroneously injecting into the build process is unhelpful.

If you don't want to create include, fine. But would you please reopen to address the compiler warning? conda-build should not be causing compiler warnings.

msarahan commented 6 years ago

I think what you might be missing is that conda-build is not creating those folders. Those folders exist because some package that conda installed had files in them. None of your dependencies have files in a $PREFIX/include folder, so you have to create it. Conda-build does create some analogs of these folders on windows. We should either decide to do the same everywhere, or perhaps remove the windows behavior. The former is probably a less breaking change. Packages still would not include empty folders.

maddenp commented 6 years ago

Please help me understand this. Here are the contents of a package I built:

~/conda/sigio % tar tjf /lscratch/pmadden/miniconda/conda-bld/linux-64/conda-libsigio-2.0.1-0.tar.bz2 
info/hash_input.json
info/files
info/index.json
info/paths.json
info/about.json
info/git
include/sigio_module.mod
include/sigio_r_module.mod
lib/libsigio.a

If I require that package for my build, e.g.

% cat meta.yaml
package:
  name: foo
  version: 0.0.1

source:
  path: .

requirements:
  build:
    - {{ compiler('fortran') }}
    - make 4.2.1
    - conda-libsigio

build:
  script: ls -l $PREFIX

and then conda build . that, here's the layout of $PREFIX as listed by that (ginned up for demonstration purposes, obviously) build script:

drwxr-xr-x 2 pmadden spire-nwp 4096 Jul 12 08:38 bin
drwxr-xr-x 2 pmadden spire-nwp 4096 Jul 12 08:38 conda-meta
drwxr-xr-x 2 pmadden spire-nwp 4096 Jul 12 08:38 lib
drwxr-xr-x 4 pmadden spire-nwp 4096 Jul 12 08:38 share
drwxr-xr-x 3 pmadden spire-nwp 4096 Jul 12 08:38 x86_64-conda_cos6-linux-gnu

If I understand @msarahan, the bin, lib, etc. directories are there because one of the installed packages has files in those directories. My local package has files in include. Why isn't that created?

Also, nobody has tried to convince me that it's ok for a build tool to create compiler warnings that the user has no reasonable control over. Can we agree on that?

maddenp commented 6 years ago

Is there something wrong with my conda-libsigo package?

msarahan commented 6 years ago

You are putting your package in the build section, but it needs to be in host. The build section is only for build tools, and the split between build and host is the default behavior when using any kind of {{ compiler }} expression.

I don't disagree with you about the warnings, but here I'd say the warnings are actually a good thing. They're indicating that your package got installed to the wrong place. They're not as direct as they should be, but if we just blindly created the include folder, you'd end up a bit further perhaps with a header not found error.

maddenp commented 6 years ago

Aha! Thanks as usual for your kind and patient support, @msarahan . I was under the mistaken impression that the host section was only for cross-platform builds. I will re-read the latest conda-build / meta.yaml docs to better familiarize myself with this.

Indeed, the compiler warning was useful here and led to my better understanding conda-build, in a number of ways. Knowing that the required packages determine what directory structure is created under $PREFIX was key and, knowing that, I completely agree with @mingwandroid that conda-build shouldn't make opinionated choices about which directories to create.

But would it perhaps be possible for conda-build to only set -I in FFLAGS if the include directory exists (or will be created prior to compiler invocation), to avoid the gfortran warning? It seems that all I can do about the warning now is manually create a $PREFIX/include directory that I (potentially) don't need. I don't think any user is going to expect that. I see that gcc doesn't complain about a non-existent include directory the way gfortran does, so it's not surprising (since, as you mentioned in the Google group, Fortran understandably gets less attention) that this wouldn't have attracted attention before.

I hope I'm not still missing a key point, or just being obstinate, but I feel strongly that training developers to ignore warnings is a bad idea. I sure don't want to see them appear in the build after I've addressed all the actual issues in my code. I'd be happy to try to write a patch & PR for this if it had a chance of being accepted. But I'd need to know that you consider this an issue before spending the time.

mingwandroid commented 6 years ago

What are the downsides to having your recipe creating the folder? File a bug with GCC if you care enough about this issue. For me this seems very easily ignored.

maddenp commented 6 years ago

The downside is that it would be useless work to hack around an issue being introduced by conda-build. It is work that would have to be repeated in many recipes. It is work to fix an issue that no user would expect. I mean, who would add a -I/some/directory/that/does/not/exist flag to their compiler invocation?

For me this seems very easily ignored.

If you are unconvinced about the dangers of ignoring compiler warnings, and the harm done by adding useless noise to an important signal, I don't know what to tell you.

mingwandroid commented 6 years ago

The downside is that it would be useless work

How so? You complained about a compiler warning about a missing folder, I told you how to silence it; in the obvious way, by creating the folder.

an issue being introduced by conda-build.

This is not an issue introduced by conda build. Conda build is a general build system that has no business in creating any folders in the final packages (even empty ones which we strip). The CFLAGS etc are set by activation scripts in the compiler packages, so are completely orthogonal to conda build.

If you are unconvinced about the dangers of ignoring compiler warnings,

You don't get a warning if you make the folder. That's my recommendation to you here.

maddenp commented 6 years ago

The CFLAGS etc are set by activation scripts in the compiler packages, so are completely orthogonal to conda build.

OK, that's the crucial point I was missing. @msarahan wrote in the Google group that "We may need to tweak FFLAGS in our recipe" and now I believe, based on what you said, that he's referring to the build recipe in the compiler package. This is not a conda-build problem.

Thank you for taking the time to convince me.

msarahan commented 6 years ago

FWIW, @mingwandroid fought pretty strongly to not include the -I stuff in the default compiler flags. Without it there, it was necessary to add it to tons of recipes (poke around conda-forge, you'll find plenty). We ended up agreeing that if conda-build is the tool using the compiler, we'll add -I$PREFIX/include, and if the compiler is being used some other way, we won't.

The split of the build and host envs is a pretty hard learning curve, and having it happen automatically with the compiler jinja2 stuff perhaps exacerbates things. I think it's still the right technical choice, but if you have ideas on how we can improve the user experience via docs or messaging in conda-build, please do recommend them.

github-actions[bot] commented 2 years ago

Hi there, thank you for your contribution!

This issue has been automatically locked because it has not had recent activity after being closed.

Please open a new issue if needed.

Thanks!