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

[gdal] Add Static Library Dependencies #16500

Closed
wants to merge 61 commits into from
Closed

[gdal] Add Static Library Dependencies #16500

wants to merge 61 commits into from

Conversation

n4z4m3
Copy link
Contributor

@n4z4m3 n4z4m3 commented Mar 2, 2021

Add geos.lib, shell32.lib, and ole32.lib so that downstream projects do not need to explicitly include them

@n4z4m3 n4z4m3 changed the title Add Static Library Dependencies [gdal] Add Static Library Dependencies Mar 2, 2021
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 2, 2021

If somebody could figure out why this is failing the x86_windows check that would be great. It is not failing for me when I build locally. Could it just be timing out? Because it was failing on the x64_windows and x64_windows static before I updated the CONTROL file and it re-did the checks. So I don't think there is a real problem.

I clicked on through the error details and it gets to this:

Found the following errors:
Error: While reading versions for port gdal from file: C:\a\1\s\versions\g-\gdal.json
Version 3.1.3#3 was not found in versions file.
Run:
vcpkg x-add-version gdal
to add the new port version.

What does that mean? Is that x-add-version something I am supposed to do?

Thanks in advance.

@NancyLi1013
Copy link
Contributor

Hi @nazame

Thanks for your PR.

For the failure on x86-windows, you can run .\vcpkg x-add-version gdal and then submit the changes for versions to solve.

@NancyLi1013 NancyLi1013 added the category:port-bug The issue is with a library, which is something the port should already support label Mar 3, 2021
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 3, 2021

Thank you @NancyLi1013

Is that x-add-versions a new command that is required for contributing changes?
I guess I need to remember that one for next time.

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Mar 3, 2021
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 3, 2021

I just realized that this also resolves issue #15526

@n4z4m3 n4z4m3 closed this Mar 3, 2021
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 3, 2021

Oops. I accidentally clicked the close button. How do I unclose this?

@n4z4m3 n4z4m3 reopened this Mar 3, 2021
ports/gdal/portfile.cmake Show resolved Hide resolved
@JackBoosY JackBoosY removed the info:reviewed Pull Request changes follow basic guidelines label Mar 4, 2021
@JackBoosY
Copy link
Contributor

And please notice #16088 and #15808 makes almost same changes.

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Mar 4, 2021
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 4, 2021

Thanks @JackBoosY . I feel like an idiot. I really should have specified that this was for the static version right at the beginning.
Because I am aware that you know much more about this than I do I was really questioning myself and whether the lib files were really needed.
I appreciate your diligence in making sure the correct changes are being made.

@vicroms
Copy link
Member

vicroms commented Mar 29, 2021

/azp run

@azure-pipelines
Copy link

Pull request contains merge conflicts.

@JackBoosY JackBoosY added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Mar 29, 2021
@JackBoosY
Copy link
Contributor

Can you please solve the file conflicts?

Thanks.

@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Mar 29, 2021

Can you please solve the file conflicts?

Thanks.

@JackBoosY I noticed that there have been changes to master since originally forked which are causing the conflicts. The biggest one is that the port has been modified to use the new vcpkg.json instead of CONTROL which is a good thing.
But I am wondering in this case what is the best way to resolve the conflicts. I was thinking that I could re-base. Is that appropriate? Or should I just manually make this branch match the master by deleting the CONTROL, and adding the vcpkg.json and manually making the other changes that are in master?

@JackBoosY
Copy link
Contributor

@nazame Rebase to master, apply your changes would be okay.

Add geos.lib, shell32.lib, and ole32.lib so that downstream projects do not need to explicitly include them
* [libffi] fix build on apple silicon

* [libffi] add version files

* [libffi] changes

* [libffi] add version files

* [libffi] Rearrange target conditional list

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
@ras0219-msft
Copy link
Contributor

This looks like a nice improvement for now, thanks @nazame! Hopefully in the future, we can handle this with pkg-config :)

DigitalInBlue and others added 19 commits April 2, 2021 14:18
* [celero] Updated to Celero v2.8.0

* [celero] Improving static linking support in VCPKG.

Celero issue #154 is tracking a fix.
DigitalInBlue/Celero#154

* [celero] Updated to v2.8.1

* [celero] Updated version for VCPKG.

* [celero] Updates to better support VCPKG.

* [celero] Updated versions.

* Update celero.json

Co-authored-by: JonLiu1993 <63675417+JonLiu1993@users.noreply.github.com>
* [qt5-base] fix glib link issues on linux

* add version

* remove unused vars

* update version

* write port info file.

* version stuff

* remove paraview from baseline. It was added due to the same spurious glib failures.

* trying out a different approach

* more changes

* change the if to actually use the buildtype instead

* remove x_vcpkg_get_port_info

* add option QT_OPENSSL_LINK back in

* use INCLUDE_DIRS_(DEBUG|RELEASE) instead of just INCLUDE_DIRS

* regen docs

* remove function call I forgot to remove

* [vcpkg-pkgconfig-get-modules] Move to port

* revert changes to ports.cmake

* include the file in qt5-base

* fix path

* remove unnecessary include

* Apply suggestions from code review

Co-authored-by: nicole mazzuca <mazzucan@outlook.com>

* Apply suggestions from code review

* update version

* ws removal

* version stuff

Co-authored-by: Robert Schumacher <roschuma@microsoft.com>
Co-authored-by: nicole mazzuca <mazzucan@outlook.com>
* [gdal] Fix configure error on OSX

* update version record

* Revert changes

* add touch command

* update version record
* [cmake] update cmake test port

* correct hash
add expat as dependency

* remove double expat dependency
…ug,opengl,openjpeg,libssh,tensorflow,tesseract,webp,libxml2 dependencies. (#15787)

* [tesseract] Use vcpkg_fixup_pkgconfig.

* [libxml2] Correct pkgconfig lib name.

* [libwebp] Use vcpkg_fixup_pkgconfig.

* [libssh] Export pkgconfig on windows.

Also move to using git to get source.

* [modplug] Export pkgconfig on windows.

* [ffmpeg] Add support for fontconfig,freetype,fribidi,modplug,openjpeg,libssh,tesseract,libxml2 dependencies.

* [openjpeg] Correct required static link libs in pkgconfig.

* [modplug] Combine vcpkg_from_github using variable.

* Update ports/libssh/CONTROL

* Improve portfile.cmake

* update version records.

* [openjpeg] Update libs in pkgcfg.

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>

* update version records

* [libssh] Add pthread to pkgconfig when using mbedtls.

* [libssh] Correct pthread naming on windows.

* [fontconfig] Add libintl to pkgconfig on windows.

* update version records

* [ffmpeg] Fixup FindFFmpeg.

* [ffmpeg] speex now supports non-windows.

* [ffmpeg] Add feature libass.

* [ffmpeg] Add dav1d feature.

* [ffmpeg] Add feature ilbc.

* [ffmpeg] Add tensorflow feature.

* [ffmpeg] update version record.

* [ffmpeg] Add CI feature test.

* [ffmpeg] Limit features based on CI failures.

* [ffmpeg] Update version record.

* [ffmpeg] limit features based on ci.baseline.

* [various ports] Update supports field.

* [ffmpeg] Limit features more based on CI.

* update version records.

* [ffmpeg] Add detection of additional non target deps..

select_library_configurations_from_names currently detects the debug libs even for release builds as  _IMPORT_PREFIX was not being set.

* [aubio] Silence warning about FindFFMPEG.

* [pangolin] Use vcpkg supplied FindFFMPEG.

* update version files.

* [ffnvcodec] Set as not supporting uwp.

ffnvcodec will build under uwp as its a header only lib, but it can not be used as it requires dynamic dll loading.

* [ffmpeg] Update feature all.

* update version records

* [tesseract] Wrap debug pkgcfg update.

* [libssh,libxml2,openjpeg,fontconfig] Fix pkg-config for release only triplets.

* [libssh] Correct port version after merge.

* [ffmpeg] Fixup after merge.

* Update version files.

* [ffmpeg] Add opengl support.

* [ffmpeg] Update package version.

* [ffmpeg] Fix ffnvcodec support.

* [ffmpeg] Fix x265 detection on osx.

* [libvpx] Enable arm-uwp build.

* [ffmpeg] Fixup x265 patch.

* trigger sdl rebuild

* [ffmpeg] Disable opengl on osx.

* Revert "trigger sdl rebuild"

This reverts commit 94065bf.

* [ffmpeg] Disable failing features on osx.

* Update ports/ffmpeg/FindFFMPEG.cmake.in

Co-authored-by: Matthias C. M. Troffaes <matthias.troffaes@gmail.com>

* [ffmpeg] Add ass dependencies to FindFFmpeg.

* Update ports/ffmpeg/FindFFMPEG.cmake.in

Co-authored-by: Matthias C. M. Troffaes <matthias.troffaes@gmail.com>

* update version

* [fontconfig] disable pthread/json as they are not needed for lib builds.

Only used for tests.

* [ffmpeg] Enable fontconfig on static+windows.

* update versions

* update versions.

* Fix incorrectly included commits

* revert pangolin commit

Reverts most of 2543be2

* update versions

* Correct port version after merge.

* update versions.

* [ffmpeg] Fix cmake dependency detection on non-windows.

* Revert "revert pangolin commit"

This reverts commit f59bc5a.

* [ffmpeg] Fix dependency loading that does not define separate debug/release libs.

* update versions.

* [ffmpeg] set CMP0072 policy.

* [wavpack] Fix cmake config export.

* [ffmpeg] Add optional system dependent libraries.

* update versions after merge.

* update versions.

Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com>
Co-authored-by: JackBoosY <yuzaiyang@beyondsoft.com>
Co-authored-by: Matthias C. M. Troffaes <matthias.troffaes@gmail.com>
* [libmariadb] Fix build error with cmake 3.20.0

* Update versions

* Add vcpkg.json

* Update versions/l-/libmariadb.json
* [fastcdr] Fix support Linux

* Format json and update versions

* Update versions/f-/fastcdr.json
* [sdl2] Fix pthread detection on macOS

* Add missing "FEATURES" to vcpkg_check_features

* [sdl2] Bump port-version

* Run x-add-version sdl2
* [vcpkg/scripts/make] add compiler tools to PATH

* use find_program to check if tools are already in PATH
…po (#17082)

* [nuklear] Upgrade from 2020-09-14 to 2021-03-18 version and switch repo

* Ran `vcpkg x-add-version --all`
* Update libjuice to 0.7.1 to update libdatachannel

* Transform the CONTROL file to .json and update version baseline.

* Update libjuice version. I don't really know why it changed.

* Fix version type

* Fix version type

Co-authored-by: Nemirtingas <nanaki89@hotmail.fr>
* [libmt32emu] update to 2.5.0

* add version files
* [sqlite3] Update to 3.35.4

* Run x-add-version sqlite3
Add geos.lib, shell32.lib, and ole32.lib so that downstream projects do not need to explicitly include them
There was a change in master which updated the port version causing a conflict with this branch so I re-based and updated my port version to 5.
@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Apr 7, 2021

Are all these changes because I didn't do the force push after re-basing?

@vicroms
Copy link
Member

vicroms commented Apr 7, 2021

Probably because I've been merging PRs to master that affected gdal as well.

@JackBoosY
Copy link
Contributor

Wow, the PR contains too many changes, can you split them?

@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Apr 8, 2021

Wow, the PR contains too many changes, can you split them?

@JackBoosY I agree it contains too many changes. The problem came because there were conflicts and I thought the way to resolve them would be to re-base the branch with master. However, I didn't to the force push after rebasing which caused all these changes to be pulled in from master.
Is there a way to undo this besides creating a new PR?

@NancyLi1013 NancyLi1013 removed the info:reviewed Pull Request changes follow basic guidelines label Apr 9, 2021
@JackBoosY
Copy link
Contributor

@nazame You can revert to an earlier commit I think.

@n4z4m3
Copy link
Contributor Author

n4z4m3 commented Apr 22, 2021

I wish to close this pull request since the latest update to the gdal package resolves part of it (the geos dependencies) and the other part (the proj shell32.lib and ole32.lib dependencies) are not really necessary. If you us the Additional Dependencies option in MS Visual Studio they are added already and this is the default for a new project in Visual Studio anyway.

@n4z4m3 n4z4m3 closed this Apr 22, 2021
@n4z4m3 n4z4m3 deleted the New-Static-Library-Dependencies branch April 22, 2021 22:36
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.