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

[gstreamer][gst-rtsp-server] Support Linux, fix OSX build and fix pkgconfig #20814

Closed
wants to merge 37 commits into from

Conversation

JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented Oct 18, 2021

Add Linux support, fix OSX build and fix gstreamer components's pkgconfig files.

Fixes #20785 #20812.
Related PR: #20555.

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team. labels Oct 18, 2021
@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Oct 18, 2021
@JackBoosY JackBoosY linked an issue Oct 18, 2021 that may be closed by this pull request
@JackBoosY JackBoosY changed the title [gstreamer] Support Linux and fix pkgconfig [gstreamer] Support Linux, fix OSX build and fix pkgconfig Oct 18, 2021
ports/gstreamer/portfile.cmake Outdated Show resolved Hide resolved
ports/gstreamer/portfile.cmake Outdated Show resolved Hide resolved
@JackBoosY
Copy link
Contributor Author

JackBoosY commented Oct 18, 2021

7563 Running compile:
7564 Working directory:  /Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/x64-osx-dbg/meson-private/tmpiwjth14m
7565 Command line:  /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -L/Users/usr/Documents/vcpkg/20814/vcpkg/installed/x64-osx/debug/lib -I/Users/usr/Documents/vcpkg/20814/vcpkg/installed/x64     -osx/include /Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/x64-osx-dbg/meson-private/tmpiwjth14m/testfile.c -o /Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/x64-osx-dbg/meson-private/tmpiwjth14m/outpu     t.exe -fPIC -g -O0 -Wl,-fatal_warnings -Wl,--version-script=/Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/src/1.19.2-dbedf5df9d.clean/gstreamer-full-default.map
7566
7567 Code:
7568  int main(void) { return 0; }
7569
7570 Compiler stdout:
7571
7572 Compiler stderr:
7573  ld: unknown option: --version-script=/Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/src/1.19.2-dbedf5df9d.clean/gstreamer-full-default.map
7574 clang: error: linker command failed with exit code 1 (use -v to see invocation)
7575
7576 Compiler for C supports link arguments -Wl,--version-script=/Users/usr/Documents/vcpkg/20814/vcpkg/buildtrees/gstreamer/src/1.19.2-dbedf5df9d.clean/gstreamer-full-default.map: NO
7577

Comment on lines 218 to 225
if(VCPKG_TARGET_IS_WINDOWS)
# note: can't find where z.lib comes from. replace it to appropriate library name manually
get_filename_component(BUILD_NINJA_DBG ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/build.ninja ABSOLUTE)
get_filename_component(BUILD_NINJA_DBG "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-dbg/build.ninja" ABSOLUTE)
vcpkg_replace_string(${BUILD_NINJA_DBG} "z.lib" "zlibd.lib")
get_filename_component(BUILD_NINJA_REL ${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/build.ninja ABSOLUTE)
get_filename_component(BUILD_NINJA_REL "${CURRENT_BUILDTREES_DIR}/${TARGET_TRIPLET}-rel/build.ninja" ABSOLUTE)
vcpkg_replace_string(${BUILD_NINJA_REL} "z.lib" "zlib.lib")
vcpkg_replace_string(${BUILD_NINJA_REL} "\"-Wno-unused\"" "") # todo: may need a patch for `gst_debug=false`
endif()
Copy link
Contributor

Choose a reason for hiding this comment

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

Wait what? Remove these lines completely and figure out where those come from. Either there is a find_dependency(find_library) like call in the build or one of the dependent pkg-config files is linking zlib wrongly.
These are the only two sources where it might come from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about the osx regression in my comment above? Is that related to this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Neumann-A I couldn't access this discord channel, anyway, it seems like a upstream bug.
If the upstream doesn't have any respose in these days, I will revert the baseline changes and keep it to fail.

Copy link
Contributor

Choose a reason for hiding this comment

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

z.lib might come from -lz in a pkg-config file, when calling pkg-config --msvc-syntax ....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dg0yt If it's true, does this mean that there is a common issue with pkgconfig in Windows?

Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean that there is a common issue with pkgconfig in Windows?

There is a common issues with people assuming stuff which isn't true. A lot of ports using cmake assume pkg-config is only available on UNIX system. They in addition assume there is probably no pkg-config file for all their dependencies and just hardcode wrong stuff into Libs: . -lz in Libs is just one very common example for this.
I don't know why this even crept in the unix package world..... I mean not deploying pc files .....

Copy link
Contributor

Choose a reason for hiding this comment

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

@JackBoosY No. It is just some packages blindly adding -lz, and some ports installing pc files without review. And you don't notice it until you use it.
(In vcpkg, I have .pc file issues with openssl and netcdf-c, in addition to the pending PRs. gdal+nmake+pkg-config turns out to be a great test case.)

Copy link
Contributor

Choose a reason for hiding this comment

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

also use something like

zlib_dep = dependency('zlib', required : true)
if not zlib_dep.found()
   message('TEST ZLIB not found!') # a) check meson syntax; don't know if message is correct. b) I doubt this will ever trigger. 
endif

Copy link
Contributor

@luncliff luncliff Oct 19, 2021

Choose a reason for hiding this comment

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

Indeed there must be one who adds -lz but couldn't find it.
#17394 (comment)

I previously confirmed that @Neumann-A 's suggestion is working well

zlib_dep = dependency('zlib', required : true) # in Windows

I can't remember well what was in the generated build.ninja. Probably both zlib path(in the ${vcpkg_root}/installed/${triplet}/...) and -lz together?

@JackBoosY
Copy link
Contributor Author

For osx regression, the upstream issue: https://gitlab.freedesktop.org/gstreamer/gst-build/-/issues/186.

@JackBoosY JackBoosY added the depends:upstream-changes Waiting on a change to the upstream project label Oct 18, 2021
@JackBoosY JackBoosY changed the title [gstreamer] Support Linux, fix OSX build and fix pkgconfig [gstreamer][gst-rtsp-server] Support Linux, fix OSX build and fix pkgconfig Oct 18, 2021
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout 699c8779f1b0feb4ed3564716d1ed31f69663ea6 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/g-/gstreamer.json b/versions/g-/gstreamer.json
index ceecb2b..d813c8c 100644
--- a/versions/g-/gstreamer.json
+++ b/versions/g-/gstreamer.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "07be58a4726cc70bfb1e44455804bb117805e8f0",
+      "git-tree": "d0f8d5d0a7204411a1ef31c8de847525e879a732",
       "version": "1.19.2",
       "port-version": 2
     },

@JackBoosY
Copy link
Contributor Author

Close and reopen this PR to retrigger the suggestion agent.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gst-rtsp-server/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gst-rtsp-server/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You have modified or added at least one vcpkg.json where a "license" field is missing.

If you feel able to do so, please consider adding a "license" field to the following files:

  • ports/gst-rtsp-server/vcpkg.json

Valid values for the license field can be found in the documentation

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for gst-rtsp-server but no changes to version or port version.
-- Version: 1.19.2#2
-- Old SHA: 7ee8619e03c47128fc5c633393e965436c769d29
-- New SHA: df17772b4069b1cd540b21804cc3397582831032
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout e809a42f87565e803b2178a0c11263f462d1800a -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 4a518eb7..1b790e73 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -5910,7 +5910,7 @@
     },
     "qtmultimedia": {
       "baseline": "6.2.4",
-      "port-version": 0
+      "port-version": 1
     },
     "qtnetworkauth": {
       "baseline": "6.2.4",
diff --git a/versions/q-/qtmultimedia.json b/versions/q-/qtmultimedia.json
index fc552d13..1bce662e 100644
--- a/versions/q-/qtmultimedia.json
+++ b/versions/q-/qtmultimedia.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "270c58ea89a27c45c553756bb6a5f6d69b3a4b10",
+      "version": "6.2.4",
+      "port-version": 1
+    },
     {
       "git-tree": "d0e46329a8e66cf3f95eca5d4018c955a8b54a15",
       "version": "6.2.4",

@PhoebeHui PhoebeHui marked this pull request as draft May 9, 2022 05:48
@JackBoosY
Copy link
Contributor Author

The OSX regression fixed by the upstream PR: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/2261

@JackBoosY JackBoosY closed this Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
7 participants