Skip to content

Conversation

@dougbu
Copy link
Contributor

@dougbu dougbu commented Sep 27, 2021

  • Add source index build step #33534
  • use 1ES agents because default VS is incompatible with our .NET SDK version
    • windows-latest might work but prefer to protect binary log
  • wait to source-index for other jobs
  • use binary log from Linux_x64 build
    • we're waiting for that build anyhow
  • only source index the 'main' branch
    • anything else would be redundant

nits:

  • remove unnecessary backslashes in the Linux_x64_build job's scripts
    • instead separate commands with a semicolon
  • confirm test build success before BAR updates in manual builds

@dougbu dougbu added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Sep 27, 2021
Copy link
Member

Choose a reason for hiding this comment

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

This list is a little spooky, it seems likely people won't know to update it if/when our set of jobs changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Been this way forever unfortunately. AzDO schema doesn't provide a way to automagically wait for almost every other job.

Main mitigation is @dotnet/aspnet-build are automagically reviewers whenever a .azure/ file is changed.

Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

LGTM other than the comment

@dougbu
Copy link
Contributor Author

dougbu commented Sep 27, 2021

Fortunately, my typo was in the last commit and didn't invalidate internal build 20210927.7

@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from 7838407 to 90a72d6 Compare September 27, 2021 20:37
@dougbu
Copy link
Contributor Author

dougbu commented Sep 27, 2021

New internal (real test) build is 20210927.10

@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from 90a72d6 to 6e94598 Compare September 28, 2021 03:46
@dougbu
Copy link
Contributor Author

dougbu commented Sep 28, 2021

Sorry to those playing along for squashing all prior commits on you.

New test build is 20210927.27

@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from 6e94598 to a138fbb Compare September 28, 2021 05:32
@davidfowl
Copy link
Member

image

This link is currently broken on http://source.dot.net

@dougbu
Copy link
Contributor Author

dougbu commented Sep 28, 2021

The approach of using a binary log from another job isn't going to work at all. The processing seems to require generated files.

Unhandled exception. System.IO.DirectoryNotFoundException: Could not find a part of the path 'D:\a\_work\1\s\artifacts\obj\Microsoft.Extensions.Caching.SqlServer\Release\net7.0\.NETCoreApp,Version=v7.0.AssemblyAttributes.cs'.

My main idea is to build and process the binary log in one job then do the "Upload stage1 artifacts to source index" part using a new artifact from the first job (after the rest of the build completes successfully). The main downside will be not keeping us up-to-date w/ eng/common/templates/job/source-index-stage1.yml. Ideal would be a step template or two from Arcade that installs and runs BinLogToSln (no need for UseDotNet@2 or UploadIndexStage1 there) and another job or step template containing the UploadIndexStage1 part. But, I could copy the bits and pieces over for now.

Thoughts @alexperovich @davidfowl @wtgodbe❔ Other suggestions❔

@alexperovich
Copy link
Member

would this just be downloading a new artifact from the first build? Would the preSteps parameter not work for this?

@dougbu
Copy link
Contributor Author

dougbu commented Sep 28, 2021

would this just be downloading a new artifact from the first build? Would the preSteps parameter not work for this?

Since there's no reason not to proactively run BinLogToSln, my thought was to do that in the first job (together w/ a regular build) then only run UploadIndexStage1 later, after everything else succeeds.

Just using preSteps would require a new artifact containing at least all artifacts/**/*.cs. That wouldn't be too gigantic an artifact but I'm not sure how many other types of files we'd need nor whether we would need generated files from elsewhere in the sources directory.

@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from a138fbb to 7023165 Compare September 29, 2021 17:04
@dougbu
Copy link
Contributor Author

dougbu commented Sep 29, 2021

Have the C# upload / download working now, I hope. Latest test build is https://dev.azure.com/dnceng/internal/_build/results?buildId=1393133&view=results

This time, the exact internal build commit is present here on GitHub too. I just added the main-branch-only commit here. That should help your checks @davidfowl (assuming the test build works).

Comment on lines +290 to +287
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The key word here is "runtime" i.e. the value of artifact.path for the Windows_arm_csharp artifact is $(Build.ArtifactStagingDirectory), not e.g. D:\a\_work\1\a.

@alexperovich
Copy link
Member

I don't have a problem with running BinLogToSln in a separate job, I can update the code in arcade once we get a design that works for aspnetcore.

@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from 7023165 to ebbcd42 Compare September 29, 2021 22:26
- #33534
- start source indexing after other jobs
  - do not update https://source.dot.net/ from failed builds
- use 1ES agents
  - `windows-latest` might work but prefer to protect binary log
- use Windows_arm_build binary log
  - align binary log with source-indexing agent i.e. always use Windows
  - much faster than Windows x64/x86 build
  - provide another binary log, even in internal builds

nits:
- remove unnecessary backslashes in the Linux_x64_build job's `script`s
  - instead separate commands with a semicolon
- confirm test build success before BAR updates in manual builds
- merge a couple of `in(...)` expressions
dougbu added 6 commits October 1, 2021 17:45
- required for source-indexing from the binary log
- this approach is taking us down a rabbit hole

This reverts commit 033ba5696bbc545538bcf9172825a70e5e7bff49.
- this approach is taking us down a rabbit hole

This reverts commit 6426a6195972dc778de4552eb70a978f1ed57adf.
- current template not well suited to reuse a binary log from another job
@dougbu dougbu force-pushed the dougbu/source.indexing.33534 branch from ebbcd42 to 9721e92 Compare October 2, 2021 04:23
@dougbu
Copy link
Contributor Author

dougbu commented Oct 2, 2021

Very different than previous iterations but working 🎉 Build 20211001.17 was successful and uploaded what looks like a complete set of sources.

Due to how I organized the template expressions, the commits here are identical. https://source.dot.net/ should have no broken links. But, right now, it's using the now-lost commit from my first (prototype) successful run.

- name: testSourceIndexing
default: false
displayName: Test source indexing? !Danger! see comments in YAML.
type: boolean
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This parameter is visible in the AzDO UI but, as mentioned in the comments, ignored in public builds.

@dougbu
Copy link
Contributor Author

dougbu commented Oct 4, 2021

@alexperovich, @wtgodbe is OOF now. I'd appreciate you double-checking this PR before I merge it. Thanks…

@dougbu dougbu merged commit fc286da into main Oct 4, 2021
@dougbu dougbu deleted the dougbu/source.indexing.33534 branch October 4, 2021 20:54
@ghost ghost added this to the 7.0-preview1 milestone Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants