CI: Set vstsFeed with an environment variable

[DeeDeeG]

Trying #13 again.

Instructions for setting this up:

Visit "https://feeds.dev.azure.com/${YOUR_ORGANIZATION_HERE}/_apis/packaging/feeds"
to find the "fullyQualifiedId" of your organization's
default artifact feed. That value is what you should set
the "ARTIFACT_FEED" environment variable to in Azure DevOps UI.

(IDs are formatted like this: "af6c77da-ef34-bcde-daaf-e2b489a23b2c")

The "description" for the feed should say "Default feed for ${organization}",
e.g. "Default feed for atomcommunity". If the description says something else,
or "null", then it's the wrong feed.

Requirements for Contributing a Bug Fix

Identify the Bug

#1 (comment)

#11 (comment)

Description of the Change

Rather than hard-coding upstream Atom's feed ID, set it with an environment variable. We can configure this in Azure DevOps so that our environment variable points to our fork's (technically our Azure DevOps oganization's) artifact feed.

Having this artifact feed set up properly can save about 10 minutes per OS.

Alternate Designs

• Just hard-code our ID in place of upstream's.

• We could also make separate .yml config files for our CI entirely I guess, and stop using the original ones. This avoids the possibility of merge conflicts. But I think that's not worth the hassle and the mess of files it would make. Upstream ahrdly ever updates their CI config, and I think we would likely want to track their changes if they do.

Possible Drawbacks

Some people don't like to save time. (Just kidding).

I guess with caching you have to make sure it updated when you want it to. So keep an eye out that it doesn't restore a cache when it shouldn't (i.e. when something in the repo changed and node_modules needs to be rebuilt).

Verification Process

Successful node_modules restoration on my personal fork: https://dev.azure.com/DeeDeeG/b/_build/results?buildId=67&view=logs&j=4fbced83-508e-5fe0-c978-5c71ec0fc506&t=96ecc7f1-cdb5-518e-2cb8-d9226cce9313

Release Notes

N/A

[aminya]

Didn't I merge this before in 82a8377 or these are the remaining ones?

[DeeDeeG]

Whoops, yes you did merge this.

I just got confused with all the activity at upstream and here at the same time. A lot of branches to read the histories of. This can be closed, I suppose. Though I'm going to watch if it's working properly yet, so I'll leave this open to see if the cache gets restored or not.

Will want to discuss fixing it if it's not working yet.

Looking at how we manage pulling in upstream changes in #28

[DeeDeeG]

On the topic of making sure CI works 100%:

vscode-ripgrep failed to download. https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=74&view=logs&j=e2cf4b02-5697-54ad-cf7c-fc2a840d53af&t=7b322bfb-2315-51bc-a379-d2aa2470ae6d&l=239

Maybe we don't have a valid GITHUB_TOKEN set on the "Release branch build" Pipeline.

If we do have a valid token, then maybe we are running too many CI runs.

Edit: I just I saw this on my own personal fork too, even though I know I have a valid token, so there's no obvious explanation other than it's not using the token for some reason, or macOS is getting rate limited for an unknown reason despite using the token.

[aminya]

I think this still needs to be merged because the diff is not empty! I think you had forgotten these in the first PR.

[DeeDeeG]

If you expand the diff you can see every instance of vstsFeed is in the diff. It's the same initial change all over again.

Apologies for the messy situation. It's a quirk of complex branching that has been going on. I kind of didn't know what to rebase this work on (not sure I strictly needed to rebase, come to think of it) because upstream master has changed since we merged their stuff in as well. So I compromised by rebasing on the most recent shared ancestor of upstream master and our master.

I rebased this on upstream master where this change isn't landed. So it must be showing the diff versus my commit's ancestor... as opposed to the final merge to this repo, which wouldn't change anything beside tacking on another effectively empty commit to the commit history. Maybe I should rebase again on our master, but then I'd have to do "--allow-empty" I think!

[aminya]

I rewrote master and reverted #13. You can now include all the ARTIFACT_FEEDs in this PR.

[DeeDeeG]

Upstream did something a bit odd that breaks our Pull Requests pipeline at the "checkout the git repo" step.

https://dev.azure.com/atomcommunity/atomcommunity/_build/results?buildId=80&view=logs&j=0d2f351d-5899-57e2-0cb5-b37eb91cc930&t=70660e94-83e1-50ea-1291-f0fa8e688aaf

It's trying to set up git submodules up for a dummy repo they recently added. (It's used as part of their test suite for automated dependency bumps, I think.)

This error isn't happening on upstream's CI for some reason:

https://github.visualstudio.com/Atom/_build/results?buildId=74890&view=logs&j=0d2f351d-5899-57e2-0cb5-b37eb91cc930&t=70660e94-83e1-50ea-1291-f0fa8e688aaf&l=1276

We might have some different config than theirs. Maybe they have disabled submodule syncing. Or maybe submodule syncing doesn't happen on Windows Server 2016; (We are using Windows Server 2019 after #3.)

[aminya]

I saw sub-module settings in the Azure. Let me send you an email with Azure credentials, so you check it out.

[DeeDeeG]

You should be able to invite "DeeDeeG" to the Azure DevOps organization. (I think?)

[DeeDeeG]

I think the setting is here:

• Visit the DevOps project at dev.azure.com/{organization}/{project}
• Press "Pipelines" from sidebar area
• Press a specific pipieline
• Press "Edit" button
• Press vertical three dots button "..."
  • Press "Triggers" from the dropdown menu
• Click over to the "YAML" tab/section of the settings
• Press the "Get Sources" step to view its settings
• Scroll down to the various checkboxes
  • Uncheck "Checkout Submodules"
  • FWIW, only "Report build status" is checked on my personal fork.

[DeeDeeG]

Off-topic a bit:

While you're in the secret hidden settings menu, (via the "Triggers" button), it might be good to adjust when each pipeline runs:

• Click over to the "Triggers" tab again
• For the "Release Branch Build" pipeline, press under "Pull request validation"
  • Check the box next to "Override the YAML pull request trigger from here"
    • Select the radio button next to "Disable pull request validation"
• For the "Pull requests" pipeline, press under "Continuous integration"
  • Check the box next to "Override the YAML continuous integration trigger from here"
    • Select the radio button next to "Disable continuous integration"
• For the "Nightly Release" pipeline, I did the following; Press "Add" next to "Scheduled":
  • Check the boxes for all days of the week, Monday through sunday
  • Set time to 0 hours 0 minutes (midnight).
  • Check the box next to "Only schedule builds if the source or pipeline has changed"
  • Set "Branch filters" to [include] [master]
  • Disable both "Continuous integration" and "Pull request validation" triggers

[aminya]

Does this include all the variables now?

[aminya]

I think the setting is here:

• Visit the DevOps project at dev.azure.com/{organization}/{project}

• Press "Pipelines" from sidebar area

• Press a specific pipieline

• Press "Edit" button

• Press vertical three dots button "..."

  • Press "Triggers" from the dropdown menu

• Click over to the "YAML" tab/section of the settings

• Press the "Get Sources" step to view its settings

• Scroll down to the various checkboxes

  • Uncheck "Checkout Submodules"
  • FWIW, only "Report build status" is checked on my personal fork.

Thanks for the docs.

These are the settings for the release branch

For pull requests it is like this:

Are these what we want?

[DeeDeeG]

Beside the settings in my two comments above, I have these variables:

• ARTIFACT_FEED
  • set from my fork here: https://feeds.dev.azure.com/DeeDeeG/_apis/packaging/feeds
    • for atomcommunity it would be here: https://feeds.dev.azure.com/atomcommunity/_apis/packaging/feeds
  • see the value for fullyQualifiedID of the feed with description: Default feed for ${organization}
• GITHUB_TOKEN
  • Set from a personal access token I generated here: https://github.com/settings/tokens
  • All permissions boxes unchecked. Should just be enough to access the GitHub API, that's it.

---

I also had these, which were necessary before #9 was merged:

• ATOM_MAC_CODE_SIGNING_DOWNLOAD_URL
• ATOM_MAC_CODE_SIGNING_CERT_PATH
• ATOM_WIN_CODE_SIGNING_DOWNLOAD_URL
• ATOM_WIN_CODE_SIGNING_CERT_PATH
  • All of these were defined, but set as empty. To skip code signing on mac & Windows. (unfortunately, CI still tried to notarize on mac, which only works if you've successfully code-signed already. So mac CI was still somewhat broken with these set until Makes code-sign optional  #9 was merged. None of them are needed with Makes code-sign optional  #9 though.)

[DeeDeeG]

• Checkout Submodules

^ I think that's what's breaking the Pull Requests pipeline. I would say try unchecking it. I expect Pull Requests pipeline to work again after that.

(Everything else in your screenshot matches my personal fork.)

[aminya]

Can't we set these submodule settings from inside our ymls? Going through the GUI is a bit awkward.

[aminya]

Yes it is possible: https://docs.microsoft.com/en-us/azure/devops/pipelines/process/resources?view=azure-devops&tabs=schema#schema-7

[DeeDeeG]

We can. I never expect upstream to take those commits, so it's more that we carry around/maintain here if we commit it to the repo.

And yes, the GUI is very awkward. Hmm. It's a trade-off, I could support going either way. (commit the settings in yml, or continue to use the UI to override).

For my fork, I think I have it set up the way I like it (a way that works), so I don't expect to need to touch it very often.

[aminya]

We can try that first with upstream. If they accepted it, we will merge it here as well

[DeeDeeG]

FWIW I'm not sure why submodules was checked. Probably a mis-click. It is off by default.

Upstream tried to set the PR pipeline to only run on PRs, but maybe the # comment mark messed the line up? https://github.com/atom/atom/blob/110b05baf7cb1e50ad7d679173e6fc8f698cab0d/script/vsts/pull-requests.yml#L1

(inline comments like that are valid YAML, but maybe Microsoft's tools get tripped up on it anyhow?)

[aminya]

FWIW I'm not sure why submodules was checked. Probably a mis-click. It is off by default.

Not sure. It is possible that I have clicked it by mistake.

[DeeDeeG]

If we could get upstream to export their whole config, that would be amazing. So long as they could do so without exporting secret variables.

[aminya]

Could you fix the conflicts so I can merge this? Thanks for your contribution!

[DeeDeeG]

This is totally incompatible with #30, as #30 deletes/replaces the old caching tasks, and this PR #27 simply modifies the old caching tasks.

We can use one or the other approach, I don't have a strong opinion which. Though I do like Cache@v2 better (shiny/new, I read docs and worked hard to understand/implement it here) but they each have trade-offs. I don't think the speed is much different between them, Cache@v2 seems a bit faster, but as mentioned before, it doesn't save any cache immediately after bootstrap; It saves cache after the whole job, so when build/tests have succeeded.

And we want to propose one of these to upstream (or propose both and see what they prefer). So if upstream goes for one we should go with what they do I reckon?

[DeeDeeG]

With #30 merged, the easiest thing to do here is close this PR.

The only option to go forward with this PR is to revert #30 first.

[aminya]

Oh, I see. I thought this is still relevant. I will close this in favor of #30