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

ansible: use gcc 4.9 on cross compiling machine #1094

Merged
merged 3 commits into from
Apr 3, 2018
Merged

ansible: use gcc 4.9 on cross compiling machine #1094

merged 3 commits into from
Apr 3, 2018

Conversation

seishun
Copy link
Contributor

@seishun seishun commented Jan 27, 2018

Currently the cross-compiling machine uses gcc 4.8 while Node.js requires "gcc and g++ 4.9.4 or newer". See #762.

4.9.3 < 4.9.4, but it successfully compiles code that requires gcc 4.9.

As far as I can tell, the release machines don't need changes as they already use 4.9 on Jessie: https://github.com/nodejs/build/blob/master/setup/raspberry-pi/ansible-playbook.yaml#L159-L160

@joaocgreis
Copy link
Member

The change here looks good and easy to deploy on the cross compiler machine.

However, I see the release machines have not be updated to 4.9 (gcc --version prints gcc (Raspbian 4.8.2-21~rpi3rpi1) 4.8.2), even if the Ansible exists. Won't updating those cause issues for LTS releases?

@joaocgreis
Copy link
Member

Also, if this goes ahead, the native compiler should also be updated. It's Ubuntu 14.04 so it'll be easy enough to copy the relevant bits to the cross compiler script.

@seishun
Copy link
Contributor Author

seishun commented Feb 5, 2018

Won't updating those cause issues for LTS releases?

Are LTS releases built on Jessie or Wheezy machines?

Also, if this goes ahead, the native compiler should also be updated.

What do you mean by native compiler? The one used for building releases on Raspberry Pi?

@joaocgreis
Copy link
Member

Are LTS releases built on Jessie or Wheezy machines?

If I'm not mistaken, all release machines have the same system installed, Raspbian Wheezy.

What do you mean by native compiler? The one used for building releases on Raspberry Pi?

No, I mean in this PR, for the cross compiling machine. When cross compiling node for ARM there's a part that must be compiled natively, to be run later during the cross compilation process (to prepare snapshots IIRC). So, two compilers end up being used: the one that builds for ARM (CC/CXX) and the native gcc that is installed in the machine (CC_host/CXX_host).

@seishun
Copy link
Contributor Author

seishun commented Feb 5, 2018

If I'm not mistaken, all release machines have the same system installed, Raspbian Wheezy.

That's unfortunate. Can a couple of them be upgraded to Jessie to be used for building Node 10 and above?

No, I mean in this PR, for the cross compiling machine.

I see. So you're saying using ubuntu-toolchain-r/test similarly to 1a4efea would be fine?

@joaocgreis
Copy link
Member

Can a couple of them be upgraded to Jessie to be used for building Node 10 and above?

That might be an option to move #762 forward, but it requires a time investment that I can't commit to at the moment (and probably physical access to the devices) so I'll have to defer to @nodejs/build.

Windows already uses different VS versions for different node versions, and it takes some effort to maintain.

So you're saying using ubuntu-toolchain-r/test similarly to 1a4efea would be fine?

Yes. If we end up with different compilers for different node versions, we'll have to have both gcc versions, duplicate the cc-*.sh scripts and add some logic in the Jenkins script to select the right one.

@seishun
Copy link
Contributor Author

seishun commented Feb 6, 2018

That might be an option to move #762 forward, but it requires a time investment that I can't commit to at the moment (and probably physical access to the devices) so I'll have to defer to @nodejs/build.

Would either of these alternatives be easier?

  • Upgrade to gcc 4.9 on Wheezy
  • Repurpose one of the Jessie test machines as a release machine

If we end up with different compilers for different node versions, we'll have to have both gcc versions, duplicate the cc-*.sh scripts and add some logic in the Jenkins script to select the right one.

Why? Is there an issue with building old node versions with gcc 4.9 on test machines?

@seishun
Copy link
Contributor Author

seishun commented Feb 8, 2018

@joaocgreis It now updates the native compiler, PTAL.

@joaocgreis
Copy link
Member

joaocgreis commented Feb 12, 2018

Looks good, but the scripts should be duplicated, for gcc-4.8 and 4.9, and the multilib packages are needed for both. The issue with testing old versions with 4.9 is that problems with 4.8 will only be uncovered when building the release, or later. We should only go ahead here when the releases are sorted out.

@seishun
Copy link
Contributor Author

seishun commented Feb 14, 2018

The issue with testing old versions with 4.9 is that problems with 4.8 will only be uncovered when building the release, or later.

Wouldn't problems with 4.8 be uncovered on other machines that are certainly staying on 4.8? CentOS 5, for instance. I assumed that was the reason for not keeping 4.8 in #797 and #809.

@seishun
Copy link
Contributor Author

seishun commented Feb 17, 2018

Rebased on master.

@joaocgreis
Copy link
Member

Wouldn't problems with 4.8 be uncovered on other machines that are certainly staying on 4.8?

Most of them probably, but I'm not sure if everything. In my opinion, we should only have differences in test a release infra where we absolutely must.

In this case, we already make a huge allowance with cross compilation, and it looks easy enough to me to duplicate the script (having one explicitly using 4.8 and the other 4.9) and install all multilib packages. It would not change the default, so this could land right away. When the release machines are ready, we would only need to add a line in the Jenkins script to run the right script depending on NODE_MAJOR_VERSION.

@seishun
Copy link
Contributor Author

seishun commented Feb 28, 2018

In this case, we already make a huge allowance with cross compilation, and it looks easy enough to me to duplicate the script (having one explicitly using 4.8 and the other 4.9) and install all multilib packages.

My concern was that it would significantly prolong the upgrade process, for instance #1020 has been ongoing for 3 months. But if you're sure that it's just an extra line in Jenkins, then I don't mind. I just need to know how the scripts should be named.

@rvagg
Copy link
Member

rvagg commented Feb 28, 2018

Can we hold off the 4.9 push just for a short time? I'm going to open an issue in the next couple of days proposing a support strategy for Node 10+ that sets new minimums of support. See #1149 as initial work on that front and #1153 as further work to better support handling different system support levels across all our platforms like we do now for Windows. If we can be more selective on what's run for what version of Node then we should be able to maintain older compilers and libc for older Node and use newer ones for new node. In fact, regarding the cross compiler, we're going to have to cut Wheezy off for 10 and perhaps even consider cutting Jessie off too since it runs out of support soon. We don't have any Pi's running Stretch yet but it may be the case they we have to set that up as our minimum for Node 10, in which case this concern about 4.9 should mostly go away (I think).
I'll try and get that up in the next couple of days because we don't have long to lock down a strategy and start implementing it.

@seishun
Copy link
Contributor Author

seishun commented Mar 14, 2018

@rvagg Any news on that front? Both issues you linked seem stalled.

@seishun
Copy link
Contributor Author

seishun commented Mar 24, 2018

@nodejs/build could someone look into this?

@joaocgreis
Copy link
Member

@seishun pushed a commit to your branch with changes to have two scripts. Does it look good to you?

@seishun
Copy link
Contributor Author

seishun commented Mar 29, 2018

Looks good, although cc-4.8-armv6 etc seems a bit better to me.

@rvagg
Copy link
Member

rvagg commented Mar 30, 2018

So I gather, from looking on the server, that this PR has been deployed there already? Now that we have flexibility in the distro of the test machines, I've put this into the Jenkins node-cross-compile job:

gcc_version=4.8
node_major=`cat src/node_version.h | grep "#define NODE_MAJOR_VERSION" | awk '{ print $3}'`
if test $node_major -ge 10; then
  gcc_version=4.9
fi
echo "Compiling Node $node_major with GCC $gcc_version"

case $label in
  cc-armv6)
    DEST_CPU=arm
    source /opt/raspberrypi/cc-armv6-${gcc_version}.sh
    ;;
  cc-armv7)
    DEST_CPU=arm
    source /opt/raspberrypi/cc-armv7-${gcc_version}.sh
    ;;
  *) echo Warning: Not using a cross compiler.
esac
$CC --version

and this into node-test-binary-arm:

node_major=`cat src/node_version.h | grep "#define NODE_MAJOR_VERSION" | awk '{ print $3}'`
if test $node_major -ge 10; then
  case $label in
    pi1-docker) raspbian=jessie;;
    pi2-docker) raspbian=jessie;;
    pi3-docker) raspbian=stretch;;
    *) echo Error: Unsupported label $label; exit 1
  esac
else # less than Node 10
  case $label in
    pi1-docker) raspbian=wheezy;;
    pi2-docker) raspbian=wheezy;;
    pi3-docker) raspbian=jessie;;
    *) echo Error: Unsupported label $label; exit 1
  esac
fi

echo "Running tests in Raspbian $raspbian"

So we're going to get 4.9 compiled binaries on both and we'll test on jessie, jessie, stretch for 10+. You can see the first successful job here https://ci.nodejs.org/job/node-test-commit-arm-fanned/29/

This should match what we do for release builds too. 4.8 & Wheezy <10, 4.9 & Jessie>=10.

When we need to move beyond 4.9 we might be in for a bit of pain since Raspberry Pi doesn't ship an x64 toolchain for it. We're going to have to use some generic one, perhaps one that ships with Ubuntu or Debian or maybe from Linaro. The longer we can put that off the better but I suppose V8 will continue to drag us into the future kicking and screaming.

@joaocgreis joaocgreis merged commit 6bd9147 into nodejs:master Apr 3, 2018
@seishun seishun deleted the cross-compiler-gcc49 branch April 4, 2018 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants