Provide setup overrides

[jakirkham]

Fixes #284

Provides a way to bootstrap conda-forge-build-setup. Also provides the ability for feedstocks to override their configurations manually at a local level if needed. This should hopefully give everyone a little bit of what they want while addressing a notable issue.

Testing of this can be seen in PR ( conda-forge/conda-forge-build-setup-feedstock#30 ).

TODO:

• Handle changes to the upload script. ( Use external upload script #301 )

[jakirkham]

This appears to be working, but would like to hear your thoughts on it, @pelson.

[pelson]

I think I see the usecase (dealing with the recursive issue of building conda-forge-build-setup with conda-forge-build-setup). I'm more in favour of making this a configuration item in conda-forge.yaml and modifying the rendered CI script upon rerender, rather than having a global if statement that all feedstocks see by default. In principle though, I'm certainly up for letting a feedstock override how the setup is done.

[jakirkham]

It might get a bit hairy if we try to add the override script to the config. Might I suggest we just inject the script contents into the CI scripts as we did with yum_requirements.txt? I'd still prefer the current proposal, but the latter is minimally invasive and improves readability.

[jakirkham]

Copying you on this, @jjhelmus. Basically this is the only safe way I know of to re-render conda-forge-build-setup ATM.

[jakirkham]

So I had another look at trying to inject the contents of these files into the CI YAML files. TBH I'm not sure it is such a good idea. There are lots of things that might not behave right or as expected.

For instance, one can provide arguments to the intended shell on POSIX systems in the shebang. Other examples include things like pushd and popd not working on AppVeyor with cmd, but working in batch scripts. I can't really envision all of the cases where things could go wrong, but these are a few.

These are the options I see for us. Accept this basically as is with whatever minor tweaks are needed. Reject it and come up with another way to re-render things like conda-forge-build-setup/provide feedstocks control. IMHO this still seems like a reasonable solution.

[jakirkham]

@pelson can you please let me know your thoughts on this when you have a chance?

[jakirkham]

Am sure you are utterly swamped, but would definitely appreciate your feedback on this when you have a chance @pelson .

As we need to get some of the re-rendering changes at conda-forge-build-setup to at least unpin conda-build if nothing else, this will be pretty valuable. While I agree the ifs are ugly and undesirable, injecting the contents of these scripts into CI configuration files won't work either. Maybe we can check for file existence during the re-render and run the included scripts if found. Please let me know your thoughts.

[pelson]

Thanks @jakirkham. I'll take a look at this in the context of https://github.com/conda-forge/conda-forge-build-setup-feedstock today (its already Friday here 😉 ).

[jakirkham]

So I have rewritten this entirely. It is a large departure from its original form as it attempts to detect the scripts from conda-forge-build-setup during re-rendering time instead of build time.

The result is this is a no-op for an ordinary feedstock. This should hopefully address the noise concerns raised. Also it should keep this as a feature of conda-smithy than something that appears in the CI scripts ordinarily.

As we cannot inject these scripts into the CI scripts due to the aforementioned problems, we still don't do that for conda-forge-build-setup. However, we do change the paths to point to the local copies of these scripts in the recipe directory. The result is a clean looking re-rendering for conda-forge-build-setup with mere path changes.

Would be curious to know your thoughts on these changes @pelson . 😄

[jakirkham]

Coverage dips, but I'm not worried about that.

[jakirkham]

cc-ing @jjhelmus for awareness. Have tested this in PR ( conda-forge/conda-forge-build-setup-feedstock#30 ) and it does fine.

[jakirkham]

Thoughts @conda-forge/core ?

[pelson]

This is precisely how I would have implemented this change, thanks for following through and persevering! I'd just like to confirm, this is absolutely a no-op for existing feedstocks, right? I'd like to avoid having to have to re-render all feedstocks for newlines etc.

Otherwise, 👍 from me.

[jakirkham]

Thanks for taking a look Phil.

Cool, glad to hear this is what you had in mind.

Correct this is a no-op on other feedstocks. There are not even whitespace changes. Worked very hard to ensure even whitespace changes did not occur. Have tried with things that use yum_requirements.txt (e.g. qt-console ) and ones that don't (e.g. blosc, yaml, etc.).

Please let me know if there is anything else.

[jakirkham]

Anything else?

[pelson]

My downtime is so long at the moment that you can definitely take my judicious 👍 as approval to merge 😉 .

[jakirkham]

Thanks Phil.