crate-ci / azure-pipelines

Easy continuous integration for Rust projects with Azure Pipelines
MIT License
90 stars 24 forks source link

Test script broken for cargo workspaces on nightly #64

Closed hawkw closed 5 years ago

hawkw commented 5 years ago

The test script currently always passes the --features flag to cargo test, even when no features are configured for the build. Previously, passing --features , would simply ignore the --features flag. However, a recent nightly change (https://internals.rust-lang.org/t/help-us-test-the-breaking-bug-fix-to-cargo-features/7317) seems to have changed this behavior, and the test scripts no longer work.

As an example, here's a build of tokio-rs/tracing which has failed on the most recent nightly due to this change:

Starting: Run tests
==============================================================================
Task         : Command line
Description  : Run a command line script using Bash on Linux and macOS and cmd.exe on Windows
Version      : 2.151.2
Author       : Microsoft Corporation
Help         : https://docs.microsoft.com/azure/devops/pipelines/tasks/utility/command-line
==============================================================================
Generating script.
Script contents:
cargo test --all --features ","
========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /home/vsts/work/_temp/e7cbca46-8139-4694-a3ed-a533a38a2095.sh
error: --features is not allowed in the root of a virtual workspace
##[error]Bash exited with code '101'.
Finishing: Run tests
jonhoo commented 5 years ago

Hmm, this is actually pretty awkward to fix due to Azure's weird limited templating language. The main issue is this line: https://github.com/crate-ci/azure-pipelines/blob/98af6ab83f56ab1b712d978038c8c76abb537859/azure/tests.yml#L37

You are running into the case where both parameter lists are empty, but I think this will break whenever either one is empty. One solution here would be to trim the string of , to either side, but of course Azure has no such function. The best fix here would probably to make both parameter things lists rather than strings, find a way to combine the lists (I think we could do this with some templating magic @epage), and then use join to produce the final string.

hawkw commented 5 years ago

@jonhoo

You are running into the case where both parameter lists are empty, but I think this will break whenever either one is empty.

My understanding, based on the internals thread I linked above, is that using --features will break for any virtual Cargo workspace, regardless of whether we pass a comma, a list of features, or an empty string?

It's possible I'm misunderstanding, but based on that, I think the script should default to not adding a --features flag when both sets of features are empty?

jonhoo commented 5 years ago

Ah, interesting, so it even goes beyond what I expected. Good call. Let me add that in.

hawkw commented 5 years ago

@jonhoo thanks! Ping me when you have a patch up and I can merge a tracing PR to see if it fixes our build?

jonhoo commented 5 years ago

@hawkw #65 is going to be the one to watch.