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.

Sign up for GitHub

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

Jump to bottom

[qt5-base] Add zstd as a dependency #12342

Merged
merged 10 commits into from
Aug 1, 2020
Merged

[qt5-base] Add zstd as a dependency #12342

merged 10 commits into from
Aug 1, 2020

Conversation

Neumann-A
Copy link
Contributor

@Neumann-A Neumann-A commented Jul 9, 2020 •
edited
Loading

also moves embree3 dependency to the correct port.
also moves the hdf5 fixes to vtk via applying an upstream patch.

For testing: Added Default-Features: vtkm, tools to paraview.

@Neumann-A Neumann-A mentioned this pull request Jul 9, 2020
Merged
@LilyWangL LilyWangL requested a review from JackBoosY July 10, 2020 01:37
@JackBoosY
Copy link
Contributor

Please ping me if this PR ready for review.

@Neumann-A
[vtk] add hdf5 api patch
8cfd8d4 
[paraview] remove hdf5 use flags from portfile
[field3d] use flags instead of patching to fix hdf5.
@Neumann-A
[paraview] remove message on osx since it now works with qt 5.15
bfff85d
@Neumann-A
Copy link
Contributor Author

@JackBoosY: Can I keep feature tools in paraview as a default-feature? It is basically testing if Qt and VTK are linking everything correctly. If CI is green this should be good to go.

@JackBoosY
Copy link
Contributor

@Neumann-A That's okay.

@Neumann-A Neumann-A marked this pull request as ready for review July 10, 2020 09:32
@Neumann-A
Copy link
Contributor Author

@JackBoosY: ready

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist requires:testing Needs tests added before merging labels Jul 13, 2020
@ras0219-msft
Copy link
Contributor

I think a better approach than enabling the tools by default would be to add a new helper port (vcpkg-ci-test-helper) which depends on the features we want built in CI. That way, need for CI testing can be separated from what users actually need by default.

@Neumann-A
Copy link
Contributor Author

@ras0219-msft: The problem here is I cannot deactivate zstd without patching qt's configure.json somewhere. zstd will be picked up if it is available either within vcpkg or by the system. So it is better to activate it so that the vcpkg built version always gets picked up.

@Neumann-A
Copy link
Contributor Author

@ras0219-msft: disregard you probably meant the paraview tools change ;)

@Neumann-A
Copy link
Contributor Author

@ras0219-msft: maybe vcpkg should use a port overlay for CI for ports testing other ports in CI? So the main port dir does not get cluttered. maybe something like ci-ports or ports-ci or ports-testing

This was referenced Jul 16, 2020
Closed
Merged
@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Jul 20, 2020
@Neumann-A Neumann-A mentioned this pull request Jul 22, 2020
Closed
@JackBoosY JackBoosY added info:reviewed Pull Request changes follow basic guidelines requires:discussion and removed requires:testing Needs tests added before merging labels Jul 27, 2020
@strega-nil
Copy link
Contributor

@Neumann-A Could you merge with master?

@Neumann-A
Merge remote-tracking branch 'upstream/master' into qt_add_zstd
84dcc9f 
# Conflicts:
#	ports/field3d/0003_hdf5_api.patch
#	ports/field3d/CONTROL
#	ports/field3d/portfile.cmake
@strega-nil strega-nil self-requested a review July 30, 2020 22:10
@strega-nil
Copy link
Contributor

I'm gonna wait until after #12529 and rerun the CI.

@JackBoosY JackBoosY added depends:different-pr This PR or Issue depends on a PR which has been filed and removed info:reviewed Pull Request changes follow basic guidelines labels Jul 31, 2020
@JackBoosY JackBoosY mentioned this pull request Jul 31, 2020
Merged
@strega-nil
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@strega-nil strega-nil added info:reviewed Pull Request changes follow basic guidelines and removed depends:different-pr This PR or Issue depends on a PR which has been filed labels Jul 31, 2020
@strega-nil strega-nil merged commit f7fe9e3 into microsoft:master Aug 1, 2020
@NancyLi1013 NancyLi1013 mentioned this pull request Aug 4, 2020
Merged
@Neumann-A Neumann-A deleted the qt_add_zstd branch August 6, 2020 13:50
@Neumann-A Neumann-A mentioned this pull request Aug 6, 2020
Closed
hellozee pushed a commit to hellozee/vcpkg that referenced this pull request Sep 11, 2020
@Neumann-A @hellozee
[qt5-base] Add zstd as a dependency (microsoft#12342)
88088c7 
* [qt5-base] add hidden optional not switchable zstd dependency.

* [netcdf/paraview] correct embree3 dependency
[paraview] make tools default feature for CI testing.

* [paraview] fix typo and remove unnecessary flags

* [paraview] add flags back into portfile

* [vtk] add hdf5 api patch
[paraview] remove hdf5 use flags from portfile
[field3d] use flags instead of patching to fix hdf5.

* [paraview] remove message on osx since it now works with qt 5.15

* [paraview] remove default feature and force CI features by a CI test port

* [paraview|CI] back out python from CI testing for now

* [qt5-base] add in the macos deployment target fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@strega-nil strega-nil strega-nil approved these changes

@JackBoosY JackBoosY JackBoosY approved these changes

Assignees

@JackBoosY JackBoosY

Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Neumann-A @JackBoosY @ras0219-msft @strega-nil