crate-ci / azure-pipelines

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

Git submodules support #38

Closed Ralith closed 5 years ago

Ralith commented 5 years ago

It looks like git submodules aren't getting checked out after setting up a repo according to the instructions: https://dev.azure.com/benesaunders/benesaunders/_build/results?buildId=2

I need this for https://github.com/Ralith/openxrs. It seems to be possible, but it's not clear to me how to apply that here.

Ralith commented 5 years ago

It looks like submodules: true or submodules: recursive needs to be set from https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema#checkout, but it's not clear to me where to set that.

epage commented 5 years ago

it's not clear to me where to set that.

A checkout step is implicit to every job. We need to add an explicit one. I'm assuming we'd want to make this configurable. We'd change the jobs to take this information in as a parameter and pass it to each individual job.

Ralith commented 5 years ago

A checkout step is implicit to every job.

That doesn't tell me much, as someone who's not already familiar with how this system works. An example would help.

I'm assuming we'd want to make this configurable.

Under what circumstances does a repository have a submodule and require that it not be checked out when building? I think enabled, or even recursive, is at the very least the reasonable default.

epage commented 5 years ago

That doesn't tell me much, as someone who's not already familiar with how this system works. An example would help.

Sorry, the rest of my statement was meant also in answer but yes, it does not go into enough detail.

An excerpt from cargo-check.yml as an example

jobs:
- job: ${{ parameters.name }}
  ${{ if eq(parameters.rust, 'stable') }}:
    displayName: cargo check
  ${{ if ne(parameters.rust, 'stable') }}:
    displayName: cargo +${{ parameters.rust }} check
  pool:
    vmImage: ubuntu-16.04
  steps:
  - template: install-rust.yml
    parameters:
      rust: ${{ parameters.rust }}
      setup: ${{ parameters.setup }}
  - script: cargo check --all --bins --examples --tests
displayName: Check compilation w/ default features

would become

jobs:
- job: ${{ parameters.name }}
  ${{ if eq(parameters.rust, 'stable') }}:
    displayName: cargo check
  ${{ if ne(parameters.rust, 'stable') }}:
    displayName: cargo +${{ parameters.rust }} check
  pool:
    vmImage: ubuntu-16.04
  steps:
  - checkout: self
    submodules: true  # or `recursive`
  - template: install-rust.yml
    parameters:
      rust: ${{ parameters.rust }}
      setup: ${{ parameters.setup }}
  - script: cargo check --all --bins --examples --tests
displayName: Check compilation w/ default features

And we'd need to do that to every job.

I think enabled, or even recursive, is at the very least the reasonable default.

The fact that Microsoft didn't make this a default makes me wonder if there is a reason to not just enable it for everyone. I defer on that decision though to @jonhoo .

jonhoo commented 5 years ago

My thinking here is that you choose whether or not submodules should be checked out is something you set up in Azure when setting up the pipeline. There's an argument to be had whether it should explicitly be part of the build script, though I'm leaning towards leaving it as a pipeline configuration option (and thus not exposing it directly in the CI scripts).