Created attachment 182983 [details] tensorflow.shar dirty port of tensorflow: TensorFlow is an open source software library for numerical computation using data flow graphs. Nodes in the graph represent mathematical operations, while the graph edges represent the multidimensional data arrays (tensors) communicated between them. The flexible architecture allows you to deploy computation to one or more CPUs or GPUs in a desktop, server, or mobile device with a single API. TensorFlow was originally developed by researchers and engineers working on the Google Brain Team within Google's Machine Intelligence research organization for the purposes of conducting machine learning and deep neural networks research, but the system is general enough to be applicable in a wide variety of other domains as well. WWW: https://www.tensorflow.org
Thank you for your submission Jov. Can you please confirm that this port passes QA (portlint and poudriere in particular).
(In reply to Kubilay Kocak from comment #1) portlint pass and has 2 warining,I will submit v2 patch which has 1 warning: WARN: Makefile: possible use of absolute pathname "/proc)". caused by: mount -t procfs proc /proc I don't know how to get rid of this,but I think it's harmless? I do not run poudriere for this port because the mount /proc and also bazel download should do some configuration to poudriere.I create this port on 11.0-RELEASE and test the build on a fresh installed 12-current vm(installed using FreeBSD-12.0-CURRENT-amd64-20170510-r318137-disc1.iso).So for 11.0/12 it works. this port do the build mainly as this post： https://forums.freebsd.org/threads/59479/ v2 also restrict the build only on 11.0+.
Created attachment 183004 [details] tensorflow.shar v2 only build on 11+ add run time depends using pip package meta info
Created attachment 183026 [details] tensorflow.shar v3 v3 patch 1.mv output of bazel to WRKDIR. the default output of bazel is /root/.cache which out of WRKDIR,may left garbage even after make clean. 2.patch the src after confiure,then build.v1 and v2 patch do the sequence of build-failed-patch-build,very dirty.
Initial review: 1) USES=python implies {RUN,BUILD}_DEPENDS on Python (can remove python:lang/python). Additionally, use of python:lang/python is always incorrect, as it only means 'depends on the `python` symlink' not 'depends on python'. Only use USES=python:<args> 2) Upstream stipulates Python 2.7 *and* 3.3+ support. USES=python:2.7 limits it to 2.7 only. 3) Regarding BROKEN (${OSVERSION} < 1100101): 3a) clang and llvm 3.8 exist in ports and can be depended on. If there is nothing intrinsic or super-complex preventing other versions of FreeBSD from building tensorflow, it should do so. 3b) If clang is not a requirement for building tensorflow, it should not be a specific dependency. I see no mention of it on the 'building tensorflow from sources' document, but further, there *is* mention of builds with GCC, which motivates this comment. USES=compiler:<args> should be used if a specific feature is required (See Porters Handbook -- USES -- compiler for details [1]) 4) Vendored dependencies should be extracted/removed and those ports/packages in the tree depended on instead. 5) It's unlikely that all python dependencies are both RUN and BUILD dependencies. Please make these as accurate to their actual dependency types as possible. 6) -march=native should not be used for CC/CFLAGS. Binary packages are built from ports and build host CPU architecture has no bearing on end-user hardware, which may be different. 7) configuration (in this case ./configure) steps should be run in the do-configure target, not the build target. Additionally and ideally, using CONFIGURE_TARGET and other CONFIGURE_* variables, without requiring a custom do-configure target override. If bazel is a pre build or 'configuration' step, it should be done in the configure target as well. 8) Patching of WRKSRC files should occur in {pre,post}-patch target, not build target. 9) Was using the port/package of pip tested rather than configuring/building the bundled tensorflow/tools/pip_package? The former should be used if and where possible. This port requires additional QA iterations [1] https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#uses-compiler
(In reply to Kubilay Kocak from comment #5) Thanks for your review,from your comment I learn more about freebsd ports,I appreciate it! My reply is inlined: >Initial review: >1) USES=python implies {RUN,BUILD}_DEPENDS on Python (can remove python:lang/python). Additionally, use of python:lang/python is always incorrect, as it only means 'depends on the `python` symlink' not 'depends on python'. Only use USES=python:<args> will fix >2) Upstream stipulates Python 2.7 *and* 3.3+ support. USES=python:2.7 limits it to 2.7 only. will fix by using USES=python:2.7+,and test python3. >3) Regarding BROKEN (${OSVERSION} < 1100101): >3a) clang and llvm 3.8 exist in ports and can be depended on. If there is nothing intrinsic or super-complex preventing other versions of FreeBSD from building tensorflow, it should do so. see blow 3b >3b) If clang is not a requirement for building tensorflow, it should not be a specific dependency. I see no mention of it on the 'building tensorflow from sources' document, but further, there *is* mention of builds with GCC, which motivates this comment. I have try to use gcc but it is failed for bazel to find gcc.If I set --compiler=gcc-5.4.0,bazel report error: (cd /usr/ports/science/tensorflow/work/tensorflow-1.1.0 && bazel --output_user_root=/usr/ports/science/tensorflow/work/bazel_ot build --compiler=gcc-5.4.0 --config=opt //tensorflow/tools/pip_package:build_pip_package --verbose_failures) ERROR: No toolchain found for --cpu='freebsd' --compiler='gcc-5.4.0'. Valid toolchains are: [ --cpu='armeabi-v7a' --compiler='compiler' --glibc='armeabi-v7a', --cpu='local' --compiler='compiler' --glibc='local', --cpu='darwin' --compiler='compiler' --glibc='macosx', --cpu='freebsd' --compiler='compiler' --glibc='local', --cpu='x64_windows' --compiler='windows_mingw' --glibc='local', --cpu='x64_windows' --compiler='windows_msys64_mingw64' --glibc='local', --cpu='x64_windows' --compiler='windows_clang' --glibc='local', --cpu='x64_windows' --compiler='windows_msys64' --glibc='local', --cpu='x64_windows_msvc' --compiler='cl' --glibc='msvcrt140', --cpu='ios_x86_64' --compiler='compiler' --glibc='ios', After read this,I think I need to write a toolchain config to work with gcc: https://github.com/bazelbuild/bazel/wiki/Building-with-a-custom-toolchain >USES=compiler:<args> should be used if a specific feature is required (See Porters Handbook -- USES -- compiler for details [1]) will fix after I solve 3b. >4) Vendored dependencies should be extracted/removed and those ports/packages in the tree depended on instead. tensorflow use bazel manage dependencises. the tensorflow-1.1.0/tensorflow/workspace.bzl contains dependencises info like this: native.new_http_archive( name = "org_pocoo_werkzeug", urls = [ "http://bazel-mirror.storage.googleapis.com/pypi.python.org/packages/b7/7f/44d3cfe5a12ba002b253f6985a4477edfa66da53787a2a838a40f6415263/Werkzeug-0.11.10.tar.gz", "https://pypi.python.org/packages/b7/7f/44d3cfe5a12ba002b253f6985a4477edfa66da53787a2a838a40f6415263/Werkzeug-0.11.10.tar.gz", ], strip_prefix = "Werkzeug-0.11.10", sha256 = "cc64dafbacc716cdd42503cf6c44cb5a35576443d82f29f6829e5c49264aeeee", build_file = str(Label("//third_party:werkzeug.BUILD")), ) And the count of dependencies is bigger than normal ports： grep -c 'sha256 =' workspace.bzl 90 some dependencises even are not available on FreeBSD ports,like grpc. It is a lot work to make this fixed and need to patch the bazel config ,this will make future update difficult also. >5) It's unlikely that all python dependencies are both RUN and BUILD dependencies. Please make these as accurate to their actual dependency types as possible. will fixed >6) -march=native should not be used for CC/CFLAGS. Binary packages are built from ports and build host CPU architecture has no bearing on end-user hardware, which may be different. will fixed by using $CFLAGS >7) configuration (in this case ./configure) steps should be run in the do-configure target, not the build target. Additionally and ideally, using CONFIGURE_TARGET and other CONFIGURE_* variables, without requiring a custom do-configure target override. If bazel is a pre build or 'configuration' step, it should be done in the configure target as well. I hope to do this as you say,but the configure of this port is not gnu autoconfig or similar,it is a bash script! It used to read user input from stdin and set some variables then do bazel fetch. I will fix this by moving configure to do-configure target. >8) Patching of WRKSRC files should occur in {pre,post}-patch target, not build target. As 4 and 7 mentioned, the configure will fetch all dependencises and the patch is for the grpc and protobuf which only are available after configure. So I will fix tis by moving patch to post-configure target. >9) Was using the port/package of pip tested rather than configuring/building the bundled tensorflow/tools/pip_package? The former should be used if and where possible. yes I understand.but there is no pip package for freebsd,tensorflow offical suport is linux and macos.tensorflow/tools/pip_package will build one，it will include all the dependencises python lib and C/C++ libs to the pip package,you can see the pkg-plist. >This port requires additional QA iterations I will setup poudriere to test this when all of the above fixed.
Quick review, the mount /proc has to go. Ports are built as a regular user, there is no way it can possibly ever do anything.
Created attachment 183346 [details] tensorflow_v4.shar v4 patch: 1.fix build for freebsd 10.x 2.add python prefix to pkg name 3.clean up build depends and run depends 4.move configure phase to do-configure 5.respect CFLAGS 6.fix build for i386 7.use bazel batch mode so not left java process after build QA: portlint:no error,1 warning for mount /proc poudriere test port tested and pass: 11.0R-amd64-py36 11.0R-amd64-py27 11.0R-i386-py27 10.3R-amd64-py27
(In reply to Kubilay Kocak from comment #5) Hi Kubilay, I try to remove the python:lang/python from BUILD_DEPENDS as you say, the build will fail to find /usr/local/bin/python for testport.And I find python27 do not depend on lang/python so I think the USES= python:2.7+ do not install lang/python.So I do not remove the python depends.
(In reply to Jov from comment #9) Software packages should be PEP-394 [1] compatible in terms of how they invoke Python. Additionally, for Python package/software ports in FreeBSD, they should invoke a specific Python version (pythonX.Y, see variable PYTHON_CMD) and be as PEP-394 compliant as possible. Both of the above apply at build-time and run-time. Where `python` points (or if it is present at all) can change at any time at the discretion of the user. That aside, if and when present, can only happen to point to a 'default' version of Python, which has no necessary correlation with what Python environment version the software has been installed in. For files/scripts, maintainers may use USES=shebangfix [2] and SHEBANG_FILES to correct shebang lines [1] https://www.python.org/dev/peps/pep-0394/ [2] https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/book.html#uses-shebangfix
(In reply to Kubilay Kocak from comment #10) Thank you for your comment.I fixed the problem and will submit v5 patch. By the way, is there any wiki pages or articles about the python port FAQ?
Created attachment 183358 [details] tensorflow_v5.shar v5 patch: 1.remove lang/python depend 2.add python shebangfix test pass poudriere 11R-amd64
The python_CMD definition is not needed, remove it. post-patch and post-configure: Never call SED directly, we have REINPLACE_CMD for that. The procfs mount must be removed. Packages are build as regular user, no regular user can ever mount anything. Also, they are built in jails where mounting is not allowed, so even if root was building, it could not mount either. Please merge do-configure and post-configure, there is no reason to have two targets when only one is needed. In do-configure, you seem to be setting enviroment variables without using SETENV, please add it. do-build, same, use SETENV. The do-install target is a bit strange, it looks like you are "installing" files by moving them with MV. you should instead be copying them with COPYTREE_SHARE.
(In reply to Mathieu Arnold from comment #13) Thanks Mathieu!all fixed by v6 patch
Created attachment 183372 [details] tensorflow_v6.shar 1.remove python_CMD 2.use REINPLACE_CMD instead of SED 3.remove mount /proc 4.merge do-configure and post-configure 5.use SETENV to set environment variables 6.use COPYTREE_SHARE instead of MV
(In reply to Jov from comment #15) Also poudriere testport pass
You should not overwrite do-patch but use post-patch for the REINPLACE_CMD.
NEW patch move to here： https://reviews.freebsd.org/D11194
Created attachment 183637 [details] tensorflow_v7.shar v7: 1.fix network access at do-configure 2.update port version to 1.2.0, which release 2 days ago.
Created attachment 183701 [details] tensorflow_v8.shar v8: fix tensorflow-1.2.0 runtime depends, these dependencies com from the pip package meta data. Upstream 1.2.0 add a new runtime depend named backports.weakref, this software do not port to FreeBSD, so I port it. PR:https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220206
Created attachment 183782 [details] tensorflow_v9.shar As I make the bazel_clang38 port do not use /porc mount，see:https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219840#c4, now tensorflow depend bazel_clang38 on all version so we can build on all version without monting /proc.
Created attachment 183836 [details] tensorflow_v10.shar V10: per lwshu@, use PATCHFILES to download large patch files.
Created attachment 183901 [details] tensorflow_v11.shar sync with https://reviews.freebsd.org/D11194: 1.add version to external patch files per lwhsu@ 2.remove unnecessary SHEBANG_LANG and merge sed cmd per mat@
Created attachment 184298 [details] tensorflow.diff sync patch with https://reviews.freebsd.org/D11194
Created attachment 184326 [details] tensorflow.diff Change the build time depend bazel_clang38 to bazel-clang38.
Could you please update to the current version 1.2.1?
(In reply to Yuri Victorovich from comment #26) I will update the version after the blocker solved: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219840 If you really want to use 1.2.1,you can try to change the PORTVERSION to 1.2.1 and make makesum, then make install. The offline third party should work as someone tested it on Linux.
IMO, 219840 shouldn't be a blocker for tensorflow. You should add : > .if ${OSVERSION} >= 1100000 > BUILD_DEPENDS+=bazel:devel/bazel > USE_PROCFS=yes > .else > BROKEN "Waiting for devel/bazel-clang38, see bug#219840" > .endif No need to block it for everybody for non-essential reasons.
Also, did you consider naming it science/py-tensorflow? Everything it installs is under site-packages/.
(In reply to Yuri Victorovich from comment #28) For broken, please see comment 5. For procfs, please see comment 7.
(In reply to Yuri Victorovich from comment #29) The package name already contains pyXX-.
(In reply to Jov from comment #31) The directory should be science/py-tensorflow Look how project depending on tensor lists it in setup.py: https://github.com/tensorflow/magenta/blob/master/magenta/tools/pip/setup.py#L38 It is listed among other python packages. Also all files it installs are under site-packages/.
Created attachment 184500 [details] py-tensorflow.shar 1.update to tensorflow-1.2.1. 2.change dirname to py-tensorflow. 3.add freebsd version condition to make 11.x+ use devel/bazel install bazel-clang38 because procfs patch already included. 4.add PLIST_SUB to make plist file easy maintain when update.
Hi Kubilay, I see you added 219794 as a dependent, but I think PR 219794 do nothing with Tensorflow. Tensorflow builds and works well when PR 219794 unresolved. It should only block PRs about py-backports*. Is it possible to remove the dependency relations, and let this PR move on?
(In reply to Jov from comment #34) +1 I agree. IMO, delete the dependency and commit.
(In reply to Jov from comment #34) I can't explain why I added it (possible typo) If this port doesn't provide or touch backports directories or files, it doesn't depend, otherwise it does.
I see why: tensorflow depends on backports.weakref which led to bug 220749 which was closed duplicate of bug 219794 Since backports.weakref is already in the tree (with its conflicts) resolving bug 219794 is not an 'actual' blocker, *as long* as installing tensorflow does not install two dependencies that each depend on different and conflicting backports ports. I believe this would have come up in testing if it did.
A commit references this bug: Author: lwhsu Date: Tue Aug 15 12:08:03 UTC 2017 New revision: 447979 URL: https://svnweb.freebsd.org/changeset/ports/447979 Log: Ad TensorFlow 1.2.1 PR: 219609 Submitted by: Jov <amutu@amutu.com> Changes: head/science/Makefile head/science/py-tensorflow/ head/science/py-tensorflow/Makefile head/science/py-tensorflow/distinfo head/science/py-tensorflow/files/ head/science/py-tensorflow/files/patch-WORKSPACE head/science/py-tensorflow/files/patch-tensorflow__third__party_grpc_include_grpc_impl_codegen_port__platform.h head/science/py-tensorflow/files/patch-tensorflow__third__party_protobuf_src_google_protobuf_compiler_plugin.pb.cc head/science/py-tensorflow/files/patch-tensorflow__third__party_protobuf_src_google_protobuf_compiler_plugin.pb.h head/science/py-tensorflow/files/patch-tensorflow_workspace.bzl head/science/py-tensorflow/pkg-descr head/science/py-tensorflow/pkg-plist
With minor portlint(1) changes.
Great work! Thanks, Li Wen!