Created attachment 183293 [details] bazel_clang38 This is a slave port of bazel with clang38 as default crosstool.I make this port for compiling tensorflow on FreeBSD 10.x.Tensorflow do not compile on FreeBSD 10.x with Clang3.4. see more for tensorflow PR:https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219609 this port depend on master port PR:https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219838
And I test pass this port on poudriere with 10.3R-amd64
Created attachment 183321 [details] bazel_clang38_v2.shar the parent port using "?=" instead of "+=",I update the slave port.
Created attachment 183345 [details] bazel_clang38.shar v3 patch: add clang38 as run time depends
Created attachment 183781 [details] bazel_clang38.shar changes: 1.get rid of /proc mount when build c/c++, this will make science/tensorflow build on poudriere without "USE_PROCFS=yes". 2.make this port not a slave port of /devel/bazel,so we do not need the devel/bazel do any patch(bug 219838 can be closed).
Created attachment 183783 [details] bazel_clang38.shar add missing lines.
Does it really make sense to have this as a self-contained port not building on devel/bazel? Makefile, distinfo, pkg-plist, and pkg-descr are essentially a copy of those of devel/bazel; thus the maintenance effort would be doubled which it wouldn't with your initial proposal of depending on devel/bazel. (As mentioned on bug #219838, I have no objections of making BUILD_DEPENDS overridable.)
(In reply to Klaus Aehlig from comment #6) There are two differences compare with /devel/bazel: 1.use port clang38 as default c/c++ compiler.This is needed to build tensorflow on FreeBSD 10.x. 2.patch the bazel to get rid of /proc depends.This is needed to build with unprivileged user or poudriere without USE_PROCFS=yes. For 1 there no one commits the patch so far for the last several weeks. For 2 I make a pull request on github to bazel,but the review work is too slow(I find you are happened to be the reviewer). I am pushing the tensorflow to ports and hope it is accepted before Q3 quarterly branch cut, I think making this port standalone make it easier to go forward. I will maintain this port as long as it is useful. I think after 10.x EOL and the 2 accepted,I can change tensorflow to depend on /devel/bazel,then this port will not be needed.
1. is entirely up to the committer, but keep in mind that committer can commit patches to other ports as a preparation step; as mentioned, I don't have any objections to this change (but I'm not a committer). 2. Why not use the EXTRA_PATCHES += construct? (Side remark: your pull request was sent after bazel 0.5.2 baseline was cut and for 0.5.3 we're still in time, so the time for review did not at all influence which bazel releases contain that patch.) In the end, I can live with the fork, but I don't think it's good for the ports tree to duplicate instead of reuse. But that, again, is up to the committers to decide.
(In reply to Klaus Aehlig from comment #8) > 1. is entirely up to the committer, but keep in mind that committer can commit patches to other ports as a preparation step; as mentioned, I don't have any objections to this change (but I'm not a committer). > > 2. Why not use the EXTRA_PATCHES += construct? I would like to see this becomes a OPTION and contains in the master port. We can make devel/bazel_clang38 as a slave port for devel/bazel with that OPTION on. This reduces the common codes in the ports tree. A slave port gives us a pre-built package and makes the port depends on it work. > (Side remark: your pull request was sent after bazel 0.5.2 baseline was cut and for 0.5.3 we're still in time, so the time for review did not at all influence which bazel releases contain that patch.) Upstreaming as much as we can is always a good idea. > In the end, I can live with the fork, but I don't think it's good for the ports tree to duplicate instead of reuse. But that, again, is up to the committers to decide. I think having fork for a short time to make the thing we are interested work while working with other parties is acceptable, or sometimes we just cannot make things happen. Some other comments: - We need to set CONFLICTS_INSTALL in both ports because this port and devel/bazel both instal bin/bazel - I would rename this port to devel/bazel-clang38, and using -clang38 in PKGNAMESUFFIX.
Created attachment 184324 [details] bazel-clang38.shar rename to bazel-clang38 and add use PKGNAMESUFFIX
(In reply to Li-Wen Hsu from comment #9) Hi Li-Wen, thanks for your input! I have updated the shar file to use '-' instead '_' in the port name, and use PKGNAMESUFFIX. Poudriere testport passed. For the CONFLICTS_INSTALL, I think it is not necessary to add it to both ports as the conflict will work if either port has this set. This is a test on my host: root@xx:~ # pkg install bazel Updating FreeBSD repository catalogue... FreeBSD repository is up to date. All repositories are up to date. The following 1 package(s) will be affected (of 0 checked): New packages to be INSTALLED: bazel: 0.5.0 Number of packages to be installed: 1 The process will require 127 MiB more space. 124 MiB to be downloaded. Proceed with this action? [y/N]: y [1/1] Fetching bazel-0.5.0.txz: 100% 124 MiB 314.3kB/s 06:53 Checking integrity... done (1 conflicting) - bazel-0.5.0 conflicts with bazel-clang38-0.5.0 on /usr/local/bin/bazel Checking integrity... done (0 conflicting) Conflicts with the existing packages have been found. One more solver iteration is needed to resolve them. The following 2 package(s) will be affected (of 0 checked): Installed packages to be REMOVED: bazel-clang38-0.5.0 New packages to be INSTALLED: bazel: 0.5.0 Number of packages to be removed: 1 Number of packages to be installed: 1 Proceed with this action? [y/N]:
Created attachment 184325 [details] bazel-clang38.shar reorder the PKGNAMESUFFIX to make portlint happy QA: portlint -AC WARN: Makefile: [21]: possible direct use of command "file" found. use ${FILE} instead. 0 fatal errors and 1 warning found.
Thanks! For CONFLICTS_INSTALL, I need check again, I think it is need for using `make install` And how about merging to bazel as an OPTION thing?
(In reply to Li-Wen Hsu from comment #13) If the option is set to OFF by default, the tensorflow can not be built.And also it is not reasonable to set to ON by default because of allways llvm38 depends.
(In reply to Jov from comment #14) I understand, what I mean is: devel/bazel: has an option, ex: CLANG38, default off devel/bazel-clang38: set CLANG38, dfault on And we set tensorflow depends on devel/bazel-clang38 Does this make sense to you?
(In reply to Li-Wen Hsu from comment #15) I think It's OK. Hi aehlig, is this make sense to you.
Created attachment 184421 [details] Adding a clang38 option to devel/bazel As discussed, it seems reasonable to add an option to use clang38 to devel/bazel, so that that option can be used by a slave port. If desired, we can also use https://github.com/bazelbuild/bazel/commit/fa46172bc3b9bd2398a9d79a05fc5cd55f6059ad as an additional patch, but a version of bazel containing this patch will be released soon anyway.
(In reply to Jov from comment #16) > Hi aehlig, is this make sense to you. Yes, I'm always a fan of avoiding code duplication. I've added a diff how adding such an option to devel/bazel might look like and I'm happy with this being committed to devel/bazel as part of adding devel/bazel-clang38 as a slave port.
(In reply to Klaus Aehlig from comment #18) Thanks, Klaus! I will reopen 219838 and update the patch to add the clang38 option.
Created attachment 184498 [details] bazel-clang38.shar Make this is a slave port of devel/bazel.
A commit references this bug: Author: lwhsu Date: Sun Jul 30 21:46:10 UTC 2017 New revision: 446943 URL: https://svnweb.freebsd.org/changeset/ports/446943 Log: Add bazel-clang38: using clang38 as crosstool for bazel PR: 219840 Submitted by: Jov <amutu@amutu.com> Changes: head/devel/Makefile head/devel/bazel-clang38/ head/devel/bazel-clang38/Makefile