Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: documenting a bit more FreeBSD case #30325

Closed
wants to merge 2 commits into from

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Nov 7, 2019

FreeBSD provides more up to date compilers than
the one provided by the system.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. labels Nov 7, 2019
@devnexen devnexen requested a review from rvagg November 7, 2019 10:08
BUILDING.md Outdated
@@ -119,7 +119,7 @@ platforms. This is true regardless of entries in the table below.
| macOS | x64 | >= 10.11 | Tier 1 | |
| SmartOS | x64 | >= 18 | Tier 2 | |
| AIX | ppc64be >=power7 | >= 7.2 TL02 | Tier 2 | |
| FreeBSD | x64 | >= 11 | Experimental | Downgraded as of Node.js 12 |
| FreeBSD | x64 | >= 11 | Experimental | Downgraded as of Node.js 12, it is advised to use LLVM toolchain from package manager instead, more up to date |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. Advised why? LLVM instead of what? more up to date than what?

Copy link
Contributor Author

@devnexen devnexen Nov 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Than the one shipped with the system, FreeBSD 12.0 is still 6.0 but 12.1 jumped to 8.0 ;however the package manager provide LLVM up to 9.0 which comes with the complete toolset. Just a PR to respond to one of @rvagg remark on this doc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you link to the comment? This is quite hard to understand without more context. We don't generally "advise"--- something is either required, or not. Is an llvm install (of what version?) required on some freebsd versions (which?) to get runtime libraries (which?) installed, so that node runs? Or is it necessary to have some version of llvm installed to get some kind of debug tool? (you mention "complete toolset")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right I was unsure of the words limit allowed, fair enough. Will wait @rvagg opinion on this one to update these.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so I think maybe this comes from the discussion on the latest version of V8 that's landing where it had trouble compiling on our older gcc installs which we hadn't got around to dealing with even though we'd lifted the minimums. nodejs/build#1970

./configure has: warn('C++ compiler (CXX=%s, %s) too old, need g++ 6.3.0 or clang++ 8.0.0' %

So the FreeBSD machines got vacuumed up in the same discussion as the old gcc machines. In the end I think we might have determined that the 8.0.0 might have been somewhat arbitrary and we really don't know what minimum is required and that it probably would have been (mentioned in a comment further down nodejs/build#1970 (comment) but we yanked FreeBSD 10 anyway).

SO, if FreeBSD comes with 6 by default then it's going to run afoul of our (arbitrary) minimum, so using an alternate source that gets you a newer version might be a good idea.

The reason we set these minimums is to make space for C++ feature-use creep in V8. As long as it still compiles it should be fine, but we gave ourselves that room in case we happen to introduce a new V8 in 13 (and maybe 12?) that pushes beyond what 6 can handle.

A note like you've included is probably fine but it should go into a footnote like the other notes. Add a <sup>[7](#fn7)</sup> in place of the text you've added and and add a new <em id="fn7">7</em>: block below the others with something like:

LLVM 8.0 or later is recommended for building Node.js. FreeBSD 12.0 includes LLVM 6.0 by default but 12.1 upgrades it to 8.0. The package manager provides later versions.

I'm not sure if "the package manager" is the right language here, just riffing off what you said. You have space down there to expand a bit though.

/cc @nodejs/platform-freebsd and @jbergstroem particularly might have good input here.

Copy link

@emaste emaste Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If GCC 6.3 is sufficient I'd be very surprised if Clang 8.0 is actually the minimum rather than an arbitrarily chosen value. GCC 6.3 is about 3 years old, Clang 6.0 is about a year and a half (and Clang 8.0 about half a year).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is somewhat surprising given that clang has been much quicker to introduce newer C++ features than gcc, but it's also easier to deploy, it's not hard to get a newer clang on most platforms compared to gcc, so the minimum, while arbitrary, isn't exactly a great burden.

If you want to change the minimum in configure.py then you're welcome to start that conversation with a pull request, maybe it'll uncover some of the thoughts behind it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#26719 (comment) is where the minimum Clang version was bumped to 8.0 and on reflection it has probably been conflated with the required XCode version for macOS.

FreeBSD provides more up to date compilers than
the one provided by the system.
BUILDING.md Outdated
@@ -153,6 +153,10 @@ are provided. However, tests in our infrastructure only run on WoW64.
Furthermore, compiling on x86 Windows is currently considered Experimental and
may not be possible.

<em id="fn7">7</em>: FreeBSD 12.0 system's compiler is clang 6.0.1 but
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The English and punctuation is a little off in this section, how about:

The default FreeBSD 12.0 compiler is Clang 6.0.1, but FreeBSD 12.1 upgrades to 8.0.1. Other Clang/LLVM versions are provided via the system's package manager, including Clang 9.0.

Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, but now you have to linebreak it at 80 chars or the linebreak people will be upset we merged like this

addaleax pushed a commit that referenced this pull request Nov 30, 2019
FreeBSD provides more up to date compilers than
the one provided by the system.

PR-URL: #30325
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@addaleax
Copy link
Member

Landed in 73c837b

@addaleax addaleax closed this Nov 30, 2019
addaleax pushed a commit that referenced this pull request Nov 30, 2019
FreeBSD provides more up to date compilers than
the one provided by the system.

PR-URL: #30325
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Dec 3, 2019
targos pushed a commit that referenced this pull request Dec 5, 2019
FreeBSD provides more up to date compilers than
the one provided by the system.

PR-URL: #30325
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 9, 2019
MylesBorins pushed a commit that referenced this pull request Dec 17, 2019
FreeBSD provides more up to date compilers than
the one provided by the system.

PR-URL: #30325
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants