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.

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

CI: Fix running the workaround for NuGet-Migrations issue #85692

Merged
merged 6 commits into from
May 26, 2023
Merged

CI: Fix running the workaround for NuGet-Migrations issue #85692

merged 6 commits into from
May 26, 2023

Conversation

radical
Copy link
Member

@radical radical commented May 3, 2023

The workaround adds:

(CONSOLE_TEMP_DIR="%24(mktemp -d)" %3B "$DOTNET_ROOT/dotnet" new console -o "$CONSOLE_TEMP_DIR" %3B rm -rf "$CONSOLE_TEMP_DIR") || true

which uses $DOTNET_ROOT/dotnet. But this is set for HelixPreCommand which runs before DOTNET_ROOT is set.

Instead, use HelixCommandPrefixItem.

@radical radical requested review from lewing and akoeplinger May 3, 2023 00:04
@ghost ghost assigned radical May 3, 2023
@ghost
Copy link

ghost commented May 3, 2023

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

The workaround adds:

(CONSOLE_TEMP_DIR="%24(mktemp -d)" %3B "$DOTNET_ROOT/dotnet" new console -o "$CONSOLE_TEMP_DIR" %3B rm -rf "$CONSOLE_TEMP_DIR") || true

which uses $DOTNET_ROOT/dotnet. But this is set for HelixPreCommand which runs before DOTNET_ROOT is set.

Instead, use HelixCommandPrefixItem.

Author: radical
Assignees: -
Labels:

area-Infrastructure-libraries
Milestone: -

@radical radical force-pushed the nuget-migrations-workaround-fix branch from 6f8f7d1 to 9bcb2ba Compare May 3, 2023 01:27
@radical
CI: Fix running the workaround for NuGet-Migrations issue
bafeff5 
The workaround adds:

`(CONSOLE_TEMP_DIR="%24(mktemp -d)" %3B "$DOTNET_ROOT/dotnet" new console -o "$CONSOLE_TEMP_DIR" %3B rm -rf "$CONSOLE_TEMP_DIR") || true`

which uses `$DOTNET_ROOT/dotnet`. But this is set for `HelixPreCommand`
which runs *before* `DOTNET_ROOT` is set.

Instead, use `HelixCommandPrefixItem`.
@radical
rename HelixCommandPrefixItem to HelixCommandPrefixEnvVarItem
663e91f
@radical radical force-pushed the nuget-migrations-workaround-fix branch from 9bcb2ba to 663e91f Compare May 3, 2023 02:47
@radical
Copy link
Member Author

radical commented May 3, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@radical radical marked this pull request as ready for review May 3, 2023 05:01
This was referenced May 3, 2023
Closed
Closed
Closed
@akoeplinger
Copy link
Member

we also have a copy of this logic in src/tests/Common/helixpublishwitharcade.proj

@radical
Copy link
Member Author

radical commented May 3, 2023

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost
Copy link

ghost commented May 3, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

The workaround adds:

(CONSOLE_TEMP_DIR="%24(mktemp -d)" %3B "$DOTNET_ROOT/dotnet" new console -o "$CONSOLE_TEMP_DIR" %3B rm -rf "$CONSOLE_TEMP_DIR") || true

which uses $DOTNET_ROOT/dotnet. But this is set for HelixPreCommand which runs before DOTNET_ROOT is set.

Instead, use HelixCommandPrefixItem.

Author: radical
Assignees: radical
Labels:

area-Infrastructure
Milestone: -

@radical
Copy link
Member Author

radical commented May 3, 2023

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

This was referenced May 3, 2023
Open
Closed
Closed
@radical
Copy link
Member Author

radical commented May 5, 2023

/azp run runtime-wasm,runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@radical
Copy link
Member Author

radical commented May 5, 2023

/azp run runtime-wasm,runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@radical radical requested review from akoeplinger and lewing May 6, 2023 01:51
@radical
Copy link
Member Author

radical commented May 12, 2023

@lewing @akoeplinger could you please take a look at the updated PR?
@akoeplinger where should I check for helixpublishwitharcade.proj's effect on non-wasm jobs? The wasm runtime-tests job is working fine.

@radical
Copy link
Member Author

radical commented May 26, 2023

/azp run runtime-wasm,runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@steveisok steveisok self-requested a review May 26, 2023 19:22
steveisok
steveisok approved these changes May 26, 2023
View reviewed changes
Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

Backport to 7 as well?

@carlossanlop
Copy link
Member

The issue is also happening in 6.0, so if it makes sense to backport this fix all the way back there, please do so, @radical.

Example of affected 6.0 PR: #85725

Mentioning the main issue so this PR gets linked there: #80619

This was referenced May 26, 2023
Closed
Closed
@radical
Copy link
Member Author

radical commented May 26, 2023

@steveisok @carlossanlop could you please keep an eye out for any breakage that this might cause, hopefully none! I will be watching the wasm jobs.

@radical radical merged commit 76e0313 into dotnet:main May 26, 2023
@radical radical deleted the nuget-migrations-workaround-fix branch May 26, 2023 23:50
@ghost ghost locked as resolved and limited conversation to collaborators Jun 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@steveisok steveisok steveisok approved these changes

@lewing lewing lewing approved these changes

@akoeplinger akoeplinger Awaiting requested review from akoeplinger

Assignees

@radical radical

Labels
area-Infrastructure
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@radical @akoeplinger @carlossanlop @lewing @steveisok @ViktorHofer