Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature] Detect Azure Pipelines #23

Merged
merged 2 commits into from
Dec 5, 2018
Merged

Conversation

vtbassmatt
Copy link
Contributor

Add detection for Azure Pipelines.

Azure Pipelines is the new name for both TFS Build and VSTS Build. Detection is a little messy because we intentionally don't make it obvious whether you're running on-premises (Azure DevOps Server) or hosted (Azure DevOps Services). I left TF_BUILD as the indicator for TFS, and did URL path-math on our new domain name to check for Azure Pipelines.

@sibiraj-s sibiraj-s changed the title address #7 and #22 [Feature] Detect Azure Pipelines Sep 26, 2018
Copy link
Owner

@watson watson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing.

I can't find the two environment variables you use listed at https://docs.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=vsts, but I assume they are ok?

Besides that, I just have one tiny change request (see comment), otherwise it looks good 👍

index.js Outdated
@@ -54,6 +54,8 @@ exports.isCI = !!(
env.CONTINUOUS_INTEGRATION || // Travis CI, Cirrus CI
env.BUILD_NUMBER || // Jenkins, TeamCity
env.RUN_ID || // TaskCluster, dsari
// Azure Pipelines
(env.SYSTEM_TEAMFOUNDATIONCOLLECTIONURI && env.SYSTEM_TEAMFOUNDATIONCOLLECTIONURI.startsWith('https://dev.azure.com')) ||
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just for generic environment variables used for many different CI systems. Because you added these two to vendors.json, the exports.name check will be truthy and so there's no need to add them here as well 😃

@vtbassmatt
Copy link
Contributor Author

Sorry for the confusion! The reason you don't see them on that page is they're spelled differently. We have non-environment-variable contexts where you say things like "System.TeamFoundationCollectionUri". That gets translated to "SYSTEM_TEAMFOUNDATIONCOLLECTIONURI" as an env var.

I will also follow your suggestion - didn't realize I was breaking stuff :)

also mark "TFS" as deprecated since going forward, it's Azure Pipelines
@watson
Copy link
Owner

watson commented Sep 26, 2018

No worries.

Quick question: Do you think this PR will also fix issue #7? I've been trying to look into the differences between Team Foundation Server, Visual Studio Team Services and Azure Pipelines previously, but thought it was basically all just Team Foundation Server underneath - but maybe not?

@vtbassmatt
Copy link
Contributor Author

Yeah, this should address #7 as well. Here's the story: TFS and VSTS are largely the same source code, but they're built and shipped completely differently. Most customer-observable behaviors, especially on CI agents, are identical. "Azure DevOps" is more than just a rebrand, but for purposes of this library, it's essentially a rebrand of VSTS. "Azure DevOps Server" is the new name for future versions of TFS.

@sibiraj-s
Copy link
Collaborator

sibiraj-s commented Sep 26, 2018

Maybe after Sept-10-2018, https://azure.microsoft.com/en-us/blog/introducing-azure-devops/ ?

@watson
Copy link
Owner

watson commented Sep 30, 2018

@vtbassmatt Thanks for explaining.

Are there any ways to differentiate between TFS, VSTS and Azure DevOps Server by looking at environment variables (or other things) or do you recommend that this tool just classifies it all as the same?

If we should classify these as the same, I assume we should choose call it "Azure Pipelines" as you suggests with this PR?

@vtbassmatt
Copy link
Contributor Author

I recommend they all get classified the same. In fact, I almost removed TFS with my PR, but I wasn't sure who it might break downstream.

Sorta by design, you can't easily detect which environment you're running against. You could do path-checking on a few of the variables, but that's fragile especially considering we just changed domains recently :)

@watson
Copy link
Owner

watson commented Oct 1, 2018

If a user is running their own TFS/VSTS environment I think they would be confused if we classified that as an Azure environment. Though the other way around might not be as confusing - so if you're running on Azure Pipelines, but gets told that it's actually TFS, then you might just accept that as an internal implementation detail. What do you think about that?

@vtbassmatt
Copy link
Contributor Author

We're renaming TFS to Azure DevOps Server with the next version so there will likely be that confusion anyhow 😋. Let's go with Azure Pipelines.

@watson
Copy link
Owner

watson commented Oct 1, 2018

Could we call it "Azure DevOps Server" then, or is that confusing for people using "Azure Pipelines" you think? I guess it also depends on the percentages of people using this to detect CI. If 95% of people will be using Azure Pipelines you could argue that it makes sense to default to that 😅

@vtbassmatt
Copy link
Contributor Author

vtbassmatt commented Oct 2, 2018 via email

@watson
Copy link
Owner

watson commented Oct 2, 2018

Ok, let's go with this then and see if people complain. Thanks for being patient with all my questions 😄

Will the TF_BUILD environment variable also be available in Azure Pipeline?

If so, the logic for detecting the right CI system might be confused and choose TFS instead of Azure Pipelines based on how the detection algorithm currently works. In that case it's probably best to make this a breaking change.

@vtbassmatt
Copy link
Contributor Author

Yes, TF_BUILD exists on both. I'm all for making this a breaking change.

@DSchau
Copy link

DSchau commented Nov 26, 2018

Any chance someone could give this another set of 👀? We could really use this in gatsbyjs/gatsby#10140

Thank you!

@watson watson merged commit 4534b69 into watson:master Dec 5, 2018
@watson
Copy link
Owner

watson commented Dec 5, 2018

Sorry, dropped the ball on this one 😅

@DSchau
Copy link

DSchau commented Dec 5, 2018

@watson it happens! Thanks so much for merging.

Can you let us know when it's published?

@watson
Copy link
Owner

watson commented Dec 5, 2018

Will do

@watson
Copy link
Owner

watson commented Dec 5, 2018

Support for Azure Pipelines has just been released in v2.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants