Few build changes to support Freebsd #14121

wfurt · 2017-09-21T21:12:47Z

there may not be 'python' binary - fresh install has only python2.7
__HostDistroRid for FreeBSD
small fixes for consistency and to make build of corelib easier for freebsd

…g on FreeBSD

…p=FreeBSD is similar to freebsdmscorlib in build.cmd

janvorli · 2017-09-21T22:29:31Z

CMakeLists.txt

@@ -35,7 +35,7 @@ OPTION(CLR_CMAKE_ENABLE_CODE_COVERAGE "Enable code coverage" OFF)
 OPTION(CLR_CMAKE_WARNINGS_ARE_ERRORS "Warnings are errors" ON)

 # Ensure that python is present
-find_program(PYTHON python)
+find_program(PYTHON NAMES python2.7 python2 python)


I didn't need to change this on my FreeBSD 11.0, when I run "python --version", it prints 2.7.13. I wonder what is different on your instance that causes this to be required. But the change doesn't hurt and can possibly help on some new platforms.
Btw, I have:

$ which python /usr/local/bin/python $ ls -la /usr/local/bin/python lrwxr-xr-x 1 root wheel 7 Feb 25 2017 /usr/local/bin/python -> python2

I have fresh install of 11.0 and it is not there.
This is essentially re-incarnation of 38c683c#diff-0b83f9dedf40d7356e5ca147a077acb4 submitted last yer.

Just FYI, I have installed the python-2.7_3,2 package using the pkg tool, that one includes the link I've mentioned above.

I was wondering about that. I have Python 2.7.13 (default, Apr 29 2017, 01:15:48) which comes with base OS e.g. I did not add any new package. With this we would look for more specific 2.7 first , falling back to what ever python you have.

janvorli · 2017-09-21T22:33:15Z

build.sh

@@ -553,6 +557,10 @@ case $CPUName in
        __HostArch=arm64
        ;;

+    adm64)


There is a typo in the name (adm64 instead of amd64).

janvorli · 2017-09-21T22:34:52Z

build.sh

@@ -860,6 +868,15 @@ while :; do
              exit 1
            fi
            ;;
+        -osgroup=*)


Can you please explain why this is needed?

There are two parts. corefx used -OSGroup= to pass in cross-target OS.
I wanted to be consistent. I also feel that option=value scales better than having list of singular values.

As far as the functionality it allows to build mscorelib for freebsd on Linux.

The problem I have with this option is that we already have options that can specify the same thing. Combined -skipnative, -skipnuget and -skiptests are exactly that. Having a new option that just aggregates other ones seems confusing. Moreover, the option format is inconsistent with all the others (other options with parameters separate the option and its value with space, like e.g. -cmakeargs -DXXXXX=YYY.
So I'd prefer not adding it and using the three options when you need that functionality.

I agree. That is how I originally did it. But than I look at freebsdmscorlib and mimic what it does. Setting __BuildOS does more than just passing define to make. I know this parameter is somewhat inconsistent with rest. However this is exactly how same thing is done for corefx.
I think it is easier to modify shell script to support managed/msbuild like parameters than the other way. Just fact that some places you have to use "release" but "-release" elsewhere is driving me nuts.

Thank you. The last thing I would change here is really to make the option match the others, it means "-osgroup xxxx" instead of "-osgroup=xxxx".

janvorli · 2017-09-21T23:53:55Z

src/ToolBox/SOS/lldbplugin/CMakeLists.txt


 if(LLDB_H STREQUAL LLDB_H-NOTFOUND)
    if(REQUIRE_LLDBPLUGIN)
        message(FATAL_ERROR "Cannot find LLDB.h. Try installing lldb-3.6-dev (or the appropriate package for your platform)")
    else()
-        message(WARNING "Cannot find LLDB.h Try installing lldb-3.6-dev (or the appropriate package for your platform)")
+        message(WARNING "Cannot find LLDB.h Try installing lldb-3.6-dev (or the appropriate package for your platform and set LLVM_HOME)")


Setting the LLVM_HOME is not needed in general, installing the lldb-dev package is sufficient on most platforms. Actually, I didn't need to do that on FreeBSD, just adding the find_path was sufficient.

agreed. I wanted to improve message when normal lookup does not work.
Perhaps we can work on wording. In my case I have the headers but the build was not finding them. It may be easier IMHO for distro vendors to set variable instead maintain list of all possible locations.

I would modify the wording to say:
Try installing lldb-3.6-dev (or the appropriate package for your platform). You may need to set LLVM_HOME if the build still can't find it.

I wonder - how did you install the LLVM stuff? I am asking since after we complete enabling FreeBSD, we should add instructions on what dependencies to install so that the build just works.

@janvorli simple pkg install llvm38 should do the trick, also krb5 package is required at some point. I was following https://github.com/dotnet/coreclr/blob/master/Documentation/building/freebsd-instructions.md and it's 99% correct to the build point.

wfurt · 2017-09-22T05:40:24Z

This is part of https://github.com/dotnet/corefx/issues/1626

janvorli · 2017-09-22T23:26:41Z

build.sh

@@ -2,7 +2,7 @@

 # resolve python-version to use
 if [ "$PYTHON" == "" ] ; then
-    if ! PYTHON=$(command -v python || command -v python2 || command -v python 2.7)
+    if ! PYTHON=$(command -v python || command -v python2 || command -v python2.7)


Should we look here in the same order as we do in the CMakeLists.txt above? I like the logic of starting with 2.7, then trying 2 and finally just python.

…with most specific version. uddate warning message about lldb.h and remove composite flag

wfurt · 2017-09-27T21:37:38Z

armel failure seems unrelated:
11:13:16 BEGIN EXECUTION
11:13:16 /home/coreclr/bin/tests/Windows_NT.x64.Debug/Tests/coreoverlay/corerun b49809.exe
11:13:16 qemu: Unsupported syscall: 389
11:13:16
./b49809.sh: line 243: 741 Segmentation fault (core dumped) $_DebuggerFullPath "$CORE_ROOT/corerun" $ExePath $CLRTestExecutionArguments

janvorli · 2017-09-27T21:41:02Z

src/ToolBox/SOS/lldbplugin/CMakeLists.txt


 if(LLDB_H STREQUAL LLDB_H-NOTFOUND)
    if(REQUIRE_LLDBPLUGIN)
        message(FATAL_ERROR "Cannot find LLDB.h. Try installing lldb-3.6-dev (or the appropriate package for your platform)")
    else()
-        message(WARNING "Cannot find LLDB.h Try installing lldb-3.6-dev (or the appropriate package for your platform)")
+        message(WARNING "Cannot find LLDB.h Try installing lldb-3.6-dev (or the appropriate package for your platformi). You may need to set LLVM_HOME if the build still can't find it.")


A nit - typo - platformi -> platform. Also it seems that the same message should be displayed for the message above - and we should put the text to a common variable instead of duplicating it.

wfurt and others added 8 commits September 6, 2017 14:27

also look for python27

a2c1cce

add more locations to search for lldb.h so it just work on FreeBSD

1c333d6

add adm64 to ARCH selection. This is mostly cosmetic to avoild warnin…

71b1c5a

…g on FreeBSD

add support for -osgroup=XXX - this is mostly for conistency. -OSGrou…

88a8c75

…p=FreeBSD is similar to freebsdmscorlib in build.cmd

Merge branch 'master' of https://github.com/wfurt/coreclr into freebsd

6d35f3f

add back fi removed by mistake

1a89171

add __HostDistroRid for FreeBSD

9367982

fix typo adm->amd

074ac96

wfurt requested review from stephentoub, jkotas and janvorli September 21, 2017 21:12

dnfclas added the cla-already-signed label Sep 21, 2017

janvorli reviewed Sep 21, 2017

View reviewed changes

wfurt added the os-freebsd FreeBSD OS label Sep 22, 2017

janvorli reviewed Sep 22, 2017

View reviewed changes

adress fedback from review: make python lookup consistent - starting …

8fe0464

…with most specific version. uddate warning message about lldb.h and remove composite flag

janvorli reviewed Sep 27, 2017

View reviewed changes

Tomas Weinfurt and others added 3 commits September 28, 2017 07:48

Merge branch 'master' of https://github.com/wfurt/coreclr into freebsd

dd93dde

feedback from review: type and small change on OSGroup syntax

9f527b8

few more minor fixes

861ec61

janvorli approved these changes Oct 3, 2017

View reviewed changes

Merge branch 'master' of https://github.com/wfurt/coreclr into freebsd

442e5fc

janvorli approved these changes Oct 3, 2017

View reviewed changes

janvorli merged commit 858f5e0 into dotnet:master Oct 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Few build changes to support Freebsd #14121

Few build changes to support Freebsd #14121

wfurt commented Sep 21, 2017

janvorli Sep 21, 2017

wfurt Sep 22, 2017

janvorli Sep 22, 2017

wfurt Sep 22, 2017

janvorli Sep 21, 2017

janvorli Sep 21, 2017

wfurt Sep 22, 2017

janvorli Sep 22, 2017

wfurt Sep 27, 2017

janvorli Sep 27, 2017

janvorli Sep 21, 2017

wfurt Sep 22, 2017

janvorli Sep 22, 2017

sec Sep 25, 2017

wfurt commented Sep 22, 2017

janvorli Sep 22, 2017

wfurt commented Sep 27, 2017

janvorli Sep 27, 2017

Few build changes to support Freebsd #14121

Few build changes to support Freebsd #14121

Conversation

wfurt commented Sep 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wfurt commented Sep 22, 2017

Choose a reason for hiding this comment

wfurt commented Sep 27, 2017

Choose a reason for hiding this comment