Created attachment 174316 [details] Proposed patch (since 421246 revision) Patch to improve devel/llvm-devel port. - Bump PORTREVISION - Add license information - Use VAR option helper for _USES_PYTHON variable - Simplify PORTDOCS with * value - Add UTILS (default) option to install utility binaries (e.g. count, FileCheck, not, etc.) - Change description for LIT option - Add dependency on UTILS for LIT option - Use new GH_SUBDIR option helper, instead of post-extract-* targets - Add libxml2 to USE_GNOME for CLANG option, because c-index-test requires this, according to stage-qa - Sort variables for options (e.g. GOLD_*) and change placement of some variables - Remove bsd.port.options.mk include - Add changes to .MAKEFLAGS in case of makesum target to fetch all optional distfiles for distinfo file creation, independent from selected options - Replace blank space(s) with tab for some cases - Build compiler-rt as a project in projects/compiler-rt directory - Remove COMMANDS variable, check-commands target and simplify llvm-wrapper.sh installation in post-install-script target - Replace contents of build-plist target with dynamic approach and POST_PLIST usage - Remove regression-test target; use new TEST_TARGET and TEST_WRKSRC variables for `make test` - Remove post-patch target for lit changes, which allows usage of testing targets - Install lit with using utils/lit/setup.py; add Python setuptools to build dependency for LIT option - Remove OPTIONS_SUB, PLIST_FILES and pkg-plist file - Remove curl and jq requirement with using available commands in files/gen-Makefile.snapshot.sh script - Add files/llvm-lit-wrapper.sh.in, which contains PYTHONPATH environment variable, for correct (llvm-)lit${LLVM_SUFFIX} installation and usage The build tested on FreeBSD 10.2 amd64.
I set TEST_TARGET to "check" value (TEST_TARGET=check), which does LLVM regression tests by default. Possible to set TEST_TARGET to "check-all" value, to run available tests, depending on selected options for port. But I tried this and some (sanitizer) test didn't stop, so I reverted it to "check" value. Example of output for LLVM regression tests (TEST_TARGET=check): -8<-- % make test ... [0/1] Running the LLVM regression tests Testing Time: 474.86s Expected Passes : 17246 Expected Failures : 125 Unsupported Tests : 372 -->8- Example of output for Clang regression tests: -8<-- % make TEST_TARGET=check-clang test ... [0/1] Running the Clang regression tests ... ******************** Testing Time: 310.77s ******************** Failing Tests (284): ... Expected Passes : 9410 Expected Failures : 16 Unsupported Tests : 42 Unexpected Failures: 284 ... -->8- Example of output for Clang extra tools' regression tests: -8<-- % make TEST_TARGET=check-clang-tools test ... [0/1] Running the Clang extra tools' regression tests Testing Time: 28.73s Expected Passes : 371 -->8- Results for v4.0.d20160824 on FreeBSD 10.2 amd64.
Created attachment 174317 [details] Proposed patch (since 421246 revision) Placed .ifmake makesum check before bsd.port.pre.mk include.
Created attachment 174318 [details] Proposed patch (since 421246 revision) (In reply to comment #0) > - Add changes to .MAKEFLAGS in case of makesum target to fetch > all optional distfiles for distinfo file creation, independent > from selected options Looks like, better to remove this part (for now), because of OPTIONS_DEFINE_* for other architectures.
Thank you for your submission. Many of these changes look like improvements, but some I do not like. Having an enormous patch full of unrelated changes makes it hard to separate them. For now, I reject these two changes: > - Remove COMMANDS variable, check-commands target and simplify llvm-wrapper.sh installation in post-install-script target > - Replace contents of build-plist target with dynamic approach and POST_PLIST usage In order to eventually support multiple packages per port, we need to track which parts go with which component. Since that feature is essential for the continued improvement of the system, I'd rather not throw away all the related information to save a little maintenance burden now.
(In reply to comment #4) > Thank you for your submission. Thanks for your attention. I proposed some of these changes privately in 2015 year, as you may remember. (In reply to comment #4) > Having an enormous patch full of unrelated changes makes it hard to separate > them. Most size of the patch is pkg-plist removal. Other parts were explained in description of comment #0. (In reply to comment #4) > In order to eventually support multiple packages per port, we need to track > which parts go with which component. Since that feature is essential for the > continued improvement of the system, I'd rather not throw away all the related > information to save a little maintenance burden now. Following command gives the same information after stage: % make -V TMPPLIST | xargs cat In my opinion, your decision to go with static pkg-plist (but dynamic nature of it) limits usage of the port by other people, who may update the port for newer version(s). Many of proposed changes applies to other devel/llvm* ports, because of remained errors (e.g., in COMMANDS variable with missing value(s)), including correct lit testing tool usage (e.g. necessity of utils/lit/lit/formats directory to install). In other words, include good changes what you see fit, while I agree with my opinion.
A commit references this bug: Author: brooks Date: Sat Sep 3 00:20:06 UTC 2016 New revision: 421280 URL: https://svnweb.freebsd.org/changeset/ports/421280 Log: Upgrade to 3.9.0 release. Improve support for -fopenmp with a hack inspired by a submission from Johannes Dieterich <dieterich@ogolem.org>. Implement a number of improvments submitted by lightside@gmx.com: - Add license information - Use VAR option helper for _USES_PYTHON variable - Add libxml2 to USE_GNOME for CLANG option - Sort variables for options (e.g. GOLD_*) Add USES=execinfo for LLDB. PR: 203223, 212334 Changes: head/devel/llvm39/Makefile head/devel/llvm39/distinfo head/devel/llvm39/files/clang-patch-fopenmp.diff
(In reply to comment #4) > For now, I reject these two changes: >> - Remove COMMANDS variable, check-commands target and simplify >> llvm-wrapper.sh installation in post-install-script target >> - Replace contents of build-plist target with dynamic approach and >> POST_PLIST usage I will try to change the proposed patch to use COMMANDS variable and pkg-plist file. This requires some time, including for testing build with all options.
Created attachment 174339 [details] Proposed patch (since 421246 revision) Added COMPILER_RT_IMPLIES=CLANG, because the compiler-rt requires clang and clang-headers targets, when build as a project in projects/compiler-rt directory.
Created attachment 174340 [details] Proposed patch (since 421246 revision, second variant) Attached second variant, which uses COMMANDS variable and pkg-plist, with following overall changes: - Bump PORTREVISION - Add license information - Use VAR option helper for _USES_PYTHON variable - Simplify PORTDOCS with * value - Add UTILS (default) option to install utility binaries (e.g. count, FileCheck, not, etc.) - Change description for LIT option - Add dependency on UTILS for LIT option - Use new GH_SUBDIR option helper, instead of post-extract-* targets - Add libxml2 to USE_GNOME for CLANG option, because c-index-test requires this, according to stage-qa - Sort variables for options (e.g. GOLD_*) and change placement of some variables - Remove second OPTIONS_SUB definition - Replace blank space(s) with tab for some cases - Build compiler-rt as a project in projects/compiler-rt directory; add dependency on CLANG for COMPILER_RT option - Move installation of llvm-wrapper.sh to post-install-script target - Remove macho-dump from COMMANDS and clang-modernize from EXTRAS_COMMANDS, which checked with check-commands target - Remove regression-test target; use new TEST_TARGET and TEST_WRKSRC variables for `make test` - Remove post-patch target for lit changes, which allows usage of testing targets - Install lit with using utils/lit/setup.py; add Python setuptools to build dependency for LIT option - Adapt build-plist target - Remove curl and jq requirement with using available commands in files/gen-Makefile.snapshot.sh script - Add files/llvm-lit-wrapper.sh.in, which contains PYTHONPATH environment variable, for correct (llvm-)lit${LLVM_SUFFIX} installation and usage - Adapt pkg-plist and other small changes for variables.
Created attachment 174344 [details] Proposed patch (since 421246 revision, second variant) Cosmetic fix.
Possible to replace "${PYDISTUTILS_INSTALLARGS}/${PORTNAME}${LLVM_SUFFIX}" with "--prefix=${LLVM_PREFIX}". I didn't find how to disable build of *.py files (*.pyc, *.pyo in pkg-plist) for lit installation (if this matters).
(In reply to comment #11) > I didn't find how to disable build of *.py files (*.pyc, *.pyo in pkg-plist) > for lit installation (if this matters). The `python setup.py --help install` says about --no-compile and --skip-build options, which possible to use to disable this, but I didn't try.
Created attachment 174348 [details] Proposed patch (since 421246 revision, third variant) (In reply to comment #12) > The `python setup.py --help install` says about --no-compile and > --skip-build options, which possible to use to disable this, but I didn't try. I tried this. The "--no-compile" option was enough. Attached this variant instead.
Created attachment 175323 [details] Proposed patch (since 421246 revision, third variant) Cosmetic fixes. Clarified description for overall changes: - Bump PORTREVISION - Add license information - Use VAR option helper for _USES_PYTHON variable - Simplify PORTDOCS with * value - Add UTILS (default) option to install utility binaries (e.g. count, FileCheck, not, etc.) - Change description for LIT option - Add dependency on UTILS for LIT option - Use new GH_SUBDIR option helper, instead of post-extract-* targets - Add libxml2 to USE_GNOME for CLANG option, because c-index-test requires this, according to stage-qa - Sort variables for options (e.g. GOLD_*) - Remove second OPTIONS_SUB definition - Replace blank space(s) with tab for some cases - Build compiler-rt as a project in projects/compiler-rt directory - Add dependency on CLANG for COMPILER_RT option - Adapt variables for COMPILER_RT option - Move installation of llvm-wrapper.sh to post-install-script target - Remove macho-dump from COMMANDS and clang-modernize from EXTRAS_COMMANDS, which checked with using check-commands target - Remove regression-test target; use new TEST_TARGET and TEST_WRKSRC variables for `make test` - Remove post-patch target for lit changes, which allows usage of testing targets - Install lit with using utils/lit/setup.py; add Python setuptools to build dependency for LIT option - Adapt build-plist target - Remove curl and jq requirement with using available commands from base installation of FreeBSD operating system in files/gen-Makefile.snapshot.sh script - Add files/llvm-lit-wrapper.sh.in, which contains PYTHONPATH environment variable, for correct (llvm-)lit${LLVM_SUFFIX} installation and usage - Adapt pkg-plist
Created attachment 175490 [details] Proposed patch (since 423371 revision) Updated patch, after ports r423371 changes. - Bump PORTREVISION - Add license information - Simplify PORTDOCS with * value - Add UTILS (default) option to install utility binaries (e.g. count, FileCheck, not, etc.) - Add remaining parts for files/clang-patch-fopenmp.diff, based on ports r421280 changes - Change description for LIT option - Add dependency on UTILS for LIT option - Sort variables for options (e.g. GOLD_*) - Remove second OPTIONS_SUB definition - Replace blank space(s) with tab for some cases - Build compiler-rt as a project in projects/compiler-rt directory - Adapt variables for COMPILER_RT option - Move installation of llvm-wrapper.sh to post-install-script target - Remove macho-dump from COMMANDS and clang-modernize from EXTRAS_COMMANDS, which checked with using check-commands target - Remove regression-test target; use new TEST_TARGET and TEST_WRKSRC variables for `make test` - Remove post-patch target for lit changes, which allows usage of testing targets - Install lit with using utils/lit/setup.py; add Python setuptools to build dependency for LIT option - Adapt build-plist target - Remove curl and jq requirement with using available commands from base installation of FreeBSD operating system in files/gen-Makefile.snapshot.sh script - Add files/llvm-lit-wrapper.sh.in, which contains PYTHONPATH environment variable, for correct (llvm-)lit${LLVM_SUFFIX} installation and usage - Adapt pkg-plist
Created attachment 175491 [details] Proposed patch (since 423371 revision) Cosmetic fix.
Created attachment 175493 [details] Proposed patch (since 423371 revision) Some additions for build-plist target, related to py${PYTHON_VER}.
Created attachment 175494 [details] Proposed patch (since 423371 revision)
Created attachment 176130 [details] Proposed patch (since 424411 revision) Updated patch, after ports r424411 changes.
(In reply to comment #19) > Updated patch, after ports r424411 changes. Cosmetic changes to unchanged parts. Previous patch also may apply.
Created attachment 177072 [details] Proposed patch (since 426205 revision) Updated patch, after ports r426205 changes. Removed applied changes: - Add remaining parts for files/clang-patch-fopenmp.diff, based on ports r421280 changes Added changes: - Add llvm-xray to COMMANDS - Spell CHOSEN_COMPILER_TYPE correctly (ports/199098) - Convert clang-patch-fopenmp.diff and patch-tools_llvm-shlib_CMakeLists.txt patches in files directory to UTC date format Overall remaining proposed changes: - Bump PORTREVISION - Add license information - Simplify PORTDOCS with * value - Add UTILS (default) option to install utility binaries (e.g. count, FileCheck, not, etc.) - Change description for LIT option - Add dependency on UTILS for LIT option - Sort variables for options (e.g. GOLD_*) - Remove second OPTIONS_SUB definition - Replace blank space(s) with tab for some cases - Build compiler-rt as a project in projects/compiler-rt directory - Adapt variables for COMPILER_RT option - Spell CHOSEN_COMPILER_TYPE correctly (ports/199098) - Move installation of llvm-wrapper.sh to post-install-script target - Remove macho-dump from COMMANDS and clang-modernize from EXTRAS_COMMANDS, which checked with using check-commands target - Add llvm-xray to COMMANDS - Remove regression-test target; use new TEST_TARGET and TEST_WRKSRC variables for `make test` - Remove post-patch target for lit changes, which allows usage of testing targets - Install lit with using utils/lit/setup.py; add Python setuptools to build dependency for LIT option - Adapt build-plist target - Remove curl and jq requirement with using available commands from base installation of FreeBSD operating system in files/gen-Makefile.snapshot.sh script - Add files/llvm-lit-wrapper.sh.in, which contains PYTHONPATH environment variable, for correct (llvm-)lit${LLVM_SUFFIX} installation and usage - Convert clang-patch-fopenmp.diff and patch-tools_llvm-shlib_CMakeLists.txt patches in files directory to UTC date format - Adapt pkg-plist
Created attachment 177073 [details] Proposed patch (since 426205 revision)
To Brooks Davis: What issues do you have with remaining proposed changes in this PR? What are the reasons to not include them (even after converting to your pkg-plist approach)? If remaining proposed changes are unwanted, then better just to close this PR, in my opinion. There were unnecessary extra work, related to support of this PR, because of other (external) changes to the port.
Created attachment 177076 [details] The devel/llvm-devel port (v4.0.d20161115) with proposed changes Added complete devel/llvm-devel port with proposed changes in tar.bz2 archive, just in case.
Current issues (since 426205 revision), found by portlint program (from ports-mgmt/portlint port): -8<-- % cd /usr/ports/devel/llvm-devel && portlint -C FATAL: Makefile: [47]: use a tab (not space) after a variable name FATAL: Makefile: [140]: use a tab (not space) after a variable name FATAL: Makefile: [230]: use a tab (not space) after a variable name WARN: Makefile: [255]: use a tab (not space) after a variable name WARN: Makefile: [43]: You may remove pkg-plist if you use PLIST_FILES and/or PLIST_DIRS. WARN: Makefile: [0]: possible direct use of command "awk" found. use ${AWK} instead. WARN: Makefile: possible use of absolute pathname "/omp.h|${EXTRAS_PATT...". WARN: Makefile: Consider defining LICENSE. WARN: /usr/ports/devel/llvm-devel/files/patch-tools_llvm-shlib_CMakeLists.txt: patch was not generated using ``make makepatch''. It is recommended to use ``make makepatch'' when you need to [re-]generate a patch to ensure proper patch format. 3 fatal errors and 6 warnings found. -->8- After applied patch in attachment #177076 [details]. -8<-- % portlint -C WARN: Makefile: [54]: You may remove pkg-plist if you use PLIST_FILES and/or PLIST_DIRS. WARN: Makefile: possible use of absolute pathname "/lib/clang/${LLVM_RE...". WARN: Makefile: possible use of absolute pathname "/omp.h|${EXTRAS_PATT...". 0 fatal errors and 3 warnings found. -->8-
(In reply to comment #25) > After applied patch in attachment #177076 [details]. After applied patch in attachment #177073 [details].
Example of output for LLVM regression tests (TEST_TARGET=check): -8<-- % make test ... Testing Time: 531.50s Expected Passes : 18090 Expected Failures : 119 Unsupported Tests : 465 -->8- Example of output for Clang regression tests: -8<-- % make TEST_TARGET=check-clang test ... ******************** Testing Time: 402.42s ******************** Failing Tests (302): ... Expected Passes : 9698 Expected Failures : 16 Unsupported Tests : 46 Unexpected Failures: 302 ... -->8- Example of output for Clang extra tools' regression tests: -8<-- % make TEST_TARGET=check-clang-tools test ... Testing Time: 19.52s Expected Passes : 446 Expected Failures : 1 -->8- Results for v4.0.d20161115 on FreeBSD 10.2 amd64.
Some proposed changes were committed. This PR closed.