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

nodejs14/15: don't require xcode. Allow ccache. #9164

Closed
wants to merge 2 commits into from

Conversation

xeron
Copy link
Contributor

@xeron xeron commented Nov 18, 2020

Description

https://trac.macports.org/ticket/60973

I believe these options are outdated and just a copy-paste from the old versions of nodejs port.

I have no issies building nodejs without XCode and with ccache enabled.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 11.0.1 20B29
Xcode N/A
CLT 12.2.0.0.1.1604076827

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

sudo port -vst install fails with Too many open files.
sudo port -vs install works.

@macportsbot
Copy link

Notifying maintainers:
@ci42 for port nodejs14, nodejs15.

@macportsbot macportsbot added maintainer: open Affects an openmaintainer port type: bugfix labels Nov 18, 2020
@xeron
Copy link
Contributor Author

xeron commented Nov 18, 2020

BTW I can't build with -t due to Too many open files error.

@ryandesign
Copy link
Contributor

I have no issies building nodejs without XCode

You've only changed nodejs14 and nodejs15 here. Does that mean that you verified that nodejs13 and earlier really do still require Xcode?

and with ccache enabled.

Your change doesn't enable ccache, it disables it.

@macportsbot
Copy link

Travis Build #15263 Passed.

Lint results
--->  Verifying Portfile for nodejs14
--->  0 errors and 0 warnings found.
--->  Verifying Portfile for nodejs15
--->  0 errors and 0 warnings found.

The build timed out.

@xeron
Copy link
Contributor Author

xeron commented Nov 19, 2020

You've only changed nodejs14 and nodejs15 here. Does that mean that you verified that nodejs13 and earlier really do still require Xcode?

I only tested node 14 and 15. I'm not planning to test this change with older versions.

Your change doesn't enable ccache, it disables it.

I'm removing configure.ccache no. Whatever is default, it works just fine, meaning this option is not needed.

@ryandesign
Copy link
Contributor

I only tested node 14 and 15. I'm not planning to test this change with older versions.

It's undesirable to have unnecessary differences between port versions so it would be good if someone could test this.

I'm removing configure.ccache no.

Oh, you're right.

Whatever is default, it works just fine, meaning this option is not needed.

Do you have the ccache port installed and have configureccache yes in macports.conf? As far as I recall, you would only see a problem in that scenario.

@xeron
Copy link
Contributor Author

xeron commented Nov 19, 2020

It's undesirable to have unnecessary differences between port versions so it would be good if someone could test this.

I can imagine that some older versions actually needed xcode.

Do you have the ccache port installed and have configureccache yes in macports.conf? As far as I recall, you would only see a problem in that scenario.

Oh, I thought it was enabled by default.

Just enabled it and recompiled with ccache couple of times and it works. I only tested 15 tho.

@xeron
Copy link
Contributor Author

xeron commented Nov 26, 2020

Anyone?

@xeron xeron changed the title nodejs14/15: don't require xcode. Enable ccache. nodejs14/15: don't require xcode. Allow ccache. Nov 26, 2020
@macportsbot
Copy link

Travis Build #15412 Passed.

Lint results
--->  Verifying Portfile for nodejs14
--->  0 errors and 0 warnings found.
--->  Verifying Portfile for nodejs15
--->  0 errors and 0 warnings found.

The build timed out.

@xeron
Copy link
Contributor Author

xeron commented Dec 3, 2020

@ci42 @ryandesign could you merge? If ccache change is questionable here I can revert it…

@macportsbot
Copy link

Travis Build #15538 Passed.

Lint results
--->  Verifying Portfile for nodejs14
--->  0 errors and 0 warnings found.
--->  Verifying Portfile for nodejs15
--->  0 errors and 0 warnings found.

The build timed out.

@macportsbot
Copy link

Travis Build #15707 Passed.

Lint results
--->  Verifying Portfile for nodejs14
--->  0 errors and 0 warnings found.
--->  Verifying Portfile for nodejs15
--->  0 errors and 0 warnings found.

The build timed out.

@xeron
Copy link
Contributor Author

xeron commented Dec 24, 2020

Anyone?

@xeron
Copy link
Contributor Author

xeron commented Jan 4, 2021

@ci42 could you please take a look?

@kencu
Copy link
Contributor

kencu commented Jan 18, 2021

There were problems with certain versions of node and certain mixes of the CLTs https://github.com/nodejs/node-gyp/blob/master/macOS_Catalina.md#solutions.

The problem shows up intermittently, and might happen to one person, but not the next, in an unpredictable way. There are many Trac tickets about this. The fact that nodeXY works on your particular system without any trouble is no reassurance that it will not happen again to the next person.

The simplest solution was to require Xcode, so that is presumably what @ci42 did.

There are many other ports that don't work right without a full Xcode installation. MacPorts had tried 18months ago to see how well it could operate with a full Xcode... the answer is "sorta".

So:

  1. don't hold your breath that this PR will ever be accepted and committed. It probably won't be.
  2. that is absolutely nothing personal on you. Thanks for contributing, and please do much more!
  3. who knows about ccache? I don't know. It's not a big deal. Somewhere, something led @ci42 to disable it. He's not stupid, so there was a reason at that time. As 99% of people download a prebuilt nodeXY from the buildbot, this is no big deal.

If @ci42 doesn't commit this in a week (and I doubt he will, but who knows?) I think we'll go ahead and close it, if you don't mind, as the PR queue is very long and we have to focus on stuff we are going to commit here.

Once again, thanks again, please PR more, we love submissions, upgrades to ports, especially with documented test runs that pass, etc, etc!!!

Best,

Ken

@kencu
Copy link
Contributor

kencu commented Jan 22, 2021

I'm going to close this now. Please reopen if you can get buy-in from @ci42

@kencu kencu closed this Jan 22, 2021
@xdxu
Copy link

xdxu commented Feb 14, 2021

The upgrading to 14.15.5 was broken due to this issue with only CLT installed. If this PR won't be accepted, maybe it is ideal to create a binary package instead?

@kencu
Copy link
Contributor

kencu commented Feb 14, 2021

The maintainer of this port believes that nodejs* requires xcode to be installed for this port to reliably be installed. Sometimes, apparently, people can install nodejs* without xcode (as the PR purports). Other times, based on all the tickets, Xcode is required.

Why this happens has not (apparently) been properly sorted out. Obviously, there is a reason for it. What the reason is I have no idea. The maintainer has not sorted it out either, so he forces Xcode.

Until that gets explained, and the maintainer buys in, the port continues to require Xcode (as do many other ports, by the way).

I don't think MacPorts is going to be installing binary packages of much of anything any time soon, but that is not up to me in the end.

@kencu
Copy link
Contributor

kencu commented Feb 15, 2021

Please see some or all of these for inspiration on why things might be the way they are now:

https://github.com/nodejs/node-gyp/blob/master/macOS_Catalina.md

nodejs/node-gyp#1779

nodejs/node-gyp#1927

https://trac.macports.org/ticket/59382

https://trac.macports.org/ticket/59974

https://trac.macports.org/ticket/59419

For you, it looks like you're quite facile. It takes literally 10 seconds to do this:

bbedit `port file nodejsXY`

delete use_xcode line 

save and build away

if that works for you, all the better. if it always works for everybody -- take it up with @ci42 and see if you can convince him. It's not like he needs a PR for this. It's a flicker of a moment to do it.

@alexkuc
Copy link

alexkuc commented Mar 4, 2021

In case someone stumbles upon this issue, I decided to add additional instructions in addition to @kencu instructions. Before starting, you might make yourself familiar with the official guide on installing older versions.

After that, the general gist is…

  • clone this repository
  • navigate to devel/nodejs<version>
  • change use_xcode from yes to no
  • sudo port install

As mentioned by others, your milage may vary!… For some it works, for others it didn't!…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainer: open Affects an openmaintainer port type: bugfix
Development

Successfully merging this pull request may close these issues.

7 participants