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

Use windows agent by default #3

Merged
merged 1 commit into from
Jul 3, 2020
Merged

Use windows agent by default #3

merged 1 commit into from
Jul 3, 2020

Conversation

aminya
Copy link
Member

@aminya aminya commented Jul 3, 2020

To test azure pipeline

@aminya aminya mentioned this pull request Jul 3, 2020
@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

Pasting the error here for posterity:

A task is missing. The pipeline references a task called
'1ESLighthouseEng.PipelineArtifactCaching.RestoreCacheV1.RestoreCache'. This usually
indicates the task isn't installed, and you may be able to install it from the Marketplace:
https://marketplace.visualstudio.com. (Task version 1, job 'Windows', step ''.)

For the record, (as mentioned in #1) I think installing this artifact caching task thing to the atomcommunity Azure DevOps org should get rid of this error message.

@aminya aminya closed this Jul 3, 2020
@aminya aminya reopened this Jul 3, 2020
@aminya
Copy link
Member Author

aminya commented Jul 3, 2020

I installed the after-mentioned package. Let's see

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

I got the same node-gyp rebuild error when building @atom/watcher. So I think that's a genuine bug with the Atom dependencies.

Probably the wrong V8 version/the wrong Node version or wrong Electron version, or something.

@aminya aminya closed this Jul 3, 2020
@aminya aminya reopened this Jul 3, 2020
@aminya
Copy link
Member Author

aminya commented Jul 3, 2020

I got the same node-gyp rebuild error when building @atom/watcher. So I think that's a genuine bug with the Atom dependencies.

Probably the wrong V8 version/the wrong Node version or wrong Electron version, or something.

That seems strange. The upstream repo does not have this issue
https://github.visualstudio.com/9e67709c-1fe5-47f2-8cf6-4902878a11f5/_build/results?buildId=73864

We might have to install some other packages from the marketplace. Might be good to ask the upstream team instead.

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

The CI caches successful npm install results (node_modules) so maybe that's why.


Explanation/research:

I think there's an old @atom/watcher in [repo root]/package-lock.json. Leading to outdated nan which fails on some versions of Node 12. nodejs/nan#849 (comment) nodejs/nan#849 (comment)

This was fixed here: atom/watcher#234 (released in @atom/watcher v1.3.2+. Actually, there is no v1.3.2, so v1.3.3+ I guess?)

But [repo root]/package-lock.json still has an older version of @atom/watcher: https://github.com/atom/atom/blob/aa3c34bedb361e09a5068dce9620b460a20ca3fb/package-lock.json#L55 (v1.3.1).

I can even reproduce this failure on my local machine by running npm install in the repo root (atom/) dir under Node v12.18.2.


So I'm pretty sure it's an upstream Atom bug. Guess it's time to post an issue there... and if we get impatient we can patch it here.

@aminya aminya closed this Jul 3, 2020
@aminya aminya reopened this Jul 3, 2020
@aminya
Copy link
Member Author

aminya commented Jul 3, 2020

In triggers > YAML > the default agent was Ubuntu. I noticed upstream is using Windows, while for us it is shown as Ubuntu.

It is a hosted agent:

Agent name: 'Hosted Agent'
Agent machine name: 'fv-az639'
Current agent version: '2.171.1'

https://github.visualstudio.com/Atom/_build/results?buildId=74130&view=logs&j=0d2f351d-5899-57e2-0cb5-b37eb91cc930&t=e687bea5-6de6-42f9-9697-c2cf69fb992a&l=2

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

Interesting. Well, checking out the git repo and doing npm install should work cross-platform. And then the later steps get run in their specified OSes. I think we can proceed trying to make this work in Ubuntu for now.

Edit: It got past the npm install step under Windows... I don't understand why... But it did. Maybe that missing V8 feature isn't used the same way on Linux as on Windows?

@aminya aminya changed the title test_azure Fix Azure Jul 3, 2020
@aminya aminya linked an issue Jul 3, 2020 that may be closed by this pull request
@aminya
Copy link
Member Author

aminya commented Jul 3, 2020

Interesting. Well, checking out the git repo and doing npm install should work cross-platform. And then the later steps get run in their specified OSes. I think we can proceed trying to make this work in Ubuntu for now.

Edit: It got past the npm install step under Windows... I don't understand why... But it did. Maybe that missing V8 feature isn't used the same way on Linux as on Windows?

That's something to consider for later. I guess it is more important to get this working first 😄

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

I found some docs for getting this to run automatically via closer GitHub integration, rather than just manually.

https://www.azuredevopslabs.com/labs/vstsextend/github-azurepipelines/#task-1-installing-azure-pipelines-from-github-marketplace

I'm not 100% sure but it looks like you may have done part or all of this already.

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

This has to be the docs we're looking for: https://docs.microsoft.com/azure/devops/pipelines/repos/github

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

I figured out why it works in Windows and not Ubuntu.

There are lines like this, with backslashes in some paths:

cd script\vsts

The backslash is treated as an escape character on Ubuntu/macOS, so the cd doesn't happen:

sh: line 1: cd: scriptvsts: No such file or directory

Changing the backslashes to forward-slashes also works, as an alternative to forcing Windows.
https://dev.azure.com/DeeDeeG/b/_build/results?buildId=44&view=logs&j=0d2f351d-5899-57e2-0cb5-b37eb91cc930&t=4bcffbf6-507a-564a-0cc4-d6754c0b024f

(^ This is a successful run of the GetReleaseVersion job, on Ubuntu 16.04.)

I'd like to post an issue/maybe a PR at upstream about this.

@aminya
Copy link
Member Author

aminya commented Jul 3, 2020

@DeeDeeG Awesome! One you made the PR we can merge it here.

@DeeDeeG
Copy link
Member

DeeDeeG commented Jul 3, 2020

Whoops, I just saw your comment. Yeah, trying to get this accepted upstream.

Issue: atom#21020

PR: atom#21021

@aminya aminya changed the title Fix Azure Use windows agent by default Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting up Azure pipelines
2 participants