crate-ci / azure-pipelines

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

Should we use a Azure Task? #11

Open epage opened 5 years ago

epage commented 5 years ago

spontoreau's Task might be a good basis for us. See discussion below

Original

Including

Inputs

epage commented 5 years ago

@andoriyu brought up on gitter the idea of making and publishing tasks. For rustup and gh-install (#3), I think this solves a lot of problems brought up in here.

One set of rust tasks already exists though it might have some extra policy compared to our rustup steps. We should reach out and see about collaborating.

andoriyu commented 5 years ago

I think managing shell scripts inside yaml is too much work and messy. Having ci folder with all the scripts is sub-optimal as well in my opinion - it's not part of the code base. That's why I wanted to write my own tasks but ran into an already existing project.

epage commented 5 years ago

Which scripts are you concerned about? The ones inside rustup template are trivial and seem to work well. Wanting to better understand the concern here.

If talking about things more like gh-install, see my note on #3

andoriyu commented 5 years ago

My concern is for consumers of this scripts. If I understand this project correct the idea is to place those templates into a repository and use them in pipeline definition.

Which leads to two way of handling it:

  1. Git Submodule (doubt anyone will try it)
  2. Copy scripts over (most people will go this way

Then if script gets better or changes people won't get an updated version. However, if it's treated as black box and hosted somewhere else — everyone uses the same version.

epage commented 5 years ago

If I understand this project correct the idea is to place those templates into a repository and use them in pipeline definition.

Ah, this is where the confusion is. This is meant to be a central resource rather than something people copy/paste. I want to get tooling in place so we no longer need trust and its approach to CIs (copy everything but install.sh).

nickbabcock commented 5 years ago

(For azure pipelines) What are the tangible benefits of tasks over something like a template? I know that they are more easily versioned. Anything else?

andoriyu commented 5 years ago

(For azure pipelines) What are the tangible benefits of tasks over something like a template? I know that they are more easily versioned. Anything else?

Discoverable via market place. I guess feels more natural since there're tasks for many other things already. Easier to share with people IMO.

If I understand this project correct the idea is to place those templates into a repository and use them in pipeline definition.

Ah, this is where the confusion is. This is meant to be a central resource rather than something people copy/paste. I want to get tooling in place so we no longer need trust and its approach to CIs (copy everything but install.sh).

Like a submodule or like an example for DIY?

epage commented 5 years ago

@andoriyu

Like a submodule or like an example for DIY?

By adding the org as an endpoint in the GUI, declaring the repo as a resource, and then referencing the templates in that repo

https://docs.microsoft.com/en-us/azure/devops/pipelines/process/templates?view=azure-devops#using-other-repositories

epage commented 5 years ago

(For azure pipelines) What are the tangible benefits of tasks over something like a template? I know that they are more easily versioned. Anything else?

End-user overhead for adding a remote repo as a resource is one of the reasons I'm preferring using a single repo and a single branch for all our needs but a lot of processes don't work as well in that case.

I'm assuming it is much lighter weight for users to add a task from the market place than to use a remote repo's templates.

The downside is it looks like tasks are more complex to write. If I could package up a template as a task, that'd be slick.

spontoreau commented 5 years ago

You're right, tasks are not really easy to write. Unit testing capabilities are really low, that's why the extension is moving slowly, because it needs a lot of manual testing before releasing it!

Also, tasks can be used in YAML and with the visual designer (vNext build definition). I'm currently working on a better integration of the extension in YAML.

Anyway, nice repository it can really help :)

nickbabcock commented 5 years ago
  • We'd have to have separate test folders for every major version

Would that need to be the case? It seems like testing every individual template major version would be a huge boon as users on an older version wouldn't have any functionality removed out from under them.

  • Branching and integration is weirder

Not sure I understand. If we're only dealing with a single repo and one branch, if a template is changed in a PR -- then everything that the PR touched is tested in the PR build

  • Browsing commits to see what has changed is weirder

Would a per template or global changelog fix that issue?

I'm assuming it is much lighter weight for users to add a task from the market place than to use a remote repo's templates.

I think I disagree. In the last several days, I have not come across any notion of a marketplace. A marketplace (/ custom tasks) are not described in the pipeline documentation nor listed in the pipeline ui. Templates are. I want pipelines to be copy and paste-able between projects with the minimal amount of out-of-band steps to help me avoid mistakes. So far templates seem to be the best way. Not sure what my past self was thinking -- custom tasks are most definitely documented

epage commented 5 years ago

@spontoreau

You're right, tasks are not really easy to write. Unit testing capabilities are really low, that's why the extension is moving slowly, because it needs a lot of manual testing before releasing it!

Let me check for understanding from the market place page for your tasks

You provide three tasks

What is the value of generic cargo and rustup tasks? When looking at writing step templates, I only wrote them for the "high boiler plate" stuff like rustup install and gh-install. I feel like it'd be just as much to type out the yaml for a cargo step template as it would be for just writing the cargo command.

The main benefit I can think of is

Also, from the screenshot, it looked like your rustup install had a nightly checkbox. Does this mean it only supports nightly and stable? I have found that being able to take in any toolchain name is important, including pinning exact versions (consistent warnings), including nightlies.

I'm curious what you think of my rustup step's "API" compared to the current task's "API"

https://github.com/crate-ci/resources/blob/master/az-pipeline/unstable/rustup.yml (See also #8 for more work planned)

epage commented 5 years ago

@nickbabcock

We'd have to have separate test folders for every major version

Would that need to be the case? It seems like testing every individual template major version would be a huge boon as users on an older version wouldn't have any functionality removed out from under them.

I feel like we're saying the same thing, so I'm unsure where the disconnect is. We'd want every major version tested. If we have everything in the same repo, we'd then need separate test pipelines for every major version. This is in contrast to if we used separate branches, we'd have the same amount of files but we'd be able to leverage the merging, diffing features of git because they'd be treated as different iterations of the same file.

Branching and integration is weirder

Not sure I understand. If we're only dealing with a single repo and one branch, if a template is changed in a PR -- then everything that the PR touched is tested in the PR build

If we decide to make a breaking change in this model, we then copy the v1 folder into the v2 folder (the current unstable is a master with a clear warning). We'd be copying instead of doing git checkout -b v2. We'd have to open up diff programs and compare rather than doing a git merge. etc.

Browsing commits to see what has changed is weirder

Would a per template or global changelog fix that issue?

Ok, this is me being bad at managing a changelog as I go shows through. I normally write them after-the-fact. Granted, that doesn't really work with a "continually deployed" project like templates.

The question is global vs per section (gh-install, az-pipelines) vs per version. I'd lean towards per-version changelogs since people might not want to upgrade past a breaking change for reasons but they want to track what is being changed within that version.

Not sure what my past self was thinking -- custom tasks are most definitely documented

I think we are all finding how terrible the documentation is and we all have our subset of Pipelines knowledge. I knew about native tasks (bash, powershell) and someone at work recently mentioned creating tasks was possible but not the market place. @andoriyu filled my gaps here. It looks like @andoriyu might not have known about getting templates from remote repos.

spontoreau commented 5 years ago

@spontoreau

You're right, tasks are not really easy to write. Unit testing capabilities are really low, that's why the extension is moving slowly, because it needs a lot of manual testing before releasing it!

Let me check for understanding from the market place page for your tasks

You provide three tasks

* rustup install

* generic cargo

* generic rustup

What is the value of generic cargo and rustup tasks? When looking at writing step templates, I only wrote them for the "high boiler plate" stuff like rustup install and gh-install. I feel like it'd be just as much to type out the yaml for a cargo step template as it would be for just writing the cargo command.

The main benefit I can think of is

* Flags that are CI best practices (always passing `-verbose`, replacing test with suity)

* Passing the configured `--target`

Also, from the screenshot, it looked like your rustup install had a nightly checkbox. Does this mean it only supports nightly and stable? I have found that being able to take in any toolchain name is important, including pinning exact versions (consistent warnings), including nightlies.

I'm curious what you think of my rustup step's "API" compared to the current task's "API"

https://github.com/crate-ci/resources/blob/master/az-pipeline/unstable/rustup.yml (See also #8 for more work planned)

Ok, let's give you some context ;)

First, it's important you understand that the extension was initially built before the YAML support was released. So it was designed for vNext when Azure DevOps was named Visual Studio Team Service. Before YAML, all the commands has to be included in PowerShell/Bash steps, that was not really userfriendly. That's why I decided to build the extension, then YAML was released.

Now the extension have 4 steps (rustc is missing in your list) and they were designed (as you said) in a generic way. Now there are a lot of Rust developers who are intresting in using Azure Pipeline and templates seems to be the defaut way to handle CI. That's fine for me, it's more powerfull and CI as code is a better option for everybody ;)

My opinion is that Cargo, Rustup and Rustc tasks from the extension are now useless because YAML is the default way to add CI in a project. I just keep them as legacy steps for people who are using it in some build definitions with the visual editor.

On the opposite, the install task can be really usefull in YAML because it's a crossplatform one (it works on Linux/macOS/Windows without doing anything, it's handled inside the extension) and it can reduce the number of line in scripts inside the YAML. I'm currently working to improve YAML support around the extension. It will looks like this:

pool:
  vmImage: ubuntu-16.04
steps:
- task: RustTool@1
  displayName: 'Rust Tool Installer'
  inputs:
    nightly: true

So we have a good option now if you define & maintain templates. Maybe I can just focus the extension as a "Rust Tool Installer" provider like Microsoft do with NodeTool task. It means that the current extension have to be depreceated and replaced by one that just contains a Rust toolchains installer task. As you notice this task doesn't handle all the scenarios... but if i'm just focus on it, maybe I can deliver missing features in few days !

What's your opinion on that?

cc @rylev, @johnterickson & @martinwoodward what do you think about that?

epage commented 5 years ago

Thanks for the clarification / background @spontoreau.

I was wondering if this was from the pre-yaml days where a lot more task types would make sense.

As you notice this task doesn't handle all the scenarios... but if i'm just focus on it, maybe I can deliver missing features in few days !

I don't know if all scenarios need to be handled but I think the basics plus most common for CI setup. For example, I went ahead and put rustup component add in my step. People could create a script for that but I wanted a "initialize my CI" step. With that said, I don't handle everything but people can make their own rustup calls for very custom logic.

Personally preference but

Overall though, it sounds like we are aligned. If you want, you are welcome to move it to the crate-ci org to help raise visibility (seems like a lot of us never heard about your work before), help with bus factor, etc.

Like the reasons you gave for RustTools being a task, I think we should probably make gh-install a task as well. That just leaves my rustfmt, clippy, and warnings templates. I mostly made those because I was playing with separate jobs for them (though a single job would be faster, see #4) and the per-job boiler plate was significant. I'm unsure if we should keep them.

I'd offer to help with the tasks but without knowing typescript yet or the APIs, I'm assuming my ramp up would be too much to be of benefit. I eventually need to get up to speed on these things, even if its just for $DAYJOB (migrating from perforce/jenkins to AzDO). If you want me to help, let me know. Otherwise, I'll probably continue my immediate task, rounding out cargo-releases features.

johnterickson commented 5 years ago

I agree that the "get rustup installed and in the PATH" is the most important one. It should expose as parameters the two rustup parameters:

OPTIONS:
        --default-host <default-host>              Choose a default host triple
        --default-toolchain <default-toolchain>    Choose a default toolchain to install

As I mentioned in Gitter, I think there might also be use for a "cargo test that also publishes test results", but I've not thought through that one as much.

Hmm given the size of rustup, maybe we can even get it in the default image (just rustup, no toolchains).

nickbabcock commented 5 years ago

If we decide to make a breaking change in this model, we then copy the v1 folder into the v2 folder (the current unstable is a master with a clear warning). We'd be copying instead of doing git checkout -b v2. We'd have to open up diff programs and compare rather than doing a git merge. etc.

Hmm yeah I envision the best way would be to lay out this repo in a similar fashion to how the azure pipeline tasks are structured. So we'd have

azure-pipelines/
├── rustup-1.yml
└── rustup-2.yml

rustup-2.yml would be the most current. A breaking change would then necessitate a bump and rustup-3.yml would be current

azure-pipelines/
├── rustup-1.yml
├── rustup-2.yml
└── rustup-3.yml

So no need for branches or copying folders / files or breaking existing pipelines.

spontoreau commented 5 years ago

I agree that the "get rustup installed and in the PATH" is the most important one. It should expose as parameters the two rustup parameters:

OPTIONS:
        --default-host <default-host>              Choose a default host triple
        --default-toolchain <default-toolchain>    Choose a default toolchain to install

As I mentioned in Gitter, I think there might also be use for a "cargo test that also publishes test results", but I've not thought through that one as much.

Hmm given the size of rustup, maybe we can even get it in the default image (just rustup, no toolchains).

@johnterickson it seems easy to implement inside the extension :)

@epage don't worry about the development, TypeScript is my main langage (Rust isn't yet :p) and if i'm focusing the install step it will be ok ASAP. I just need to finalize the version 1.2 first because it contains YAML support (it will be ok for the end of next week).

A name like "Rust Tool Installer" is more compliant with the current naming convention in Azure DevOps, that's why I choose it in my YAML exemple:

screenshot 2019-03-03 at 21 58 09

Before moving the repository, we can try something. Let's see if a feature request can be accepted by the Azure DevOps team to integrate an official Rust installer task in Azure Pipelines tasks. If they don't agree, I will move the repository to the crate-ci organization in order to group our efforts around this topic ;)

epage commented 5 years ago

@nickbabcock

Hmm yeah I envision the best way would be to lay out this repo in a similar fashion to how the azure pipeline tasks are structured. So we'd have

Thanks, I didn't know this was a standard.

@spontoreau

A name like "Rust Tool Installer" is more compliant with the current naming convention in Azure DevOps, that's why I choose it in my YAML exemple:

Good to know. I dislike that standard but sometimes its better to adopt a less than ideal standard than to do your own thing.

Thanks for your help on this!

epage commented 5 years ago

@johnterickson

Hmm given the size of rustup, maybe we can even get it in the default image (just rustup, no toolchains).

What are the trade offs for being in the base image? For example, I don't know how often rustup changes but keeping up to date?

I do know of at least one potential change we'll want ASAP, https://github.com/rust-lang/rustup.rs/issues/998

davidB commented 5 years ago

During the last 2 weekends I rework the CI of one of my rust project, because the travis build on windows was too long (timeout 50min on some run). I tried cirrus-ci and azure pipeline. To compare and have same behavior for task/step, I decided to move script from ci's yaml into Makefile.toml and to use cargo-make (for test, zipping, upload,...). So except the rustup toolchain setup, the scripts are ci independent.

The RustTool@1 of @spontoreau is only what I need to remove my only platform dependent steps (I don't use azure pipeline template):


# Linux
- bash: |
    curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain $RUSTUP_TOOLCHAIN
    echo "##vso[task.setvariable variable=PATH;]$PATH:$HOME/.cargo/bin"
  condition: eq( variables['Agent.OS'], 'Linux' )
  displayName: install rustup

# macOS
- bash: |
    curl https://sh.rustup.rs -sSf | sh -s -- -y --default-toolchain $RUSTUP_TOOLCHAIN
    echo "##vso[task.setvariable variable=PATH;]$PATH:$HOME/.cargo/bin"
  condition: eq( variables['Agent.OS'], 'Darwin' )
  displayName: install rustup

# Windows
- script: |
    curl -sSf -o rustup-init.exe https://win.rustup.rs
    rustup-init.exe -y --default-toolchain %RUSTUP_TOOLCHAIN%
    set PATH=%PATH%;%USERPROFILE%\.cargo\bin
    echo "##vso[task.setvariable variable=PATH;]%PATH%;%USERPROFILE%\.cargo\bin"
  condition: eq( variables['Agent.OS'], 'Windows_NT' )
  displayName: install rustup

My 2c

PS: Funny thing, my crate ffizer is a cli to bootstrap or update project/folder from online templates (maybe some collaboration is possible, I planned to add templates for rust ci).

johnterickson commented 5 years ago

If we can get https://github.com/rust-lang/rustup.rs/issues/559 fixed, then one interesting option we have is to have a "Rustup tool installer" that installs just rustup and not any toolchains. (@epage I think I was getting ahead of myself about putting it in the default image 😊)

steps:
- task: RustupTool@1
- bash: rustup install $RUSTUP_TOOLCHAIN

or once https://github.com/rust-lang/rustup.rs/issues/998 is resolved (I'm guessing at the syntax)

steps:
- task: RustupTool@1
- bash: rustup install --profile minimal $RUSTUP_TOOLCHAIN 

The upside is we won't have to iterate the task in reaction to rustup changes like https://github.com/rust-lang/rustup.rs/issues/998. The downside is that it's two steps instead of one.

epage commented 5 years ago

It looks like https://github.com/rust-lang/rustup.rs/pull/1257 is already fixed rust-lang/rustup.rs#559. We should be able to install without a toolchain unless I'm missing something.

So I guess the question is the trade offs between a minimal RustupTool like you proposed or something more akin to my rustup step in this repo

Let's compare a clippy job with a minimal RustupTool

steps:
- task: RustupTool@1
- bash: rustup install $RUSTUP_TOOLCHAIN
- bash: rustup component add clippy
- bash: cargo clippy 

Now with a theoretical RustupTool based on my step template:

steps:
- task: RustupTool@1
  inputs:
    toolchain: ${{ rustup_toolchain }}
    component: clippy
- bash: cargo clippy

Note that for a build/test job vs a clippy job, instead of specifying component: clippy, we'd have target: ${{ rustup_target }} for those users desiring cross-compilation (generally used by CLIs)

Benefits for minimal RustupTool

Benefits for the step template approach

So it me it looks like a bit of a toss up.

spontoreau commented 5 years ago

@davidB thank you for the feedback!

@johnterickson nice idea :)

@epage I like your step proposal 👍

spontoreau commented 5 years ago

Hello guys,

Some news on my side!

I can't release the YAML version of the current extension because it creates breaking changes on legacy steps (Cargo, Rustup, Rustc). For people who are using the extension it means that they need to resetup all there pipelines. There aren't a lot of developers who install the extension, but I don't want to break something on there side. Also because I'm gonna release a new one which be only focus on tool installer, I will release the last version of the current extension as a legacy one this weekend.

After that's I'm gonna create the new extension based on you're feedback. My goal is to have a first implementation in order to create the feature request inside the azure-pipelines-task repository with an exemple (maybe they will be open for a PR).

I keep you in touch asap!

PS: Also I need your organization name to share the first beta version of the new extension with you!

epage commented 5 years ago

@spontoreau thank you for your work on this! That sounds fully reasonable.

spontoreau commented 5 years ago

@epage version 1.2 of the legacy is released. Time to go further now :)

jfro commented 4 years ago

so ran into this after poking around on any options for a more robust rust solution on Azure Pipelines. Found this via the now archived azure rust devops task project, as i started wondering if it'd be worth building a custom task. we're definitely repeating ourselves a bunch in our CI config but also wanting some better integration.

One thing I notice is when rust code is built via normal script tasks, there's no bridge of warnings/errors into the Azure Pipelines UI which we notice happens when built in a VS project in a separate repo. So could be nice to process the cargo build/clippy output so it's discovered and surfaced to the Azure Pipelines UI.

My other hope is probably more work, but is having the option to take the warnings/errors etc from above and surfacing them into GitHub's Checks API. There's already a GitHub Actions plugin for rust clippy but haven't seen anything that'd help do that when using Azure.

So wondering if that'd warrant exploring doing a task as well, beyond the above discussion of the rust tool installer, but also an featureful build/clippy/test task extension? I may poke around in my free time if I can to see if I can at least figure out clippy to github checks.