Expose 'trust_ci' as a feature flag under CI #73

Open Susurrus opened 7 years ago

$CI was chosen instead of $TRAVIS and the ! -z check was done such that this can work on all CI platforms (at least GitLab CI, Travis CI, AppVeyor, and Circle CI).

This seems fine to me but:

ci should be renamed to something like trust-ci to reduce the chances of that cfg clashing from something the crate is already using.
I'm not sure if RUSTFLAGS is whitelisted and passed down to the docker containers, can you confirm this works by e.g. adding a test to src/lib.rs that would fail is executed but that has the cfg_attr(ci, ignore) attribute?

I think this is what you were asking for. I should note that I was expecting japaric/cross#91 to block this as then we can add a default whitelist for RUSTFLAGS to a template Cross.toml.

I think this is what you were asking for.

Yes, thanks.

Now it's clear that this needs whitelisting the RUSTFLAGS variable.

Interesting, it actually looks like RUSTFLAGS is being passed through on mac. On Travis only the DISABLE_TEST targets should have passed, but mac also passed and shows the test as properly ignored.

Running cross on macOS doesn't use Docker at all (cross doesn't support macOS as a host) so the cross build environment on mac is the same as the Travis environment.

Getting back to this now that japaric/cross#91 is merged, I'm actually uncertain how to set up Cross.toml for these tests in order to pass through RUSTFLAGS.

Additionally, should this whitelist RUSTFLAGS by default by having a Cross.toml set up with it and suggesting people copy that as well?

I think whitelisting RUSTFLAGS by default in Cross, itself, makes sense.

As for avoiding copying around a Cross.toml in the general case we could use Cargo's convention of making the settings in there configurable via environment variables. For instance, in Cargo land CARGO_BUILD_RUSTFLAGS="foo bar" is equivalent to [build] rustflags = ["foo", "bar"] in .cargo/config. We could have something similar like CROSS_BUILD_ENV_PASSTHROUGH being equivalent to the build.env.passthrough in Cross.toml.

I think I understand. So this would require another change to cross such that it can pull config values from environment variables as you described above. And then in addition to setting RUSTFLAGS in ci/script.rs I'd also set CROSS_BUILD_ENV_PASSTHROUGH to RUSTFLAGS. I don't know how to deal with arrays in envvars, but maybe Cargo's source would be of help here.

The only issue with this is then how does the user override this behavior easily? If they a) don't want to passthrough RUSTFLAGS or b) they want to passthrough more variables. We'd need Cross.toml to append these lists together, which I don't think is what you'd expect for envvars. What were you thinking of in this case?

@Susurrus sorry, this PR fell off my radar.

I don't know how to deal with arrays in envvars, but maybe Cargo's source would be of help here.

Cargo uses spaces to split a strings into an array of strings. I think that should work here too since env variables can't have spaces in their names (or at least I think they can't).

The only issue with this is then how does the user override this behavior easily?

Maybe put the CROSS_BUILD_ENV_PASSTHROUGH variable in the env.global field of .travis.yml / AppVeyor.yml and mention what it does and that's it's meant to be user configurable.

If they a) don't want to passthrough RUSTFLAGS

They can comment out the CROSS_BUILD_ENV_PASSTHROUGH variable altogether.

or b) they want to passthrough more variables.

They can manually edit the value of the variable to append other variable names.

I fixed everything I think, but this relies on changes in Cross such that it will read from environment variables, IIUC. So this will block until I get that functionality added.

Opened japaric/cross#147 to track the missing support for reading configuration from the environment.

japaric / trust

Expose 'trust_ci' as a feature flag under CI #73