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

[tensorflow-cc] on x64 Windows with dynamic linkage misses C++ symbols in DLL #19364

Merged
merged 31 commits into from
Aug 12, 2021

Conversation

jgehw
Copy link
Contributor

@jgehw jgehw commented Aug 4, 2021

DLL appears to only export C API, but not C++ API. Always build static C++ library until this is fixed upstream.

Fixes #18887.

jgehw and others added 27 commits August 17, 2020 15:58
incorporate changes from microsoft:master
Revert "incorporate changes from microsoft:master"
because the DLL only exports the C API, not the C++ API;
remove this error again once this is fixed upstream
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 bd5ea16b97e91cb620fed0e10b7d9b3a8a943a52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 8cadbc5..460ae8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -6206,7 +6206,7 @@
     },
     "tensorflow-cc": {
       "baseline": "2.4.1",
-      "port-version": 0
+      "port-version": 1
     },
     "tensorflow-common": {
       "baseline": "2.4.1",
diff --git a/versions/t-/tensorflow-cc.json b/versions/t-/tensorflow-cc.json
index 1711cad..83dc7f2 100644
--- a/versions/t-/tensorflow-cc.json
+++ b/versions/t-/tensorflow-cc.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "984add9e42f747af35ee3cece3460482ed4dc77b",
+      "version-semver": "2.4.1",
+      "port-version": 1
+    },
     {
       "git-tree": "868e7ae57409669692df84ddb446f4a235e0b220",
       "version-semver": "2.4.1",

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 bd5ea16b97e91cb620fed0e10b7d9b3a8a943a52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/baseline.json b/versions/baseline.json
index 8cadbc5..460ae8d 100644
--- a/versions/baseline.json
+++ b/versions/baseline.json
@@ -6206,7 +6206,7 @@
     },
     "tensorflow-cc": {
       "baseline": "2.4.1",
-      "port-version": 0
+      "port-version": 1
     },
     "tensorflow-common": {
       "baseline": "2.4.1",
diff --git a/versions/t-/tensorflow-cc.json b/versions/t-/tensorflow-cc.json
index 1711cad..83dc7f2 100644
--- a/versions/t-/tensorflow-cc.json
+++ b/versions/t-/tensorflow-cc.json
@@ -1,5 +1,10 @@
 {
   "versions": [
+    {
+      "git-tree": "984add9e42f747af35ee3cece3460482ed4dc77b",
+      "version-semver": "2.4.1",
+      "port-version": 1
+    },
     {
       "git-tree": "868e7ae57409669692df84ddb446f4a235e0b220",
       "version-semver": "2.4.1",

ports/tensorflow-cc/portfile.cmake Outdated Show resolved Hide resolved
scripts/ci.baseline.txt Outdated Show resolved Hide resolved
Co-authored-by: Adam Johnson <AdamJohnso@gmail.com>
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 bd5ea16b97e91cb620fed0e10b7d9b3a8a943a52 -- versions
./vcpkg x-add-version --all
Diff
diff --git a/versions/t-/tensorflow-cc.json b/versions/t-/tensorflow-cc.json
index 83dc7f2..b43b8b2 100644
--- a/versions/t-/tensorflow-cc.json
+++ b/versions/t-/tensorflow-cc.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "984add9e42f747af35ee3cece3460482ed4dc77b",
+      "git-tree": "77875da04a151fb36abe52cce80491a9a7f16c55",
       "version-semver": "2.4.1",
       "port-version": 1
     },

@Hoikas
Copy link
Contributor

Hoikas commented Aug 4, 2021

Issue fatal error until this is fixed upstream.

With the changes, the library now always builds static on Windows, so the PR description should probably be updated 😉

@JackBoosY JackBoosY added 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 requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Aug 5, 2021
@JackBoosY
Copy link
Contributor

LGTM on my side.

@BillyONeal BillyONeal merged commit 32950ca into microsoft:master Aug 12, 2021
@BillyONeal
Copy link
Member

Thanks for your contribution!

@jgehw jgehw deleted the tf-cc-win-warning branch August 12, 2021 18:01
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 requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tensorflow] Remove tensorflow packet on Windows
4 participants