Skip to content

[libxml2] add more features#30154

Merged
vicroms merged 16 commits intomicrosoft:masterfrom
yurybura:libxml2-features
Mar 22, 2023
Merged

[libxml2] add more features#30154
vicroms merged 16 commits intomicrosoft:masterfrom
yurybura:libxml2-features

Conversation

@yurybura
Copy link
Contributor

Move hardcoded external dependencies (libiconv, liblzma, zlib) to features, disable legacy flag by default.

Copy link
Contributor

@dg0yt dg0yt left a comment

Choose a reason for hiding this comment

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

Please don't change the formatting.

The network features are controversial.
FTP is off by default upstream, so it should be off by default in vcpkg.
HTTP is on by default upstream, but this breaks uwp.
IMO both could be off by default because they are don't offer secure communication.

@yurybura
Copy link
Contributor Author

yurybura commented Mar 13, 2023

Please don't change the formatting.

Ok.

The network features are controversial. FTP is off by default upstream, so it should be off by default in vcpkg.

FTP is off by default in the library, but it was enabled by default in VCPKG. Ok, I will follow library defaults.

HTTP is on by default upstream, but this breaks uwp.

HTTP and FTP are off on UWP in port's JSON:

    "ftp": {
      "description": "Add the FTP support",
      ->"supports": "!uwp"
    },
    "http": {
      "description": "Add the HTTP support",
      ->"supports": "!uwp"
    },

@jimwang118 jimwang118 added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Mar 14, 2023
@dg0yt
Copy link
Contributor

dg0yt commented Mar 14, 2023

HTTP and FTP are off on UWP in port's JSON:

They are not supported on UWP but still default features on all platforms. That's why the default installation is now broken for UWP.

@yurybura
Copy link
Contributor Author

HTTP and FTP are off on UWP in port's JSON:

They are not supported on UWP but still default features on all platforms. That's why the default installation is now broken for UWP.

@dg0yt Thank you for the help. Can you explain how to find this error in CI log? I see only failing ffmpeg, but not libxml2.
I thought that not supported features shouldn't be enabled by default.

@dg0yt
Copy link
Contributor

dg0yt commented Mar 14, 2023

Can you explain how to find this error in CI log? I see only failing ffmpeg, but not libxml2.
I thought that not supported features shouldn't be enabled by default.

The last assumption is wrong, unfortunately.

In CI, you can find this:

2023-03-13T21:24:25.0719853Z Determining parent hashes using HEAD~1
...
2023-03-13T21:24:37.7602418Z                          libxml2:x64-uwp:        *: fc1251a26227be5ea7e8d85511264909ea6130594175934b5e57573f149dd7c7
...
2023-03-13T21:24:38.2563362Z The following packages will be built and installed:
...
2023-03-13T21:24:38.5708655Z     libxml2[core]:x64-uwp -> 2.10.3

i.e. it libxml2 would build for master, but now for your PR merged into master:

2023-03-13T21:24:38.8263790Z Running CI using parent hashes
2023-03-13T21:24:38.9107487Z HEAD is now at 406598af0 Merge 8b1c3a90708f580b7c849e60c3f03a5119114251 into f09a2226865dd00a54b983933dd29ab29bca8bf6
...
2023-03-13T21:24:50.6519768Z                          libxml2:x64-uwp:  cascade

i.e. libxml2 is already known to be unsupported and is not even considered for build, which in turn causes

2023-03-13T21:24:50.8327278Z                  vcpkg-ci-ffmpeg:x64-uwp:  cascade
...
2023-03-13T21:24:51.4587144Z ##[error]REGRESSIONS:
REGRESSION: vcpkg-ci-ffmpeg:x64-uwp cascaded, but it is required to pass. (C:\a\1\s\scripts\azure-pipelines/../ci.baseline.txt).

because is vcpkg-ci-ffmpeg is required to pass, but has unsatisfied requirements.

@dg0yt
Copy link
Contributor

dg0yt commented Mar 14, 2023

You can probably also check locally with vcpkg install --dry-run libxml2:x64-uwp.

@jimwang118
Copy link
Contributor

@yurybura There is a conflict in the code you submitted, and the conflict needs to be resolved before CI can run.

@yurybura
Copy link
Contributor Author

yurybura commented Mar 19, 2023

@jimwang118 All checks have been done.

@jimwang118
Copy link
Contributor

All feature passed with following triplets:

x86-windows
x64-windows
x64-windows-static

@jimwang118 jimwang118 requested a review from LilyWangLL March 20, 2023 02:36
@jimwang118 jimwang118 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Mar 22, 2023
@vicroms vicroms merged commit eb11dbb into microsoft:master Mar 22, 2023
@yurybura yurybura deleted the libxml2-features branch March 23, 2023 12:23
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:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants