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

[libical] new port #20965

Merged
merged 8 commits into from
Nov 19, 2021
Merged

[libical] new port #20965

merged 8 commits into from
Nov 19, 2021

Conversation

wrobelda
Copy link
Contributor

@wrobelda wrobelda commented Oct 24, 2021

Describe the pull request
Adds new LibIcal library. Builds on top of #8754

Depends on:

@PhoebeHui PhoebeHui self-assigned this Oct 24, 2021
@PhoebeHui PhoebeHui added category:new-port The issue is requesting a new library to be added; consider making a PR! depends:different-pr This PR or Issue depends on a PR which has been filed labels Oct 25, 2021
OPTIONS
-DCMAKE_DISABLE_FIND_PACKAGE_BDB=ON
-DUSE_BUILTIN_TZDATA=ON
-DICAL_GLIB=OFF
Copy link
Contributor

Choose a reason for hiding this comment

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

could be on, if glib is found

Copy link
Contributor Author

@wrobelda wrobelda Oct 25, 2021

Choose a reason for hiding this comment

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

That would require an additional dependency of Glib and LibXml2. I was thinking about making it a separate feature, but based on your readme1, it seems very experimental, so I gave up on it. I can try adding that feature, after all, but first need to fix the icu issue.

Footnotes

  1. The libical-glib API is currently unstable and can change with any release. Until it is considered stable, there should be defined LIBICAL_GLIB_UNSTABLE_API=1 before including <libical-glib/libical-glib.h>, to indicate that the library user is aware of it and is prepared to change the calls anytime. (https://github.com/libical/libical#introduction)

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@winterz tried adding "glib" feature, but getting:

CMake Error:
  Running

   '/Users/cromo/Documents/Sourcecode/vcpkg/downloads/tools/ninja-1.10.2-osx/ninja' '-C' '/Users/cromo/Documents/Sourcecode/vcpkg/buildtrees/libical/arm64-osx-dbg' '-t' 'recompact'

  failed with:

   ninja: error: build.ninja:2151: multiple rules generate lib/libical-glib.a [-w dupbuild=err]

Copy link

@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!

All manifest files must be formatted

./vcpkg format-manifest ports/*/vcpkg.json

Diff
diff --git a/ports/libical/vcpkg.json b/ports/libical/vcpkg.json
index 9eb1ba3..7ff86fb 100644
--- a/ports/libical/vcpkg.json
+++ b/ports/libical/vcpkg.json
@@ -16,10 +16,10 @@
   "features": {
     "rscale": {
       "description": "Support for RSCALE element",
+      "supports": "!static",
       "dependencies": [
         "icu"
-      ],
-      "supports": "!static"
+      ]
     }
   }
 }
PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for libical but no changes to version or port version.
-- Version: 3.0.11
-- Old SHA: 07874cb9f7940f73738bcd1d66eff755aefbc306
-- New SHA: f4ab3492c90b3d683702cacb90aaea839cfc3d8f
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@wrobelda
Copy link
Contributor Author

wrobelda commented Nov 13, 2021

@PhoebeHui this is ready to build now. I restricted the rscale to non-static builds for now, until the icu issue is resolved.

@PhoebeHui PhoebeHui removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Nov 17, 2021
Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

LGTM. BTW, since the feature 'rscale' has been restricted to non-static builds, the #20966 will not block this PR.

@PhoebeHui PhoebeHui added the info:reviewed Pull Request changes follow basic guidelines label Nov 18, 2021
@vicroms vicroms merged commit fa145bc into microsoft:master Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants