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

BUILD(client): Report correct/current release version in overlay #5491

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

klemensn
Copy link
Contributor

@klemensn klemensn commented Jan 21, 2022

Probably overlooked while cutting the release.
Reuse the main project's version number rather than hardcoding it.

Note 1.4.230 was released with an incorrect version number and therefore
builds as

-- ##################################################
-- Mumble version:          1.4.0

which is also visible in the final executable, e.g. prominently in the
window title (xprop(1) output):

WM_NAME(STRING) = "Mumble -- 1.4.0 | FOO"
_NET_WM_NAME(UTF8_STRING) = "Mumble -- 1.4.0 | FOO"

as well as the Help > About... > About Mumble menu.

Checks

overlay/CMakeLists.txt Outdated Show resolved Hide resolved
themes/Default/overlay.mumblelay Outdated Show resolved Hide resolved
@klemensn klemensn changed the title BUILD(client): Report correnc/current release version in overlay BUILD(client): Report correct/current release version in overlay Jan 21, 2022
Probably overlooked while cutting the release.
Reuse the main project's version number rather than hardcoding it.

Note 1.4.230 was released with an incorrect version number and therefore
builds as

```
-- ##################################################
-- Mumble version:          1.4.0
```

which is also visible in the final executable, e.g. prominently in the
window title (xprop(1) output):

```
WM_NAME(STRING) = "Mumble -- 1.4.0 | FOO"
_NET_WM_NAME(UTF8_STRING) = "Mumble -- 1.4.0 | FOO"
```

as well as the `Help > About... > About Mumble` menu.
@klemensn
Copy link
Contributor Author

Not sure how I broke the windows build with this.
I have to use GitHub Actions to test those builds, so iterating over a fix takes time.

@Krzmbrzl
Copy link
Member

Krzmbrzl commented Jan 21, 2022

That's what the CI has to say about it:

D:\a\1\b\overlay\overlay_xcompile-prefix\src\overlay_xcompile-build\mumble_ol.rc(21) : error RC2127 : version WORDs separated by commas expected

seems lie the RC for the cross-compiled overlay has some syntax error(s)? 👀

But that does indeed seem odd. The first thing I would check, is whether the used cmake variable might actually turn out to be empty (for some reason) 🤔

@Krzmbrzl Krzmbrzl added the build Everything related to compiling/building the code label Jan 21, 2022
@klemensn
Copy link
Contributor Author

That's what the CI has to say about it:

D:\a\1\b\overlay\overlay_xcompile-prefix\src\overlay_xcompile-build\mumble_ol.rc(21) : error RC2127 : version WORDs separated by commas expected

seems lie the RC for the cross-compiled overlay has some syntax error(s)? 👀

But that does indeed seem odd. The first thing I would check, is whether the used cmake variable might actually turn out to be empty (for some reason) 🤔

I had the same suspicion but looked into it just now:
https://dev.azure.com/Mumble-VoIP/Mumble/_build/results?buildId=5690&view=logs&j=b9e62f99-1a98-5ed7-01d2-f4794231ed79&t=c79949eb-fa56-5b34-f22d-5c012a81aaed&l=443:

2022-01-21T11:02:50.8458770Z CMake Warning at CMakeLists.txt:10 (project):
2022-01-21T11:02:50.8558895Z   VERSION keyword not followed by a value or was followed by a value that
2022-01-21T11:02:50.8565078Z   expanded to nothing.

https://dev.azure.com/Mumble-VoIP/Mumble/_build/results?buildId=5690&view=logs&j=b9e62f99-1a98-5ed7-01d2-f4794231ed79&t=c79949eb-fa56-5b34-f22d-5c012a81aaed&l=436:

2022-01-21T11:02:50.8393201Z cmd.exe /C "cd /D D:\a\1\b\overlay\overlay_xcompile-prefix\src\overlay_xcompile-build && D:\a\1\s\overlay\scripts\build_overlay_xcompile.cmd -G Ninja -DCMAKE_TOOLCHAIN_FILE=C:/hostedtoolcache/windows/MumbleBuild/windows-static-1.5.x~2021-07-27~64f88807.x64/scripts/buildsystems/vcpkg.cmake -DVCPKG_TARGET_TRIPLET=x86-windows-static-md -DPARENT_SOURCE_DIR=D:/a/1/s -DMUMBLE_SOURCE_DIR=D:/a/1/s/src/mumble -DMUMBLE_BINARY_DIR=D:/a/1/b -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=cl.exe -DCMAKE_CXX_COMPILER=cl.exe -DBUILD_OVERLAY_XCOMPILE=ON -Dsymbols=ON -Dversion=1.5.0 D:/a/1/s/overlay && "C:\Program Files\CMake\bin\cmake.exe" -E touch D:/a/1/b/overlay/overlay_xcompile-prefix/src/overlay_xcompile-stamp/overlay_xcompile-build"

-Dversion=1.5.0 is set and comes from

"-Dversion=${PROJECT_VERSION}"

which means PROJECT_VERSION must be set properly.

My understanding is that this is the version of the overlay_xcompile project, for which my diff inherits the version from the global Mumble project

VERSION "1.5.${BUILD_NUMBER}"

via
VERSION ${CMAKE_PROJECT_VERSION}

as per CMAKE_PROJECT_VERSION coming from the parent scope.
(This variable is new in CMake 3.12 but we require 3.15 anyway, so it must be there.)

From the last link:

To obtain the version from the most recent call to project() in the current directory scope or above, see the PROJECT_VERSION variable.

So maybe the working -Dversion=1.5.0 above works because it gets the PROJECT_VERSION value from the parent (read: top-level) scope?
That'd explain why this works while the overlay's directory/project VERSION value stays empty.

I feel like I'm missing something obvious here, so I'm reluctant to just use PROJECT_VERSION everywhere to call it a day.

@Krzmbrzl
Copy link
Member

Hm, I don't understand it either. I would also have assumed that CMAKE_PROJECT_VERSION works 👀
If PROJECT_VERSION is working though, I'd be fine with just using that 🤷

@klemensn
Copy link
Contributor Author

Hm, I don't understand it either. I would also have assumed that CMAKE_PROJECT_VERSION works 👀 If PROJECT_VERSION is working though, I'd be fine with just using that 🤷

It does work in

VERSION ${CMAKE_PROJECT_VERSION}

as can be seen in the Ubuntu build
https://github.com/mumble-voip/mumble/runs/4895262077?check_suite_focus=true#step:9:302:

[289/446] Linking C shared library libmumbleoverlay.x86_64.so.1.5.0

src/CMakeLists.txt adds both overlay and overlay_gl subdirectories.
overlay fails, overlay_gl works wrt. CMAKE_PROJECT_VERSION?
overlay uses it in a set_target_properties() command, overlay_gl in a project() one.

Is there some subtle CMake difference between these functions and variable scoping?
Or is CMake on Windows behaving differently?

@Krzmbrzl
Copy link
Member

Or is CMake on Windows behaving differently?

It should not.

Is there some subtle CMake difference between these functions and variable scoping?

No that I am aware of... But I added a commit with a debug logging statement. Maybe it helps to figure out what's going on

The previous debug commit proves CMAKE_PROJECT_VERSION=1.5.0 so WHY
does it not work a few lines down below?
@klemensn
Copy link
Contributor Author

klemensn commented Jan 24, 2022

No that I am aware of... But I added a commit with a debug logging statement. Maybe it helps to figure out what's going on

So this

message(STATUS "CMAKE_PROJECT_VERSION is ${CMAKE_PROJECT_VERSION} whereas PROJECT_VERSION is ${PROJECT_VERSION}")

prints
https://dev.azure.com/Mumble-VoIP/Mumble/_build/results?buildId=5715&view=logs&j=b9e62f99-1a98-5ed7-01d2-f4794231ed79&t=c79949eb-fa56-5b34-f22d-5c012a81aaed&l=253:

CMAKE_PROJECT_VERSION is 1.5.0 whereas PROJECT_VERSION is 1.5.0

... awkward silence...

I'll do some more printf()^Wmessage() debugging!

@Krzmbrzl
Copy link
Member

FAILED: overlay/overlay_xcompile-prefix/src/overlay_xcompile-stamp/overlay_xcompile-build D:/a/1/b/overlay/overlay_xcompile-prefix/src/overlay_xcompile-stamp/overlay_xcompile-build 
cmd.exe /C "cd /D D:\a\1\b\overlay\overlay_xcompile-prefix\src\overlay_xcompile-build && D:\a\1\s\overlay\scripts\build_overlay_xcompile.cmd -G Ninja -DCMAKE_TOOLCHAIN_FILE=C:/hostedtoolcache/windows/MumbleBuild/windows-static-1.5.x~2021-07-27~64f88807.x64/scripts/buildsystems/vcpkg.cmake -DVCPKG_TARGET_TRIPLET=x86-windows-static-md -DPARENT_SOURCE_DIR=D:/a/1/s -DMUMBLE_SOURCE_DIR=D:/a/1/s/src/mumble -DMUMBLE_BINARY_DIR=D:/a/1/b -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=cl.exe -DCMAKE_CXX_COMPILER=cl.exe -DBUILD_OVERLAY_XCOMPILE=ON -Dsymbols=ON -Dversion=1.5.0 D:/a/1/s/overlay && "C:\Program Files\CMake\bin\cmake.exe" -E touch D:/a/1/b/overlay/overlay_xcompile-prefix/src/overlay_xcompile-stamp/overlay_xcompile-build"
**********************************************************************
** Visual Studio 2019 Developer Command Prompt v16.11.9
** Copyright (c) 2021 Microsoft Corporation
**********************************************************************
-- CMAKE_PROJECT_VERSION is  whereas PROJECT_VERSION is 
-- again, CMAKE_PROJECT_VERSION is  whereas PROJECT_VERSION is 
CMake Warning at CMakeLists.txt:14 (project):
  VERSION keyword not followed by a value or was followed by a value that
  expanded to nothing.


-- The CXX compiler identification is MSVC 19.29.30139.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x86/cl.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- after calling project(), CMAKE_PROJECT_VERSION is  whereas PROJECT_VERSION is 
-- Using Win7 as the oldest supported Windows version
-- Found Boost: C:/hostedtoolcache/windows/MumbleBuild/windows-static-1.5.x~2021-07-27~64f88807.x64/installed/x86-windows-static-md/include (found version "1.76.0")  
-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    version

it seems like cmake is called a second time further into the build and that is the call in which we are seeing errors 👀

@ghost
Copy link

ghost commented Jan 27, 2022

it seems like cmake is called a second time further into the build and that is the call in which we are seeing errors eyes

Yes, that is the build for x86 (a.k.a 32 bit) so that applications that are 32 bit will use a 32 bit overlay.

@Krzmbrzl
Copy link
Member

@ZeroAbility do you happen to know where we instantiate the explicit second call to cmake?

I think what is happening here is that when we invoke cmake for the x86 cross-compile build, cmake only processes the overlay cmake file and therefore did not process the main CMakeLists.txt first and thus the main project statement in there is never encountered, leading to an empty CMAKE_PROJECT_VERSION variable

@Krzmbrzl Krzmbrzl marked this pull request as draft February 6, 2022 17:51
@Krzmbrzl
Copy link
Member

Krzmbrzl commented Feb 6, 2022

I have converted this PR to a draft until you (or someone else) find the time to complete it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Everything related to compiling/building the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants