Skip to content

[xalan-c] delist port#27401

Merged
BillyONeal merged 4 commits intomicrosoft:masterfrom
rleigh-codelibre:xalan-removal
Nov 11, 2022
Merged

[xalan-c] delist port#27401
BillyONeal merged 4 commits intomicrosoft:masterfrom
rleigh-codelibre:xalan-removal

Conversation

@rleigh-codelibre
Copy link
Copy Markdown
Contributor

Describe the pull request

Deprecate the "xalan-c" port.

  • What does your PR fix?

Xalan-C is in the process of being retired (I'm the primary upstream maintainer at Apache and have previously contributed updates and fixes to the vcpkg support for it). It will shortly be moved to the Attic. No future releases, bugfixes or security updates will be provided.

Mark the port as deprecated in line with how other ports have been deprecated.

Copy link
Copy Markdown

@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 934a99dc13cabb330824ae1a5ab4a53a9acc5a49 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/x-/xalan-c.json b/versions/x-/xalan-c.json
index b9b318b..8a51613 100644
--- a/versions/x-/xalan-c.json
+++ b/versions/x-/xalan-c.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "5d5d3baabf6012c165d3cb8b81763cdc733a2837",
+      "version-string": "deprecated",
+      "port-version": 0
+    },
     {
       "git-tree": "2a2f5f469a510da8d58186fc7f58f8fc9b1e83d6",
       "version": "1.12",

You have modified or added at least one vcpkg.json where you should check the license field.

Details

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

  • ports/xalan-c/vcpkg.json

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

The Xalan-C project is being retired upstream and moved to the Apache
Attic.
Copy link
Copy Markdown

@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 934a99dc13cabb330824ae1a5ab4a53a9acc5a49 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/x-/xalan-c.json b/versions/x-/xalan-c.json
index b9b318b..8874cdc 100644
--- a/versions/x-/xalan-c.json
+++ b/versions/x-/xalan-c.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "061140d4d2fcb5b696f3453a79d13a1a6922e409",
+      "version-string": "deprecated",
+      "port-version": 0
+    },
     {
       "git-tree": "2a2f5f469a510da8d58186fc7f58f8fc9b1e83d6",
       "version": "1.12",

github-actions[bot]
github-actions bot previously approved these changes Oct 22, 2022
"xerces-c"
]
"version-string": "deprecated",
"description": "Retired port. libxslt, qt5xmlpatterns and SaxonC are alternatives to consider.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

qt5xmlpatterns is deprecated, saxonc is paid...
Btw why remove port? Just issue a warning in portfile

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The retirement of the Xalan-C project is a wholly separate concern from what other projects are or are not doing. I have put them in the PR as suggestions for users who will be affected to consider. You're right that the alternatives aren't necessarily good either, but the Xalan-C project is being wound up and end-users will need to consider their options. It may well be the case that paying for SaxonC is the better choice.

With regard to deprecation, I've followed the pattern used by other projects in vcpkg. I haven't seen any which are both marked deprecated and which are still buildable. That doesn't appear to be possible with the current practice of putting "deprecated" into the version string, rather than having some additional metadata for that purpose.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

other projects in vcpkg

please give example projects. I also see no reason to remove the port itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Examples: gdcm2, lodepng-c, qt5-modularscripts, qtquickcontrols2. Two of these have a dependency on their replacement. That's not possible to do in the case of xalan-c.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The examples you give have not been removed. They either have been renamed within vcpkg or been integrated into other ports. As such the libraries they represent still exist within vcpkg. (especially qt5-modularscripts was just an unnecessary internal vcpkg helper port)

As long as the github repo exists I don't see any real reason to remove it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If vcpkg provides a mechanism to deprecate a port while retaining the ability to build it, then please point me to an example of this and I'll be happy to update this PR to follow that example. But if that's something which vcpkg doesn't currently cater for, then that's perhaps something the vcpkg maintainers need to provide some mechanism for.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The version string is not suitable here: The version-string will not take part in version number resolution for manifest requirements. But vcpkg may need to add new patches to the current version.

@Cheney-W Cheney-W added category:port-update The issue is with a library, which is requesting update new revision info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Oct 24, 2022
@Cheney-W
Copy link
Copy Markdown
Contributor

Adding reviewed lable is just for team to further review.

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Oct 29, 2022
@BillyONeal
Copy link
Copy Markdown
Member

If an upstream wishes to be delisted, particularly when we're talking something that is likely to be security sensitive like this, I'm inclined to do what they asked. However, that action is somewhat unprecedented, so I want other maintainers to confirm before we proceed.

@rleigh-codelibre
Copy link
Copy Markdown
Contributor Author

rleigh-codelibre commented Oct 29, 2022

@BillyONeal If vcpkg provides any mechanism for continuing to allow the package to build, but at the same time marking it deprecated then I would be happy with that as an interim step before completely withdrawing it. It looks like this isn't currently possible, so I would prefer to remove completely and mark deprecated. If vcpkg needs to develop some support for this I'm happy to revisit if this changes.

Regarding "other maintainers", the short answer to that is that there aren't any. I'm the only person to have done any maintenance on this project in the last 9 years, which is the primary reason it's being retired--I'm ceasing my involvement and there is no one else left who will be working on it. However, please see this thread which is the vote of the Apache PMC to retire the project. There will be a formal announcement forthcoming as a followup, but the decision itself has been concluded and PRs like this are the followup actions I'm taking as part of winding up my responsibilities.

As a point of reference, this is how Homebrew handles the deprecation: PR. It's marked deprecated but is otherwise still fully functional. A similar process for vcpkg would fill the gap here in the support vcpkg provides for deprecation.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Oct 29, 2022

Isn't it enough for now to put "Deprecated." at the begin of the description? (There could also be warning printed from the portfile.)

"dependencies": [
"xerces-c"
]
"version-string": "deprecated",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change makes it impossible to add more patches (port versions) to 1.12.

@BillyONeal
Copy link
Copy Markdown
Member

@ras0219-msft @JavierMatosD @markle11m @vicroms @AugP @valeriaconde, and I discussed this and agree to comply with what the upstream maintainer, @rleigh-codelibre desires. The port should be deleted from ports/ and from the baseline. Customers who want to still use this can pin an older version as older versions will still be present in the versions database.

@JavierMatosD and @markle11m are weakly against this without some sort of mention in November's vcpkg blog post.

@BillyONeal BillyONeal added info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Nov 3, 2022
@BillyONeal BillyONeal changed the title [xalan-c] deprecate port [xalan-c] delist port Nov 3, 2022
@ras0219-msft
Copy link
Copy Markdown
Contributor

Optionally we can delay this merge until a tool release with better removal diagnostics.

@BillyONeal
Copy link
Copy Markdown
Member

@ras0219-msft asked until next week to improve the diagnostic in the tool when we do this to merge this. Leaving it tagged reviewed to make sure we don't drop it on the floor.

@BillyONeal BillyONeal merged commit 299b1e1 into microsoft:master Nov 11, 2022
@BillyONeal
Copy link
Copy Markdown
Member

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-update The issue is with a library, which is requesting update new revision 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.

7 participants