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

[lmdb] don't use msvc parameters with non-msvc compiler #23653

Merged
merged 8 commits into from
Mar 25, 2022

Conversation

bold84
Copy link
Contributor

@bold84 bold84 commented Mar 19, 2022

Describe the pull request

  • What does your PR fix?

    Fixes failed build on non-windows-dynamic

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    <all / linux, windows, ...>, <Yes/No>

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

@bold84 bold84 changed the title don't use msvc parameters with non-msvc compiler [lmdb] don't use msvc parameters with non-msvc compiler Mar 19, 2022
ports/lmdb/vcpkg.json Outdated Show resolved Hide resolved
Comment on lines 4 to 5
"git-tree": "5ed5bd2c01a6c4467f9be85807e7727666ddb5b7",
"version": "0.9.24",
Copy link
Contributor

Choose a reason for hiding this comment

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

please revert this and run vcpkg x-add-version lmdb again after you committed this and the other changes.

@@ -20,8 +20,8 @@ set(LMDB_LIBRARY_INSTALL_DIR lib CACHE PATH "Install directory for library")
set(LMDB_RUNTIME_INSTALL_DIR bin CACHE PATH "Install directory for binaries/dlls")
set(LMDB_CONFIG_INSTALL_DIR share/lmdb CACHE PATH "Install directory for cmake config files")


if(BUILD_SHARED_LIBS)
message("Compiler: ${CMAKE_CXX_COMPILER_ID}")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this message needed?

Copy link
Contributor Author

@bold84 bold84 Mar 19, 2022

Choose a reason for hiding this comment

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

It isn't. Forgot to remove it...

@JonLiu1993 JonLiu1993 added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Mar 21, 2022
@JonLiu1993
Copy link
Member

@bold84 ,Thanks for your pr, please consider @Thomas1664's review suggesstion

@bold84
Copy link
Contributor Author

bold84 commented Mar 25, 2022

@bold84 ,Thanks for your pr, please consider @Thomas1664's review suggesstion

Sure, it's just a pita that for a one line code change, it is required to do these version things that I have not found documentation for. Where is this version stuff documented?

For another PR previously, I was not supposed to increase the version. But for this one it seems it's required.

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!

After committing all other changes, the version database must be updated
git add -u && git commit
git checkout a76eb002a71b6cf7bad343f5e3376dfe6bb83c5c -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/lmdb.json b/versions/l-/lmdb.json
index 876c69c..b92ae50 100644
--- a/versions/l-/lmdb.json
+++ b/versions/l-/lmdb.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "e6ffab6abd2f4953adaf29622af857fab8dbd359",
+      "git-tree": "9af408cdb98ca24cfe075eacc661172c2be53a0f",
       "version": "0.9.24",
       "port-version": 2
     },

You have modified or added at least one vcpkg.json where a "license" field is missing.

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

  • ports/lmdb/vcpkg.json

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

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.

You have modified or added at least one vcpkg.json where a "license" field is missing.

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

  • ports/lmdb/vcpkg.json

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

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/lmdb/vcpkg.json b/ports/lmdb/vcpkg.json
index ed3f84a..2c2b590 100644
--- a/ports/lmdb/vcpkg.json
+++ b/ports/lmdb/vcpkg.json
@@ -3,6 +3,6 @@
   "version": "0.9.24",
   "port-version": 2,
   "description": "LMDB is an extraordinarily fast, memory-efficient database",
-  "license": null,
-  "homepage": "https://github.com/LMDB/lmdb"
+  "homepage": "https://github.com/LMDB/lmdb",
+  "license": null
 }
After committing all other changes, the version database must be updated
git add -u && git commit
git checkout a76eb002a71b6cf7bad343f5e3376dfe6bb83c5c -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/l-/lmdb.json b/versions/l-/lmdb.json
index b92ae50..5166dd8 100644
--- a/versions/l-/lmdb.json
+++ b/versions/l-/lmdb.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "9af408cdb98ca24cfe075eacc661172c2be53a0f",
+      "git-tree": "022fa44ff8c5ef9ea4b308468259c0775d4a72c1",
       "version": "0.9.24",
       "port-version": 2
     },

@JonLiu1993 JonLiu1993 added info:reviewed Pull Request changes follow basic guidelines and removed requires:author-response labels Mar 25, 2022
@dan-shaw dan-shaw merged commit a95ee82 into microsoft:master Mar 25, 2022
ekilmer added a commit to ekilmer/vcpkg that referenced this pull request Mar 26, 2022
* master: (103 commits)
  [lmdb] don't use msvc parameters with non-msvc compiler (microsoft#23653)
  Fix <version> of Python in vcpkgTools.xml (microsoft#23751)
  [sciter] escape quotes (microsoft#23752)
  [fuzzylite] Fix Linux build (microsoft#23658)
  [cpp-redis] Fix ‘sleep_for’ is not a member of ‘std::this_thread’ (microsoft#23762)
  [nlohmann-json] Fix usage text (microsoft#23749)
  [fontconfig] Do not create symlinks (microsoft#23735) (microsoft#23736)
  [winsparkle] Fix header file and debug path (microsoft#23739)
  [arb] Support dynamic build (microsoft#23743)
  [aws-sdk-cpp] update to 1.9.220 (microsoft#23729)
  Fix the VS2022 'unstable' queues. (microsoft#23742)
  [xmlsec] Bump to 1.2.33 (microsoft#23733)
  [unicorn] update to latest version v1.0.3 (microsoft#23745)
  [libpq] Update version to 14.1 2 (microsoft#22516)
  [libnoise] Export CMake files (microsoft#23682)
  [vcpk-ci] Trigger some test ports from vcpkg.cmake changes (microsoft#23430)
  [nlohmann-json] Add option to control implicit conversions behaviour (microsoft#22409)
  [libjuice] Update to 0.9.8 (microsoft#23153)
  [libgeotiff] Update to 1.7.1 (microsoft#23446)
  [minizip-ng] Updated minizip version and fixed windows build for previous version (microsoft#23684)
  ...
@bold84 bold84 deleted the lmdb_shared_linux_fix branch April 17, 2022 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants