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

[boost-gil] remove dependency boost-filesystem #20575

Merged
merged 4 commits into from
Oct 12, 2021

Conversation

Osyotr
Copy link
Contributor

@Osyotr Osyotr commented Oct 7, 2021

This dependency is optional and only used to build and run tests and examples.
See https://github.com/boostorg/gil#requirements
However, there is also BOOST_GIL_IO_ADD_FS_PATH_SUPPORT which enables
boost::filesystem::path support for IO functions, but since users have to define
it manually, they must install the dependency manually, too.

A little benefit is that there is no need to build anything anymore: GIL itself and all its dependencies are header-only.

This dependency is optional and only used to build and run tests and examples.
See https://github.com/boostorg/gil#requirements
However, there is also BOOST_GIL_IO_ADD_FS_PATH_SUPPORT which enables
boost::filesystem::path support for IO functions, but since users have to define
it manually, they must install the dependency manually, too.
@strega-nil-ms
Copy link
Contributor

I don't like doing this without making sure that this doesn't change over time when people run generate-boost.ps1; @ras0219 what do you think?

@Osyotr
Copy link
Contributor Author

Osyotr commented Oct 7, 2021

@strega-nil-ms is vcpkg.json for autogenerated? I can't find anything related.
git grep gil over entire vcpkg directory prints only this port and mapnik, which already explicitly declares boost-filesystem dependency.

@autoantwort
Copy link
Contributor

is vcpkg.json for autogenerated?

Yes the boost vcpkg.json files are autogenerated by scripts/boost/generate-ports.ps1

@Osyotr
Copy link
Contributor Author

Osyotr commented Oct 7, 2021

@autoantwort I see. So dependencies of boost libraries are determined by their includes and even though boost-filesystem includes are guarded by #ifdef ..., dependency is added nevertheless. The only solution I see now is to forcefully remove it from dependency list here

@PhoebeHui PhoebeHui self-assigned this Oct 8, 2021
@PhoebeHui PhoebeHui added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Oct 8, 2021
@PhoebeHui
Copy link
Contributor

cc @yurybura

@strega-nil-ms
Copy link
Contributor

I've made the required changes to generate-ports.ps1

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Oct 9, 2021
@@ -24,6 +24,7 @@ else {
# Clear this array when moving to a new boost version
$portVersions = @{
#e.g. "boost-asio" = 1;
"boost-gil" = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

After re-run generation script the dependency will be added again. Script's logic must be modified too. I can do this in #20432.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done by @strega-nil-ms )

@BillyONeal BillyONeal added the depends:different-pr This PR or Issue depends on a PR which has been filed label Oct 12, 2021
@BillyONeal
Copy link
Member

#20432 restores compat between generate-ports.ps1 and all boost ports and should land first.

# Conflicts:
#	scripts/boost/generate-ports.ps1
@BillyONeal BillyONeal removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Oct 12, 2021
@BillyONeal
Copy link
Member

I pushed a merge with master to get #20432 and ensured that generate-ports.ps1 results in no edits

@BillyONeal BillyONeal merged commit 9240fd0 into microsoft:master Oct 12, 2021
@Osyotr Osyotr deleted the boost_gil_dependency branch October 12, 2021 06:11
@Osyotr
Copy link
Contributor Author

Osyotr commented Oct 12, 2021

I pushed a merge with master to get #20432 and ensured that generate-ports.ps1 results in no edits

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

7 participants