Initial support for building on FreeBSD by mkrzewic · Pull Request #12996 · root-project/root

mkrzewic · 2023-06-12T15:28:10Z

This Pull request:

This is an initial attempt at getting this thing building on modern FreeBSD (i.e. without /proc)
some discussion was going on in issue #12787 and @hahnjo suggested a broader review and discussion in a PR.

Changes or fixes:

Checklist:

tested changes locally
updated the docs (if necessary)

This PR fixes #12787

phsft-bot · 2023-06-12T15:28:13Z

Can one of the admins verify this patch?

bellenot · 2023-06-12T15:30:20Z

@phsft-bot build

phsft-bot · 2023-06-12T15:30:26Z

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac12/noimt, mac11/cxx14, windows10/cxx14
How to customize builds

hahnjo · 2023-06-12T15:38:48Z

Thanks @mkrzewic! We kicked off testing to make sure it doesn't break other platforms. I will take a closer look tomorrow 😉

Axel-Naumann

This is fantastic, thank you so much!

Just suggesting some small wording (thisisabloodyhackZFS) / indentation changes. I'm curious to see how the CI will deal with the textual module!

hahnjo · 2023-06-13T07:10:54Z

+  if(CMAKE_CXX_COMPILER_ID STREQUAL Intel)
+    set(ROOT_ARCHITECTURE freebsdx8664icc)
+  else()


Is there really an Intel compiler for FreeBSD? I'd throw out anything that is not needed / tested on FreeBSD, probably only leave freebsdadm64 as ROOT_ARCHITECTURE

I got a request from the port maintainer (@eamjensen) to keep (and add/fix some of the FBSD specific naming) the cpu architectures in the SetupFreeBSD. Maybe refs to the old freebsd archs can be removed (freebsd4,freebsd5,freebsd7 in config/root-config.in and a bunch of options in etc/Makefile.arch) - instead we could glob/wildcard as they all seem to to the same thing for now plus I think it's safe to say older (gcc based) freebsd systems will not be supported (e.g. because they don't support c++14 etc.)
I'll remove the intel thing, it makes no sense indeed.

I cleaned up a bit, all in new commits.

maintained by Erik Jensen <erik@tenku.dk> (https://www.freshports.org/devel/root)

this should work on FreeBSD 9 and up. only tested on 13.

… namespace will do

Co-authored-by: Axel Naumann <Axel.Naumann@cern.ch>

jalopezg-git · 2023-06-15T11:37:10Z

+      auto parent_path = [](std::string path) {
+         return path.substr(0, path.find_last_of("/\\"));
+      };
+      fallback = parent_path(parent_path({path_str}));


I see that this part is common with the WIN32 case. Perhaps we can restructure this a bit to avoid code duplication.

possibly, I even tried, but for some reason this was better if I remember correctly... maybe not a good reason though:)

jalopezg-git · 2023-06-15T11:44:39Z

 #endif
+#if defined R__FBSD || defined __FreeBSD__
+  procstat* ps = procstat_open_sysctl();  //
+  kinfo_proc* kp = kinfo_getproc(getpid());


I saw some other usages of the procfs filesystem typically mounted on /proc on Linux, namely in:

The proof/, rootx/ and bindings/pyroot/cppyy/ directories

The config/thisroot.sh file

Maybe we should take a look into those too.

config/thisroot.sh should work as it is
rootx I'll fix, but is there no common library/place to define GetExePath (and the like) once for all components?
proof and python I'll leave for later as I have no use for those. Someone will contribute a patch if there is need:)

jalopezg-git · 2023-06-15T11:46:08Z

+char*  __progname;
+char** environ;


These are extern, no?

possibly, this is a "legacy" fix from the port maintainer, it is needed and sufficient (it seems). I think maybe for some reason the symbols needed to be visible for lld, possibly some subtle difference with ld.

jalopezg-git · 2023-06-15T11:48:06Z

+// libprocstat pulls in sys/elf.h which seems to clash with llvm/BinaryFormat/ELF.h
+// similar collision happens with ZFS. Defining ZFS disables this include.


Interesting 🧐...

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

mkrzewic · 2023-06-16T18:08:53Z

I have one more question, this gets it building and kinda working, but there are still runtime issues: bunch of tests fail, most notably some io (basic works, higher compression levels fail) and threading (some TThread tests fail, for various reasons, from trivial _REENTRANT macro not defined, to proper crashes). Question is: keep adding to this PR or open a new issue some time in the future?

Axel-Naumann · 2023-06-17T09:33:07Z

New issues, new PRs please - we prefer to work incrementally, grandiose things have the habit to fail :-) I very much appreciate this first step to make it build!

github-actions · 2023-06-17T20:54:11Z

Test Results

        9 files         9 suites 1d 21h 20m 47s ⏱️
  2 469 tests   2 459 ✔️ 0 💤 10 ❌
21 132 runs 21 122 ✔️ 0 💤 10 ❌

For more details on these failures, see this check.

Results for commit c37d609.

hahnjo · 2023-06-19T06:57:50Z

Let's not downplay the achievement here - my understanding is that this PR not only allows to compile ROOT on FreeBSD, but gets us a decently functional executable for first tests and further improvements 🚀 I agree with Axel that other problems and tests should be addressed in future issues / PRs.

I think the final point to consider is how to merge. With the current state of commits addressing review comments, I would propose to squash on merge; unless @mkrzewic wants to rework the commits and fold the followups into the respective commits 😉

Axel-Naumann · 2023-06-19T07:00:28Z

Thanks @hahnjo - @mkrzewic unless you have any objection I will merge this tomorrow in a squashed version.

mkrzewic · 2023-06-20T20:14:29Z

@Axel-Naumann sure, squash away, then I'll rebase my next branch and open a new PR.

Axel-Naumann · 2023-06-22T08:59:03Z

Thanks a LOT for your work, @mkrzewic !

hahnjo · 2023-06-22T12:18:31Z

We forgot to run the old Jenkins CI for the latest push (that is on us), #13073 takes care of an unused-but-set-variable warning...

Co-authored-by: Mikolaj Krzewicki <Mikolaj Krzewicki mkrzewicki@gmail.com>

mkrzewic requested review from Axel-Naumann, bellenot and pcanal as code owners June 12, 2023 15:28

hahnjo requested a review from jalopezg-git June 12, 2023 15:38

Axel-Naumann requested changes Jun 12, 2023

View reviewed changes

Comment thread core/unix/CMakeLists.txt Outdated

Comment thread interpreter/cling/include/cling/libc.modulemap Outdated

Comment thread interpreter/cling/lib/Interpreter/DynamicLibraryManagerSymbol.cpp

Comment thread interpreter/cling/lib/Interpreter/DynamicLibraryManagerSymbol.cpp Outdated

bellenot self-assigned this Jun 13, 2023

hahnjo reviewed Jun 13, 2023

View reviewed changes

mkrzewic force-pushed the FreeBSD branch from 8c52e4e to 1723143 Compare June 13, 2023 13:45

mkrzewic and others added 13 commits June 13, 2023 15:52

Initial set of changes based on the FreeBSD port

05d64da

maintained by Erik Jensen <erik@tenku.dk> (https://www.freshports.org/devel/root)

Add SetUpFreeBSD.cmake and use it

34e51bc

Don't use gnu extensions in sed

46adf1d

Fix linking to crypt lib in FreeBSD

e8e5b0a

Add the ability to self inspect the exe path on FreeBSD

71345e2

this should work on FreeBSD 9 and up. only tested on 13.

Add a poor man's FreeBSD xattr handling, hoping the hardcoded default…

aa14c60

… namespace will do

skip trying to load [vdso] as it has no proper path on FreeBSD

203130c

Reword defs

54c732d

Co-authored-by: Axel Naumann <Axel.Naumann@cern.ch>

Update comment on a workaround for elf.h include in libprostat (FreeBSD)

96d1820

Formatting

af96b2c

[FreeBSD] Fix ncpu reporting in root-config

65fb8d0

Clean up SetupFreeBSD.cmake and define freebsdamd64

9810d44

Remove old freebsd entries

49560a5

mkrzewic force-pushed the FreeBSD branch from d182032 to 49560a5 Compare June 13, 2023 13:52

jalopezg-git reviewed Jun 15, 2023

View reviewed changes

mkrzewic and others added 3 commits June 15, 2023 14:41

Update core/unix/src/TUnixSystem.cxx

c829cc9

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

Update interpreter/cling/lib/Interpreter/DynamicLibraryManagerSymbol.cpp

bbb27ec

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

Update interpreter/cling/lib/Interpreter/DynamicLibraryManagerSymbol.cpp

e8770e8

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

mkrzewic and others added 5 commits June 15, 2023 14:45

Update io/io/src/TFile.cxx

41e6bf3

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

Update cmake/modules/SetUpFreeBSD.cmake

7347184

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

Update core/dictgen/src/rootcling_impl.cxx

d502f3f

Co-authored-by: Javier Lopez-Gomez <javier.lopez.gomez@cern.ch>

Reuse some code between WIN32 and FreeBSD

0278fe2

FreeBSDify rootx

c37d609

Axel-Naumann approved these changes Jun 16, 2023

View reviewed changes

Axel-Naumann added the pr:squash on merge label Jun 19, 2023

Axel-Naumann merged commit 9734f9a into root-project:master Jun 22, 2023

maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jun 28, 2023

Initial support for building on FreeBSD (root-project#12996)

31b11aa

Co-authored-by: Mikolaj Krzewicki <Mikolaj Krzewicki mkrzewicki@gmail.com>

guitargeek mentioned this pull request Sep 9, 2024

Check whether ROOTSYS is defined in rootcling_stage1. #10776

Closed

1 task

		// libprocstat pulls in sys/elf.h which seems to clash with llvm/BinaryFormat/ELF.h
		// similar collision happens with ZFS. Defining ZFS disables this include.

		char* __progname;
		char** environ;

Conversation

mkrzewic commented Jun 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

This Pull request:

Changes or fixes:

Checklist:

Uh oh!

phsft-bot commented Jun 12, 2023

Uh oh!

bellenot commented Jun 12, 2023

Uh oh!

phsft-bot commented Jun 12, 2023

Uh oh!

hahnjo commented Jun 12, 2023

Uh oh!

Axel-Naumann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkrzewic Jun 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkrzewic Jun 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mkrzewic commented Jun 16, 2023

Uh oh!

Axel-Naumann commented Jun 17, 2023

Uh oh!

github-actions Bot commented Jun 17, 2023

Test Results

Uh oh!

hahnjo commented Jun 19, 2023

Uh oh!

Axel-Naumann commented Jun 19, 2023

Uh oh!

mkrzewic commented Jun 20, 2023

Uh oh!

Axel-Naumann commented Jun 22, 2023

Uh oh!

hahnjo commented Jun 22, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

mkrzewic commented Jun 12, 2023 •

edited

Loading

mkrzewic Jun 13, 2023 •

edited

Loading

mkrzewic Jun 15, 2023 •

edited

Loading