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.

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

Quote GitVersionFileExe for profile paths with spaces. #2566

Merged
merged 3 commits into from
Jan 28, 2021
Merged

Quote GitVersionFileExe for profile paths with spaces. #2566

merged 3 commits into from
Jan 28, 2021

Conversation

lee-at-work
Copy link
Contributor

@lee-at-work lee-at-work commented Jan 27, 2021

Description

GitVersion.MsBuild 5.6.4 breaks my build because my profile directory includes spaces in it.

Rebuild started...
1>------ Rebuild All started: Project: TestProject.CLI, Configuration: Release Any CPU ------
1>'C:\Users\abc' is not recognized as an internal or external command,
1>operable program or batch file.
1>C:\Users\abc 123\.nuget\packages\gitversion.msbuild\5.6.4\tools\GitVersion.MsBuild.targets(9,9): error MSB3073: The command "C:\Users\abc 123\.nuget\packages\gitversion.msbuild\5.6.4\tools\net48/gitversion.exe "c:\Builds\git.repos\TestProject.CLI\TestProject.CLI" -output file -outputfile obj\gitversion.json" exited with code 9009.
1>Done building project "TestProject.CLI.csproj" -- FAILED.
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========

Related Issue

Fixes #2555 Fixes #2540 #2541 (Duplicate of #2558)

Motivation and Context

It solves profile paths with spaces because my username on my machine has spaces.

How Has This Been Tested?

Manually modified the file GitVersion.MsBuild.targets inside the NuGet Packages folder.

  • Tested with Visual Studio 2019
  • Tested with dotnet build

Screenshots (if appropriate):

Hmm...

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@lee-at-work
Copy link
Contributor Author

CC: @asbjornu

@asbjornu
Copy link
Member

The tests are breaking, have you investigated why?

@bddckr
Copy link
Contributor

bddckr commented Jan 27, 2021

Exact same change in #2558. I suppose both have the same blocker for merging: The tests are broken by this and haven't been updated.

@asbjornu
Copy link
Member

Yeah, we'll have to investigate why that suddenly started failing, @bddckr.

@bddckr
Copy link
Contributor

bddckr commented Jan 27, 2021

Oh it's just a test failure really:

The command ""dotnet --roll-forward Major "/root/.nuget/packages/gitversion.msbuild/5.7.0-pullrequest2566-136/tools/netcoreapp3.1/gitversion.dll"" "/repo/tests/integration/core" -output file -outputfile obj/gitversion.json" exited with code 127. [/repo/tests/integration/core/app.csproj]

That is due to the double quoting now I think.

I wanted to suggest just moving the quotations this fix adds to the definition of the property instead of its usage, but that feels wrong. The property might not always be used in a way that it should be quoted.

<GitVersionFileExe>$(MSBuildThisFileDirectory)net48/gitversion.exe</GitVersionFileExe>
<GitVersionAssemblyFile>$(MSBuildThisFileDirectory)net48/GitVersion.MsBuild.dll</GitVersionAssemblyFile>
</PropertyGroup>
<PropertyGroup>
<GitVersionFileExe Condition="'$(GitVersionFileExe)' == ''">dotnet --roll-forward Major &quot;$(MSBuildThisFileDirectory)netcoreapp3.1/gitversion.dll&quot;</GitVersionFileExe>

Either that's fine, or perhaps we could introduce another property like GitVersionCommandPrefix or something.

@lee-at-work
Copy link
Contributor Author

I'll see if i can get the tests fixed appropriately.

@arturcic
Copy link
Member

@LeeShape thank you so much for your contributions 👍

@arturcic
Copy link
Member

Fixes #2558

@arturcic arturcic mentioned this pull request Jan 28, 2021
5 tasks
@lee-at-work
Copy link
Contributor Author

Thanks a lot for looking into this PR and get it merged so quickly! ❤️

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