Skip to content
New issue

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

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

Already on GitHub? # to your account

Declare CoreClrPgoDataArg variable in CI script #80324

Merged
merged 3 commits into from
Feb 9, 2023
Merged

Conversation

am11
Copy link
Member

@am11 am11 commented Jan 7, 2023

Before:

/Users/runner/work/1/s/src/coreclr/build-runtime.sh checked x64   -ci       $(CoreClrPgoDataArg)
========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /Users/runner/work/_temp/70ee6621-82c1-46c2-9a87-abb17033d481.sh
/Users/runner/work/_temp/70ee6621-82c1-46c2-9a87-abb17033d481.sh: line 1: CoreClrPgoDataArg: command not found
Commencing CoreCLR Repo build

After:

/Users/runner/work/1/s/src/coreclr/build-runtime.sh checked x64   -ci       
========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /Users/runner/work/_temp/1195a75b-c5f1-4c49-9e14-3467f75d6602.sh
Commencing CoreCLR Repo build

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jan 7, 2023
@ghost
Copy link

ghost commented Jan 7, 2023

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Runtime CI legs print this (non-fatal) error:

/Users/runner/work/1/s/src/coreclr/build-runtime.sh checked x64   -ci       $(CoreClrPgoDataArg)
========================== Starting Command Output ===========================
/bin/bash --noprofile --norc /Users/runner/work/_temp/70ee6621-82c1-46c2-9a87-abb17033d481.sh
/Users/runner/work/_temp/70ee6621-82c1-46c2-9a87-abb17033d481.sh: line 1: CoreClrPgoDataArg: command not found
Commencing CoreCLR Repo build

This is because CoreClrPgoDataArg is not passed as parameter:

# Build CoreCLR Runtime
- ${{ if ne(parameters.osGroup, 'windows') }}:
- script: $(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) $(crossArg) $(osArg) -ci $(compilerArg) $(clrRuntimeComponentsBuildArg) $(pgoInstrumentArg) $(officialBuildIdArg) $(clrInterpreterBuildArg) $(clrRuntimePortableBuildArg) $(CoreClrPgoDataArg)
displayName: Build CoreCLR Runtime
- ${{ if eq(parameters.osGroup, 'windows') }}:
- script: $(Build.SourcesDirectory)/src/coreclr/build-runtime$(scriptExt) $(buildConfig) $(archType) -ci $(enforcePgoArg) $(pgoInstrumentArg) $(officialBuildIdArg) $(clrInterpreterBuildArg) $(CoreClrPgoDataArg)

Author: am11
Assignees: -
Labels:

area-Infrastructure-coreclr, community-contribution

Milestone: -

@am11 am11 requested a review from jkoritzinsky January 7, 2023 08:39
@am11
Copy link
Member Author

am11 commented Jan 19, 2023

cc @EgorBo, @jakobbotsch

This is basically silencing the warning which shows up on top of every macOS run. But since CoreClrPgoDataArg is never set in the pipelines, we may as well delete it. Thoughts?

agocke
agocke previously approved these changes Jan 30, 2023
@agocke agocke self-requested a review January 30, 2023 21:06
@agocke agocke dismissed their stale review January 30, 2023 21:07

thought of something else

@agocke
Copy link
Member

agocke commented Jan 30, 2023

Actually, shouldn't this be an actual path? I think it's a NuGet path on some other runs (maybe musl?)

@am11
Copy link
Member Author

am11 commented Jan 30, 2023

@jkoritzinsky might remember the details when it was added (currently it is uninitialized and showing that error in logs from PR description).

@jkoritzinsky
Copy link
Member

We calculate this path in an MSBuild project and set the variable from there when applying PGO instrumentation. This PR provides a default to remove the warning. I think it's a good thing to add. I've added a suggested comment to explain whats going on.

I think I fixed this in my ASAN branch at some point, but I don't remember when.

Copy link
Member

@jkoritzinsky jkoritzinsky left a comment

Choose a reason for hiding this comment

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

LGTM with the suggested comment.

Co-authored-by: Jeremy Koritzinsky <jkoritzinsky@gmail.com>
@jkoritzinsky jkoritzinsky merged commit 46cb4ed into dotnet:main Feb 9, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 12, 2023
@am11 am11 deleted the patch-6 branch June 14, 2024 04:52
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants