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

[rsm-bsa] new port #19496

Merged
merged 29 commits into from
Sep 14, 2021
Merged

[rsm-bsa] new port #19496

merged 29 commits into from
Sep 14, 2021

Conversation

Guekka
Copy link
Contributor

@Guekka Guekka commented Aug 11, 2021

Add port for the BSA library

  • What does your PR fix?

Add new port. No one requested it

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

Supported triplets are x64 & (windows | linux) & static

Yes, CI updated

I think so

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

Yes

@ghost
Copy link

ghost commented Aug 11, 2021

CLA assistant check
All CLA requirements met.

@JonLiu1993 JonLiu1993 self-assigned this Aug 11, 2021
@JonLiu1993 JonLiu1993 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Aug 11, 2021
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 69478c5caafcde4c490bb1fccb960296801dbb5f -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index ecae6d2..75b776d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1076,6 +1076,10 @@
       "baseline": "1.11.0",
       "port-version": 0
     },
+    "bsa": {
+      "baseline": "2.0.2",
+      "port-version": 0
+    },
     "bsio": {
       "baseline": "1.0.0",
       "port-version": 0

scripts/ci.baseline.txt Outdated Show resolved Hide resolved
@JonLiu1993
Copy link
Member

Could you please resolve the conflicts against master?

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 63aa65e65b9d2c08772ea15d25fb8fdb0d32e557 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index d3bd6d2..2a57c63 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1076,14 +1076,14 @@
       "baseline": "1.11.0",
       "port-version": 0
     },
-    "bshoshany-thread-pool": {
-      "baseline": "1.9",
-	   port-version : 0
-	},
     "bsa": {
       "baseline": "2.0.2",
       "port-version": 0
     },
+    "bshoshany-thread-pool": {
+      "baseline": "1.9",
+      "port-version": 0
+    },
     "bsio": {
       "baseline": "1.0.0",
       "port-version": 0

@Guekka Guekka marked this pull request as draft August 12, 2021 09:53
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 63aa65e65b9d2c08772ea15d25fb8fdb0d32e557 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 0af07c5..2a57c63 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -1076,14 +1076,14 @@
       "baseline": "1.11.0",
       "port-version": 0
     },
-    "bshoshany-thread-pool": {
-      "baseline": "1.9",
-      "port-version": 0
-	},
     "bsa": {
       "baseline": "2.0.2",
       "port-version": 0
     },
+    "bshoshany-thread-pool": {
+      "baseline": "1.9",
+      "port-version": 0
+    },
     "bsio": {
       "baseline": "1.0.0",
       "port-version": 0

scripts/ci.baseline.txt Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
versions/b-/bsa.json Outdated Show resolved Hide resolved
Guekka and others added 2 commits August 12, 2021 10:20
@Guekka Guekka marked this pull request as ready for review August 12, 2021 10:43
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!

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 rsm-bsa but no changes to version or port version.
-- Version: 2.0.3
-- Old SHA: 59e8dee0afa59f367aed7653be1ac7c4bca6d138
-- New SHA: 465b19a5d463c1a9b54041b30600599ad0d5ac96
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

@JonLiu1993
Copy link
Member

@Guekka ,Could you please take a look?

[3/6] /usr/bin/c++  -I/mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/../include -I/mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/../src -isystem /mnt/vcpkg-ci/installed/x64-linux/include -fPIC -g -fvisibility=hidden -std=c++2a -MD -MT src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o -MF src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o.d -o src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o -c /mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/bsa/tes3.cpp
FAILED: src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o 
/usr/bin/c++  -I/mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/../include -I/mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/../src -isystem /mnt/vcpkg-ci/installed/x64-linux/include -fPIC -g -fvisibility=hidden -std=c++2a -MD -MT src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o -MF src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o.d -o src/CMakeFiles/bsa.dir/bsa/tes3.cpp.o -c /mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/bsa/tes3.cpp
In file included from /mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/bsa/tes3.cpp:1:
/mnt/vcpkg-ci/buildtrees/rsm-bsa/src/2.0.3-80256db965.clean/src/../include/bsa/tes3.hpp:3:10: fatal error: compare: No such file or directory
    3 | #include <compare>
      |          ^~~~~~~~~
compilation terminated.

@Guekka
Copy link
Contributor Author

Guekka commented Sep 10, 2021

@JonLiu1993

I already answered, actually. But I answered in a resolved conversation, so I guess you didn't get notified
As I've said previously, this port fails to build in CI because the GCC version is too old. I wanted to change ci_baseline to skip the Linux port, but you told me to remove Linux support instead
And @ras0219-msft made me change the support line again, adding back Linux

Sorry if I'm not clear. Basically, I'm asking whether I should :

  1. Change ci_baseline because it will work fine on people computers
  2. Remove Linux in support, even though the library supports Linux

@JackBoosY
Copy link
Contributor

We should not limit the user's gcc version, so the solution is as follows:

  1. When VCPKG_TARGET_IS_LINUX is ON in portfile.cmake, use a message to warn users of the minimum gcc version required.
  2. Add rsm-bsa:x64-linux=skip in scripts/ci.baseline.txt and write a related comment.

@Guekka
Copy link
Contributor Author

Guekka commented Sep 13, 2021

CI seems to work. Should I clean up the history?

@JackBoosY
Copy link
Contributor

@Guekka Please implement the first one of my suggestions.

@Guekka
Copy link
Contributor Author

Guekka commented Sep 14, 2021

@JackBoosY It has already been done in a previous commit
https://github.com/microsoft/vcpkg/pull/19496/files#diff-a16b669525fbc4d2650edeab0694190928c6582021f972a4cdf123ee12e22ec0R11-R13

@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Sep 14, 2021
@BillyONeal
Copy link
Member

Sorry for the confusion, I thought our VMs had Ubuntu 20.10 which has GCC 10.3 but they actually have 20.04 which is GCC 9.x.

… have G++10."

because we actually are on gcc 9.x :(

This reverts commit 781289d.
@BillyONeal BillyONeal merged commit 583ce28 into microsoft:master Sep 14, 2021
@BillyONeal
Copy link
Member

Thanks for the new port :)

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.

6 participants