Skip to content

Run dotnet.sh/cmd script before running dotnet restore #48375

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.

Sign up for GitHub

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

Jump to bottom
Merged
BruceForstall merged 2 commits into from
Feb 18, 2021
Merged

Run dotnet.sh/cmd script before running dotnet restore #48375

BruceForstall merged 2 commits into from
Feb 18, 2021

Conversation

@BruceForstall
Copy link
Contributor

@BruceForstall BruceForstall commented Feb 17, 2021

No description provided.

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Feb 17, 2021
@BruceForstall
Copy link
Contributor Author

BruceForstall commented Feb 17, 2021

Issue: #48376

kunalspathak
kunalspathak reviewed Feb 17, 2021
View reviewed changes
src/coreclr/scripts/superpmi_setup.py Outdated
Copy link
Contributor

@kunalspathak kunalspathak Feb 17, 2021

Choose a reason for hiding this comment

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

I now realized that the error could be because we are not running bootstrap inside jitutils_directory. I deleted _cwd from run_command, but I guess it should be there and we should continue passing jitutils_directory as current working directory.

Copy link
Contributor Author

@BruceForstall BruceForstall Feb 17, 2021

Choose a reason for hiding this comment

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

oh; I deleted this arg because it was "extra", but maybe the cwd should be there? But I still think we should invoke the shell script first to ensure the .dotnet directory is there.

Copy link
Contributor

@kunalspathak kunalspathak Feb 17, 2021

Choose a reason for hiding this comment

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

Yes, cwd should be there so we can run it inside jitutils_directory. That should fix the problem.

And yes, we can run dotnet --info just to be sure.

@BruceForstall BruceForstall force-pushed the AttemptLinuxCollectionFix branch from 16ffb2c to 1b8d37d Compare February 17, 2021 08:04
kunalspathak
kunalspathak approved these changes Feb 18, 2021
View reviewed changes
Copy link
Contributor

@kunalspathak kunalspathak 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 minor changes needed.

src/coreclr/scripts/superpmi_setup.py Outdated
Copy link
Contributor

@kunalspathak kunalspathak Feb 18, 2021

Choose a reason for hiding this comment

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

Missing comma: _cwd=None,

src/coreclr/scripts/superpmi_setup.py Outdated
Copy link
Contributor

@kunalspathak kunalspathak Feb 18, 2021

Choose a reason for hiding this comment

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

typo: _cwd

@kunalspathak kunalspathak mentioned this pull request Feb 18, 2021
Closed
@BruceForstall
Fix SuperPMI Linux collections
50ef303 
1. Run dotnet for jitutils with jitutils dir as the current working dir.
2. Run dotnet.sh/cmd script before running bootstrap (and `dotnet restore`)
to ensure `.dotnet` directory exists.
@BruceForstall
Fix typos
18dc018
@BruceForstall BruceForstall force-pushed the AttemptLinuxCollectionFix branch from 1b8d37d to 18dc018 Compare February 18, 2021 08:52
@BruceForstall
Copy link
Contributor Author

BruceForstall commented Feb 18, 2021

Triggered superpmi collection https://dev.azure.com/dnceng/internal/_build/results?buildId=1000601&view=results

@BruceForstall BruceForstall merged commit 20bba79 into dotnet:master Feb 18, 2021
@BruceForstall BruceForstall deleted the AttemptLinuxCollectionFix branch February 18, 2021 19:10
@kunalspathak kunalspathak mentioned this pull request Feb 18, 2021
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@kunalspathak kunalspathak kunalspathak approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BruceForstall @kunalspathak