Skip to content

Normalize RepoRoot to include trailing slash - #7498

Merged
ViktorHofer merged 11 commits into
mainfrom
ViktorHofer-patch-2
Jun 10, 2021
Merged

Normalize RepoRoot to include trailing slash#7498
ViktorHofer merged 11 commits into
mainfrom
ViktorHofer-patch-2

Conversation

@ViktorHofer

Copy link
Copy Markdown
Member

The Arcade SDK defines the RepoRoot repository with a trailing slash but the build scripts (build.ps1 and build.sh) pass in the property without a trailing slash which makes the use of it inconsistent and led to this source build patch: https://github.com/dotnet/runtime/blob/23e4735e5cc9956d3f3b351a252f0b1622a14cec/eng/source-build-patches/0004-Add-trailing-path-separator-to-repo_root.patch.

Normalizing the variable in the build scripts so that the property always contains the trailing slash.

To double check:

The Arcade SDK defines the RepoRoot repository [with a trailing slash](https://github.com/dotnet/arcade/blob/e81d8c9bdc1de22623657afb23fab210dd04ca82/src/Microsoft.DotNet.Arcade.Sdk/tools/RepoLayout.props#L23) but the build scripts (build.ps1 and build.sh) pass in the property without a trailing slash which makes the use of it inconsistent and led to this source build patch: https://github.com/dotnet/runtime/blob/23e4735e5cc9956d3f3b351a252f0b1622a14cec/eng/source-build-patches/0004-Add-trailing-path-separator-to-repo_root.patch.

Normalizing the variable in the build scripts so that the property always contains the trailing slash.

@MichaelSimons MichaelSimons left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for fixing this.

@ViktorHofer
ViktorHofer merged commit a419173 into main Jun 10, 2021
@ViktorHofer
ViktorHofer deleted the ViktorHofer-patch-2 branch June 10, 2021 15:41
@ghost

ghost commented Jun 10, 2021

Copy link
Copy Markdown

@ViktorHofer This seems to have broken the official build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1181292&view=results

@ViktorHofer

Copy link
Copy Markdown
Member Author

Oh no :( Should I send a follow-up change?

@ghost

ghost commented Jun 10, 2021

Copy link
Copy Markdown

I haven't had a chance to dig in just yet, so if it's obvious please do, if not we can revert while we figure it out

@ghost

ghost commented Jun 10, 2021

Copy link
Copy Markdown

OOOh, actually, let's hold for a minute. I'm seeing this exact same error in other places, which makes me think it's not the change.

@ghost

ghost commented Jun 10, 2021

Copy link
Copy Markdown

oh, nevermind, it's definitely this change, I forgot for a minute that the publishing pipeline uses arcade main. We need to revert this change asap because it's failing publishing for everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants