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 CI builds #5928

Merged
merged 1 commit into from Apr 14, 2020
Merged

fix CI builds #5928

merged 1 commit into from Apr 14, 2020

Conversation

AntonioBL
Copy link
Contributor

@AntonioBL AntonioBL commented Apr 12, 2020

This should solve the issue of nightly builds not properly generated for Windows.
"grep -P" was causing an error in Appveyor build chain (I checked by connecting via Remote Desktop to one of Appveyor builds).
I added the same modifications also for Mac to test if the absence of Mac nightly builds is due to the same cause.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • [only Win checked] I made sure the code compiles on my machine
  • [Mac to be tested] 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

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Apr 12, 2020

I believe just for consistency those changes from grep -oP "VERSION\s+\K.*" to sed -n -e "s/^.*VERSION *//p" should be done to Makefile line 23 too.

@AntonioBL
Copy link
Contributor Author

Well, apparently under Linux there are no errors, so there is no need to change. The result of the commands should basically be the same.
If it ain't broke, don't fix it. 😉

@AntonioBL
Copy link
Contributor Author

By the way, for Windows this PR works, see:
https://ci.appveyor.com/project/MuseScore/musescore/builds/32127503#L3420
(and following lines)
Under Mac we can't verify since Mac building of PRs is disabled.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Apr 13, 2020

Well, apparently under Linux there are no errors, so there is no need to change. The result of the commands should basically be the same.
If it ain't broke, don't fix it.

As said, (just) for consistency. If another change is needed, all relevant places would be easier to spot.

By the way, for Windows this PR works, see:
https://ci.appveyor.com/project/MuseScore/musescore/builds/32127503#L3420
(and following lines)
Under Mac we can't verify since Mac building of PRs is disabled.

Still I'm pretty sure this fixes the Mac issue too, looking at the output of a 'regular' (i.e. non-PR) build, which complains about that grep -P, twice. See for example https://travis-ci.org/github/musescore/MuseScore/jobs/674069975#L6187-L6191 and https://travis-ci.org/github/musescore/MuseScore/jobs/674069975#L6429-L6432

@anatoly-os anatoly-os merged commit da23c5f into musescore:master Apr 14, 2020
@AntonioBL AntonioBL deleted the fix_ci branch April 18, 2020 11:11
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Apr 20, 2020
follow up for PR musescore#5928 and just for consistency.
Also improve the syntax for all 3.
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.

None yet

3 participants