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

Missing path for ${manpage}.xml dependency #9804

Closed
karmix opened this issue Jan 24, 2024 · 7 comments · Fixed by #9813
Closed

Missing path for ${manpage}.xml dependency #9804

karmix opened this issue Jan 24, 2024 · 7 comments · Fixed by #9813
Milestone

Comments

@karmix
Copy link

karmix commented Jan 24, 2024

Describe the bug
Building FreeRDP on older versions of CMake fails with the message:

ninja: error: '../client/SDL/man/sdl-freerdp3.1.xml', needed by 'client/SDL/man/CMakeFiles/sdl-freerdp3.1.manpage', missing and no known rule to make it

This is because the build target ${manpage}.manpage lists ${manpage}.xml as a build dependency, here:

add_custom_target(
${manpage}.manpage ALL
DEPENDS
${manpage}
${manpage}.xml
${template}.xml.in
generate_argument_docbook
)
but defines no build rule for ${manpage}.xml. Instead, ${manpage}.xml is generated and placed in the binary directory by a call to configure_file at configure time, before ninja is invoked to build the project. Since the dependency declaration does not specify the path, some versions of CMake expect to find ${manpage}.xml in the source directory, where it does not exist, and abort. The latest version of CMake appears to recognize that it already generated ${manpage}.xml and implicitly uses the path to the binary directory when constructing the the build rules.

To Reproduce
Create a new directory, then in that directory:

  1. Place a copy of the attached Dockerfile (rename it to remove the .txt extension)
  2. Clone FreeRDP as "FreeRDP"
  3. Run the following commands:
mkdir deps pkgs

# Build Docker image
docker build --build-arg BASE_IMAGE=ubuntu:20.04 --tag freerdp-builder:ubuntu-20.04 --file Dockerfile .

# FreeRDP fails to provide FindcJSON.cmake, so find_package cannot find libcjson.
# Compile against newer package that includes cJSONConfig.cmake.
cd deps
CJSON_REPO=http://ftp.debian.org/debian/pool/main/c/cjson
curl -O $CJSON_REPO/libcjson-dev_1.7.14-1_amd64.deb -O $CJSON_REPO/libcjson1_1.7.14-1_amd64.deb
cd ..

# Build FreeRDP
docker run -it -v ./deps:/deps:ro -v ./FreeRDP:/src:ro -v ./pkgs:/out --rm freerdp-builder:ubuntu-20.04

Expected behavior
The build should complete successfully and place the new package files into the pkgs directory.

@karmix
Copy link
Author

karmix commented Jan 24, 2024

The dependency list for ${manpage}.manpage should only include ${manpage}. The three intermediate files added as extra dependencies in a7eeb8e are not necessary. As dependencies of ${manpage}, generate_argument_docbook (implicitly defined) and ${manpage}.xml are already included in the dependency tree of ${manpage}.manpage and will be processed when building ${manpage}. Since ${template}.xml.in is only used at configure time and is not a build target or used by any of the build targets in the project, it will never have any impact on the build process (other than to break it if the file goes missing) and does not belong in any dependency lists.

Rather than fixing the path for ${manpage}.xml in the definition of ${manpage}.manpage I would recommend DRYing up the code a bit and removing it's unneeded dependencies.

@akallabeth
Copy link
Member

@karmix we´re building the source code for that on https://ci.freerdp.com/job/freerdp-nightly-binaries/ (package builder) without any issue.
Also using dpkg-buildpackage locally on a ubuntu machine works fine.

@akallabeth
Copy link
Member

@karmix also:

# FreeRDP fails to provide FindcJSON.cmake, so find_package cannot find libcjson.
# Compile against newer package that includes cJSONConfig.cmake.
cd deps
CJSON_REPO=http://ftp.debian.org/debian/pool/main/c/cjson
curl -O $CJSON_REPO/libcjson-dev_1.7.14-1_amd64.deb -O $CJSON_REPO/libcjson1_1.7.14-1_amd64.deb
cd ..

this is kind of useless, as we already do a fallback to pkg-config in case no CMake file is detected (exactly due to ubuntu and older debian systems not installing that one)

@karmix
Copy link
Author

karmix commented Jan 25, 2024

@akallabeth: Unfortunately, the build fails with the version of CMake that ships with a fully updated release of Ubuntu Focal (20.04), which is why I opened the case. The Dockerfile can reproduce the issue. Of Ubuntu's supported releases, Focal is the oldest and appears to be the only one that fails.

I did not find a Jenkinsfile or job config, but based on the the release names I would guess that CI is building those packages on Debian. Because of the way Ubuntu forks off Debian's dev branch, some packages get frozen at slightly older releases than what has since been released into the upstream distro. Ubuntu Focal froze CMake at 3.16, while Debian Bullseye went GA with CMake 3.18. I just looked through the CMake release notes between those versions and found a note for 3.17 stating:

The add_custom_command() command learned to interpret paths in DEPENDS arguments that are specified relative to the current binary directory.

This would explain why the build fails on Ubuntu Focal but succeeds on its upstream, Debian Bullseye.

With regard to cJSON, I think you will find that the lines you quoted are not useless; if you remove them the build fails with a different error. Ubuntu Focal ships with libcjson-dev_1.7.10-1 which does not include either CMake or PkgConfig files. Those lines install Debian Bullseye's upstream library to give CMake the files it needs to find the cJSON library without a Find script so it can progress past the configure phase to start the build. However, I wouldn't consider the cJSON config issue particularly relevant to this one.

@akallabeth
Copy link
Member

@karmix oh, indeed, mixed that up with debian oldstable where only the pc file is included.
I just tried the whole thing on a ubuntu 20.04 vm and the bug you describe is indeed there, but our nightly builder works. (that runs on a debian system, yes, but packages are created in a debootstrap environment of the appropriate target, so that is still a mystery to me why that happens, maybe the issue was addressed by @bmiklautz ?)

@karmix
Copy link
Author

karmix commented Jan 27, 2024

@akallabeth: The current releases of Ubuntu are Focal, Jammy, and Mantic. The Jenkins page for freerdp-nightly-binaries shows build results for Jammy, Lunar, and Mantic. (Lunar is end of life as of yesterday, btw). It looks like Focal was dropped from the nightly builds. If I had to make a wild guess, I would say that when Mantic was added, Focal may have been accidentally dropped instead of Lunar. Because the LTS releases (Focal & Jammy) are supported much longer than interim releases (Lunar & Mantic), it is not always the oldest release that expires next.

@akallabeth akallabeth linked a pull request Jan 29, 2024 that will close this issue
@akallabeth akallabeth added this to the next-3.0 milestone Jan 29, 2024
@karmix
Copy link
Author

karmix commented Jan 29, 2024

I have confirmed that #9813 resolves this issue and the cJSON problem we discussed for Ubuntu 20.04. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants