More or less completely rewritten tensorflow-cc port #13028

jgehw · 2020-08-20T14:46:25Z

fixes static builds
upgrades tensorflow from v1.14 to recent v2.3, fixing [tensorflow] Update to 2.0 #9141
upgrades bazel from v0.25.2 to v3.1, hopefully fixing [tensorflow-cc] build failure #7995
requires PR [execute_process] Don't strip embedded semicolons #12926 (merged in this branch; need to resolve conflict once PR [execute_process] Don't strip embedded semicolons #12926 is merged into master)

…se customized bazel config is stored in wrong directory

- add patch to fix debug builds - add patch to fix exports for static linking - really build debug (instead of cloning release) - override bazel build options for debug (work around bazel bug) - bazel doesn't support static libraries: work around by building dynamic library and constructing static linkage commands from build log - Windows .pdb file can't be >4GB even on x64: work around using reduced debug information - Windows doesn't support .lib files >4GB even on x64, so split into multiple libs - vcpkg requires equal amount of libs for debug and release: work around using handcrafted empty dummy libs - fix naming of libs (.dll on Windows and .dylib on macOS)

- adapt patch files to tensorflow code changes - update bazel from v0.25.2 to v3.1 - on Windows use python installed on the host instead of embedded python obtained via vcpkg because embedded python lacks pip, which we need to obtain numpy - on Windows add MSYS2 to the PATH so that bazel tools can access MSYS2 GIT - add support for custom CA certificates when using HTTPS_PROXY

The existing implementation totally screwed up commands if the command's arguments contained semicolons (this is the case, e.g., in the FindPython modules of the cmake distribution).

…tually are use cases for this

incorporate changes from microsoft:master

…-more' as required for find_package(Python3)

…ing directory

Revert "incorporate changes from microsoft:master"

ports/tensorflow-cc/portfile.cmake

ras0219

This seems like a lot of novel work -- is this approach of static linking tensorflow coordinated with upstream at all?

Without upstream's blessing, I'm not sure this is something we can support over the long-term.

ports/tensorflow-cc/TensorflowCCConfig.cmake

ports/tensorflow-cc/fix-dbg-build-errors.patch

ports/tensorflow-cc/portfile.cmake

… file

…eading the last real libraries contents over the required number of libraries

…/vcpkg into fix-static-builds-and-more

jgehw · 2020-11-09T20:23:38Z

Prefer setting the IMPORTED_LOCATION when setting IMPORTED_LOCATION_RELEASE. The IMPORTED_LOCATION is used when the downstreams configuration type doesn't match the imported package. This will make it clear that RelWithDebInfo or MinSizeRel configurations should use the Release libraries instead of the Debug ones

@strega-nil So, we could just remove the if(@TENSORFLOW_HAS_RELEASE@) branches to make usage of IMPORTED_LOCATION_RELEASE fall back to already defined IMPORTED_LOCATION, right?

Imported libraries by default are scoped based on the import location. I am not 100% sure with vcpkg but you might want to make your import targets GLOBAL

@strega-nil I don't know how to apply this suggestion -- could you please take over?

…-and-more

…lt fall-back

strega-nil · 2020-11-11T17:08:11Z

@jgehw I'm having a weird issue, where the current build doesn't seem to actually generate any static libraries... do you know what the issue is?

jgehw · 2020-11-11T17:17:58Z

@jgehw I'm having a weird issue, where the current build doesn't seem to actually generate any static libraries... do you know what the issue is?

which platform? x64-windows-static?

strega-nil · 2020-11-11T17:19:47Z

Yeah. Are you on any IM platforms that I can talk to you on?

jgehw · 2020-11-11T17:42:19Z

Yeah. Are you on any IM platforms that I can talk to you on?

Ok, I'll rebuild on my machine to see if I can reproduce this issue.
Unfortunately, no, only email.

strega-nil · 2020-11-11T17:51:36Z

@jgehw I think I've figured it out -- it's due to an incorrect match (we check against stuff-part1-partN as opposed to stuff-partN in generate_static_link_cmd)

jgehw · 2020-11-11T17:51:40Z

I'm confident that your fix to ports/tensorflow-cc/generate_static_link_cmd_windows.py does the job. I probably broke it while also adding the "-partX" suffix to the first part.

strega-nil · 2020-11-11T17:52:25Z

@jgehw I'm doing the run now on my local machine -- me, being the dummy I am, wrote length instead of len haha!

jgehw · 2020-11-11T21:09:15Z

Found another place where the "-part1" suffix was necessary. Hopefully working now.

strega-nil · 2020-11-11T21:23:01Z

yeah, I did the same thing. I'll probably force push over yours.

….1 (microsoft#13028) * fix build issues occurring from default bazel config being used because customized bazel config is stored in wrong directory * [tensorflow-cc] Update CONTROL and ci.baseline.txt * fix also applies to windows static build * fix debug and static builds as well as library naming for non-linux - add patch to fix debug builds - add patch to fix exports for static linking - really build debug (instead of cloning release) - override bazel build options for debug (work around bazel bug) - bazel doesn't support static libraries: work around by building dynamic library and constructing static linkage commands from build log - Windows .pdb file can't be >4GB even on x64: work around using reduced debug information - Windows doesn't support .lib files >4GB even on x64, so split into multiple libs - vcpkg requires equal amount of libs for debug and release: work around using handcrafted empty dummy libs - fix naming of libs (.dll on Windows and .dylib on macOS) * upgrade tensorflow from v1.14 to v2.3 - adapt patch files to tensorflow code changes - update bazel from v0.25.2 to v3.1 - on Windows use python installed on the host instead of embedded python obtained via vcpkg because embedded python lacks pip, which we need to obtain numpy - on Windows add MSYS2 to the PATH so that bazel tools can access MSYS2 GIT - add support for custom CA certificates when using HTTPS_PROXY * fix execute process macro The existing implementation totally screwed up commands if the command's arguments contained semicolons (this is the case, e.g., in the FindPython modules of the cmake distribution). * extend overriden execute_process to more than one COMMAND as there actually are use cases for this * added another patch required for tensorflow v2.3, fixed path and working directory * Revert "incorporate changes from microsoft:master" * Revert "Revert "incorporate changes from microsoft:master"" * final fixes for static build + improving out messages * enabling linux and osx in CI to see if it works now * simplified code, fixed version numbers, fixed generated include cmake file * fix failing postbuild check on handcrafted empty dummy library by spreading the last real libraries contents over the required number of libraries * remove dead code commit by mistake again * improvements from code review * cleaner fix for debug code * find pip3 in PATH (PYTHON3_DIR apparently not valid for pip3) * fix error in python helper script * fix wrong libname in postbuild script * fix python detection + switch to python on msys2 (instead of embedded python) for Windows as we need numpy * fix order of arguments * fix command (it may contain spaces such as C:\Program Files\...) * revert last commit (root cause for CI failures is something different: there are line breaks in path) * fix regex comparision (value needs to be escaped as it may contains regex special characters such as brackets, eg C:/Program Files (x86)/...) * fix linebreaks in generated file * fix CRT linkage (macOS doesn't support static CRT linkage; it's set to dynamic even static target triplets for macOS and linux) * refactor implemenation to avoid as much code duplication as possible -- algorithmically identical * fix version numbers in helper scripts * enable work-around for Windows until bazel fix is available * install missing python3-pip on linux * fix linux build by patching * apply timeout feature now available via merged master branch * correct linux build patch * improve debug build patches (no functional difference because LOG(FATAL, ...) macro internally anyway calls abort(), which the compiler doesn't detect in debug mode... * improve linux patch * temporarily add debug to inspect what's going on on macOS CI * remove temporary debug code and fix static linking scripts for linux and macOS * fix regex escaping * fix ambiguous match while grepping for the framework link command * extend fix of ambiguous match while grepping for the framework link command * fix what merge of master broke * fix more what got broken by merging master (all packages and their dependencies are now maintained manually instead of using pacman...) * remove "unofficial" from filename * added switch do distinct classic and manifest mode when generating config.cmake file * create symlinks for libraries without version number * fix linux postbuild script * temporarily disable code making problems * add note for linking on Linux and macOS * forget to add README file in previous commit * add file forgotton in macro fixing patch * fix python library path * fix macOS static link command * update linkage instructions in README * Update ports/tensorflow-cc/CONTROL * Update ports/tensorflow-cc/portfile.cmake * Update scripts/ci.baseline.txt * use vcpkg_execute_required_process * pass C_FLAGS and CXX_FLAGS to bazel * fix INTERFACE_INCLUDE_DIRECTORIES * fix optional c/cxx arguments * also add linker opts * update README * merge static libs into one to support force_load (cannot force_load both due to duplicate symbols) * update README * quote python path (it might contain spaces that don't get escaped inside outer quotes of bash command) * fix python path also for static build * add arm(64) as currently unsupported arch * bazel 3.7 is available -> remove workaround * update README, remove necessary c-ares from deps * update msys package * add uwp specific options, and minor general improvements * fix string replace * fix control file and windows path separator * revert backslashes-fix -- the root cause was missing .exe extension * upgrade to tf 2.3.1 * fix hard-coded version * remove uwp work-in-progress code so that PR can be merged * update README and print out usage info in portfile * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-windows Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/portfile.cmake Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/portfile.cmake Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/portfile.cmake Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/portfile.cmake Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/portfile.cmake Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * improve usage hints as discussed in review * add comment * apply changes from review * make additional compiler / linker args space-proof * Update ports/tensorflow-cc/README-macos Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/tensorflow-cc-config-shared.cmake.in Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/tensorflow-cc-config-shared.cmake.in Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-linux Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * Update ports/tensorflow-cc/README-macos Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * apply changes from code review * maybe fix the config files * rob.maynard CRs * fix windows static lib naming for first part * Update ports/tensorflow-cc/generate_static_link_cmd_windows.py Co-authored-by: nicole mazzuca <mazzucan@outlook.com> * apply rob maynards 1st suggestion * nicole CRs * format/fix-compile * fix missing string termination * prefer IMPORTED_LOCATION over IMPORTED_LOCATION_RELEASE to have default fall-back * hopefully fix the issue where no libraries are generated * final stuff Co-authored-by: Gehweiler <Joachim_Gehweiler@McAfee.com> Co-authored-by: wangli28 <wangli28@beyondsoft.com> Co-authored-by: Jack·Boos·Yu <47264268+JackBoosY@users.noreply.github.com> Co-authored-by: Joachim Gehweiler <joachim@Joachims-iMac.local> Co-authored-by: nicole mazzuca <mazzucan@outlook.com>

Gehweiler and others added 18 commits July 23, 2020 21:53

fix build issues occurring from default bazel config being used becau…

0ab169d

…se customized bazel config is stored in wrong directory

[tensorflow-cc] Update CONTROL and ci.baseline.txt

543e19d

fix also applies to windows static build

820c455

fix execute process macro

dd6d89f

The existing implementation totally screwed up commands if the command's arguments contained semicolons (this is the case, e.g., in the FindPython modules of the cmake distribution).

extend overriden execute_process to more than one COMMAND as there ac…

0d611be

…tually are use cases for this

Merge pull request #1 from microsoft/master

d3121ce

incorporate changes from microsoft:master

merge branch 'fix-execute-process-macro2' into 'fix-static-builds-and…

ee969d7

…-more' as required for find_package(Python3)

added another patch required for tensorflow v2.3, fixed path and work…

ad5ca5f

…ing directory

Revert "incorporate changes from microsoft:master"

c743e49

Merge pull request #2 from jgehw/revert-1-master

2571bd1

Revert "incorporate changes from microsoft:master"

Revert "Revert "incorporate changes from microsoft:master""

98ecc0c

Merge branch 'master' of https://github.com/microsoft/vcpkg

ab8fdf7

Merge branch 'master' of https://github.com/microsoft/vcpkg

36143ac

final fixes for static build + improving out messages

ea07bb9

enabling linux and osx in CI to see if it works now

a12d22f

Merge branch 'master' into fix-static-builds-and-more

6c34a08

cenit reviewed Aug 20, 2020

View reviewed changes

ports/tensorflow-cc/portfile.cmake Outdated Show resolved Hide resolved

NancyLi1013 assigned JackBoosY Aug 21, 2020

ras0219 reviewed Aug 21, 2020

View reviewed changes

jgehw added 5 commits August 21, 2020 15:50

simplified code, fixed version numbers, fixed generated include cmake…

6e31cd6

… file

fix failing postbuild check on handcrafted empty dummy library by spr…

afc7f1e

…eading the last real libraries contents over the required number of libraries

remove dead code commit by mistake again

898bbbe

Merge branch 'fix-static-builds-and-more' of https://github.com/jgehw…

2ca1f94

…/vcpkg into fix-static-builds-and-more

Merge branch 'master' of https://github.com/microsoft/vcpkg

1866a82

apply rob maynards 1st suggestion

948faf1

nicole CRs

901938b

strega-nil approved these changes Nov 9, 2020

View reviewed changes

strega-nil and others added 4 commits November 9, 2020 16:17

Merge remote-tracking branch 'upstream/master' into fix-static-builds…

1c413c8

…-and-more

format/fix-compile

f636962

fix missing string termination

c46795f

prefer IMPORTED_LOCATION over IMPORTED_LOCATION_RELEASE to have defau…

842631e

…lt fall-back

JackBoosY requested a review from strega-nil November 11, 2020 02:17

hopefully fix the issue where no libraries are generated

dd8f4a9

final stuff

2fae377

strega-nil force-pushed the fix-static-builds-and-more branch from f9023d2 to 2fae377 Compare November 11, 2020 22:11

strega-nil merged commit 11b4a16 into microsoft:master Nov 12, 2020

jgehw deleted the fix-static-builds-and-more branch November 12, 2020 22:49

This was referenced Feb 23, 2021

[tensorflow] Update to 2.0 #9141

Closed

[TensorFlow] update to 2.3 #14176

Closed

jgehw mentioned this pull request May 25, 2021

[tensorflow-cc] build failure #7995

Closed

jgehw mentioned this pull request Aug 19, 2021

tensorflow-cc:x64-windows #19395

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

More or less completely rewritten tensorflow-cc port #13028

More or less completely rewritten tensorflow-cc port #13028

jgehw commented Aug 20, 2020

ras0219 left a comment

jgehw commented Nov 9, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

More or less completely rewritten tensorflow-cc port #13028

More or less completely rewritten tensorflow-cc port #13028

Conversation

jgehw commented Aug 20, 2020

ras0219 left a comment

Choose a reason for hiding this comment

jgehw commented Nov 9, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020

jgehw commented Nov 11, 2020

strega-nil commented Nov 11, 2020