This needs to go to staging, please rebase.

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-get-c-headers-included-in-nix-cflags-compile/21638/2

For later: I wonder if there's a more central place to remove duplicates, and whether we should do it there? E.g., what about -L flags? And all the kinds of env vars that get set up by stdenv hooks (e.g. $PATH).

@vcunat: Good suggestions! Have updated. Rebuilding now to make sure everything still works

everything looks good.

Why do you grep for "$1" and not the full path that's being added? For example, I wonder if both include and Library/Frameworks could be there at once. If so, this change would drop the latter.

You are totally right! Thanks for catching that @vcunat

I think eduke32 breaks because NIX_CFLAGS_COMPILE already contains manually added -I/...-SDL2-2.24.0-dev/include/SDL2 and grep only searches for -I/...-SDL2-2.24.0-dev/include substring. I think it's a major bug that happens to filter out any attempts of adding -I paths into /include.

This imprecision seems not easy to fix completely, I'm afraid.

Actually yes, we could grep -q -F ":$1/include:" in :$PATH:

Doesn't $PATH only contain /bin paths? I don't see any includes in $ nix develop -f. eduke32; echo $PATH.

Oops... correction: grep -q -F " $1/include " in " $NIX_CFLAGS_COMPILE "

I had a failure like the eduke32 one.

I don't think this PR was a good idea. The only reason we have multiple entries is because of strictDeps being disabled. It would be better to finally after all these years just enable it everywhere.

Can you expand on how strictDeps is related?

strictDeps=0 causes packages to be processed with the "wrong" env hooks (wrong in the cross compilation sense) as a sledgehammer approach to dealing with incorrectly categorized deps. However running the same hooks from multiple compilers can bloat search paths by appending the same new element to them more than once.

From that it doesn't sound to me that strictDeps would eliminate all duplicates, but I don't recall all corners (say recursive propagation of inputs).

Ah, the role_post suffix was ignored 🤦🏽 Perhaps you meant that?

OK, I suppose it will be better to revert until we have a better version of this deduplication.

Any preliminary comments on this approach?

ccWrapper_addCVars () {
    # See ../setup-hooks/role.bash
    local role_post
    getHostRoleEnvHook
    # Avoid adding non-existent directories and duplicates.

    local CFLAGS_varname="NIX_CFLAGS_COMPILE${role_post}"
    if [ -d "$1/include" ] \
            && (! printf %s " ${!CFLAGS_varname} " | grep -q -F " $1/include " ); then
        export ${CFLAGS_varname}+=" -isystem $1/include"
    fi

    if [ -d "$1/Library/Frameworks" ] \
            && (! printf %s " ${!CFLAGS_varname} " | grep -q -F " $1/Library/Frameworks " ); then
        export ${CFLAGS_varname}+=" -iframework $1/Library/Frameworks"
    fi
}

• I quickly tried <<< but it caused some issues even on linux. I didn't look deeply what was wrong there, as printf sounds nice.
• I fixed the missing $role_post suffix, but I couldn't avoid some complication of the code.
• I moved this new condition into the if, because it seemed cleaner.
• I checked that the original problem with darwin is gone (on x86_64-darwin).

I guess because we are appending, this is safe with respect to search path order?

If so, the new version is good.

Sorry, just catching up on this. These deep files are hard, but so interesting to hack on 😅

I was waiting anyway, as it felt better not to hurry with this into 21.11 release and wait for forking it off.

We could just revert if that is what we think is best. Duplicates aren't a huge deal. I thought it would be an easy fix, but clearly there was some subtlety that I missed

Right now it is reverted on all platforms (for a week or two, see above).

sorry, I was unclear. I meant to say that I would be fine with not dealing with the duplicates after 22.11 if it is going to be complicated to get working. If you and Ericson2314 have it sorted now, then everything is good! I need to learn more about cross compiling ....

I'd hope that the suggestion above will work well, but we can only be really sure after it's spent months in nixpkgs master (after it switches to 23.05).