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

fix Windows nightly build uploads #5968

Merged
merged 1 commit into from
Apr 21, 2020
Merged

Conversation

AntonioBL
Copy link
Contributor

This is another tentative at fixing Windows nightly uploading when using Appveyor MSVC 2019 image.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • [N/A] I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [N/A] I created the test (mtest, vtest, script test) to verify the changes I made

Copy link
Contributor

@Spire42 Spire42 left a comment

Choose a reason for hiding this comment

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

I just looked into this.

The SETLOCAL and ENDLOCAL commands are there because msvc_build.bat should not be polluting all developers' environments with mysterious generically-named variables just because AppVeyor happens to need one of them. (Also, we need to make sure that command extensions are enabled, and SETLOCAL is the command that does that.)

Anyway, to address the problem at hand:

Appveyor is apparently relying on BUILD_FOLDER having been set by msvc_build.bat, which is not a good idea at all. (OTOH, it's not using INSTALL_FOLDER, which I guess we can be thankful for.)

To fix this, I don't think we should be reverting to the old behavior of polluting the environment.

The real fix would be to provide a better and clearly documented optional mechanism for msvc_build.bat to communicate any required information back to its caller. (My suggestion is to add an optional command-line parameter that explicitly specifies the target build folder, which would actually turn the communication the other way around.)

In the meantime, I think we can implement a quick fix without touching msvc_build.bat that takes advantage of the fact that msvc_build.bat ends with the current working directory set to the build folder.

To do this in your current PR, first undo the changes to msvc_build.bat, then add this line to the end of build\appveyor\build_script.bat (after the two calls to msvc_build.bat):

FOR %%I in (.) DO SET BUILD_FOLDER=%%~nxI

I don't have access to an AppVeyor environment to test this, so could you please test and see if this works?

@AntonioBL
Copy link
Contributor Author

Thank you for your comment.
I tried, but unfortunately the folder is reset to parent folder (MuseScore folder) after the execution of
msvc_build.bat (both when running locally, and also on Appveyor, see for example here: https://ci.appveyor.com/project/AntonioBL/musescore/builds/32282810/job/w473c4kdhn3k97wc#L3462 ).
But then I thought that this (deploy and upload) script just need to correctly run on Appveyor only, so I tried to use env variables defined by Appveyor (as they are already used by the script), and I found that the architecture value is defined in "PLATFORM" variable.
It works, for both x86 (see https://ci.appveyor.com/project/AntonioBL/musescore/builds/32283051/job/8nk9vkckp5pseco5#L3464 ) and x64 (see https://ci.appveyor.com/project/AntonioBL/musescore/builds/32283051/job/cyd4l5p6u1q8kgh4#L3439 ).
I updated the PR.
Thank you for your help.

@Spire42
Copy link
Contributor

Spire42 commented Apr 19, 2020

Sorry that my suggestion didn't end up being useful. I'm glad that you were able to come up with something better!

@anatoly-os anatoly-os merged commit 333752a into musescore:master Apr 21, 2020
@Jojo-Schmitz
Copy link
Contributor

Indeed fixed, MuseScoreNightly-2020-04-21-1415-master-333752a-x86_64.7z is there now

@AntonioBL AntonioBL deleted the provassh branch October 26, 2020 11:35
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.

4 participants