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

Make windows_buildenv.bat work with spaces in path #4896

Merged
merged 1 commit into from
Aug 18, 2022

Conversation

fwolter
Copy link
Contributor

@fwolter fwolter commented Aug 15, 2022

If the Windows user name contains a space (and so the user directory), the execution of windows_buildenv.bat failed. This fixes it.

@@ -150,7 +150,7 @@ EXIT /B 0

:READ_ENVNAME
ECHO Reading name of prebuild environment from "%MIXXX_ROOT%\packaging\windows\build_environment"
SET /P BUILDENV_NAME=<%MIXXX_ROOT%\packaging\windows\build_environment
SET /P BUILDENV_NAME=<"%MIXXX_ROOT%\packaging\windows\build_environment"
SET BUILDENV_NAME=!BUILDENV_NAME:PLATFORM=%PLATFORM%!
SET BUILDENV_NAME=!BUILDENV_NAME:CONFIGURATION=%CONFIGURATION%!
SET RETVAL=%BUILDENV_NAME%
Copy link
Member

Choose a reason for hiding this comment

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

Do we need quotes here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, otherwise the batch file demands a user input.

@daschuer
Copy link
Member

Thank you. CI is still green. However it would be nice to have a look from a windows user before merge.

@JoergAtGithub
@qgazq

Can you confirm this is still working for you?

@daschuer
Copy link
Member

Can this go to the 2.3 branch?

@JoergAtGithub
Copy link
Member

JoergAtGithub commented Aug 17, 2022

I tried this, but it fails in a strange way:

Version from Main branch:

C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\tools>windows_buildenv.bat
Reading name of prebuild environment from "C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\packaging\windows\build_environment"
Environment name: mixxx-deps-2.4-x64-windows-56c7437
Build environment path: C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437
Environment Variables:
- VCPKG_ROOT='C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437'
- CMAKE_GENERATOR='Ninja'
Generating "CMakeSettings.json"...
CMakeSettings.json already exists, creating backup at "CMakeSettings_2022-08-17_17-09-04.json"...
Drücken Sie eine beliebige Taste . . .

Version from this PR:

C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\tools>windows_buildenv.bat
Reading name of prebuild environment from "C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\packaging\windows\build_environment"
Environment name: mixxx-deps-2.4-x64-windows-56c7437
Build environment path: C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437
Environment Variables:
- VCPKG_ROOT='C:\Users\Joerg\source\repos\JoergAtGithub\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437'
- CMAKE_GENERATOR='Ninja'
Generating "CMakeSettings.json"...
CMakeSettings.json already exists, creating backup at "CMakeSettings_2022-08-17_17-09-53.json"...
Das Sprungziel - AddCMakeVar2CMakeSettings_JSON wurde nicht gefunden.
Syntaxfehler.

The download and the unzipping (using 7zip) worked fine, but the CMakeSettings.json output is incomplete:
CMakeSettings.json_Main.txt
CMakeSettings.json_PR4896.txt

@fwolter
Copy link
Contributor Author

fwolter commented Aug 17, 2022

That is really weird. The JSON file is generated until FFMPEG and then, out of a sudden, AddCMakeVar2CMakeSettings_JSON can't be found anymore? When I run this PR's batch file, it looks like this (Powershell and cmd.exe, with a space in the path and without)

C:\Users\Fabian Wolter\git\mixxx\tools>windows_buildenv.bat
Reading name of prebuild environment from "C:\Users\Fabian Wolter\git\mixxx\packaging\windows\build_environment"
Environment name: mixxx-deps-2.4-x64-windows-56c7437
Build environment path: C:\Users\Fabian Wolter\git\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437
Environment Variables:
- VCPKG_ROOT='C:\Users\Fabian Wolter\git\mixxx\buildenv\mixxx-deps-2.4-x64-windows-56c7437'
- CMAKE_GENERATOR='Ninja'
Generating "CMakeSettings.json"...
CMakeSettings.json already exists, creating backup at "CMakeSettings_2022-08-17_21-17-25.json"...
Drücken Sie eine beliebige Taste . . .

There's something fishy about the reported "Syntaxfehler". Is your windows_buildenv.bat corrupted maybe?

@JoergAtGithub
Copy link
Member

I found the issue: My file had Unix line ends. I didn't checked out the whole PR, just downloaded this file. I guess this was my failure.
With correct line ends, it works as expected!

@daschuer
Copy link
Member

Thank you for testing.

Did you consider to rebase this to the 2.3 branch?
I will later merge 2.3 to main so it will land in main as well. Would this work or are the files too different?

@fwolter fwolter changed the base branch from main to 2.3 August 18, 2022 09:34
@fwolter fwolter changed the base branch from 2.3 to main August 18, 2022 09:34
@fwolter
Copy link
Contributor Author

fwolter commented Aug 18, 2022

Tried, but there are some conflicts... I'd like to keep it on main in this PR.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you. LGTM

@daschuer daschuer merged commit 509a6a4 into mixxxdj:main Aug 18, 2022
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