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

nodejs18: Make python a build dependency #16573

Merged
merged 1 commit into from
Nov 25, 2022
Merged

Conversation

Robotex
Copy link
Contributor

@Robotex Robotex commented Nov 3, 2022

Description

Makes python a build dependency instead of a library dependency because seems like it is not needed at runtime (used only for gyp), just like how the brew counterpart formula already declares

Related for nodejs16: #16482

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

macOS 12.6 21G115 arm64
Xcode 14.0.1 14A400

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 --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot
Copy link

Notifying maintainers:
@ci42 for port nodejs18.

@macportsbot macportsbot added maintainer: open Affects an openmaintainer port type: enhancement labels Nov 3, 2022
@catap
Copy link
Contributor

catap commented Nov 8, 2022

Shall it be reused at another nodejs ports?

@reneeotten
Copy link
Contributor

it seems that Python is also used for testing - at least from the documentation?

If so, it should also be added to "depends_test". Please verify your current change and make sure the tests still work when running in trace-mode (i.e., sudo port -dvt test nodejs18) where undeclared dependencies will not be available.

@Robotex
Copy link
Contributor Author

Robotex commented Nov 13, 2022

it seems that Python is also used for testing - at least from the documentation?

If so, it should also be added to "depends_test". Please verify your current change and make sure the tests still work when running in trace-mode (i.e., sudo port -dvt test nodejs18) where undeclared dependencies will not be available.

Both with (untouched) and without python as dependency running sudo port -dvt test nodejs18 fails with the following error:

/Library/Developer/CommandLineTools/usr/bin/make -s tooltest
......
----------------------------------------------------------------------
Ran 6 tests in 0.001s

OK
/Library/Developer/CommandLineTools/usr/bin/make -s test-doc

added 120 packages, and audited 121 packages in 8s

94 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
Running JS linter...
node:internal/modules/cjs/loader:998
  throw err;
  ^

Error: Cannot find module '/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_nodejs18/nodejs18/work/node-v18.12.1/tools/node_modules/eslint/bin/eslint.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:995:15)
    at Module._load (node:internal/modules/cjs/loader:841:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v18.12.1
make[1]: *** [lint-js-doc] Error 1
make: *** [test] Error 2
Command failed:  cd "/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_nodejs18/nodejs18/work/node-v18.12.1" && /usr/bin/make -j10 test 
Exit code: 2
Error: Failed to test nodejs18: command execution failed
DEBUG: Error code: CHILDSTATUS 84188 2
DEBUG: Backtrace: command execution failed
    while executing
"system {*}$notty {*}$callback {*}$nice $fullcmdstring"
    invoked from within
"command_exec -callback portprogress::target_progress_callback test"
    (procedure "porttest::test_main" line 4)
    invoked from within
"$procedure $targetname"
Error: See /opt/local/var/macports/logs/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_devel_nodejs18/nodejs18/main.log for details.
Error: Follow https://guide.macports.org/#project.tickets if you believe there
is a bug.
Error: Processing of port nodejs18 failed

So the test was already broken before
Reference: nodejs/node#39709

BTW I noticed that even if I don't add python to depends_test-append it will ask to install it anyway,

% sudo port -v test nodejs18
--->  Computing dependencies for nodejs18..
The following dependencies will be installed:  python310
Continue? [Y/n]:

* Make python a build dependency instead of a library dependency

Closes: https://trac.macports.org/ticket/63842
@Robotex
Copy link
Contributor Author

Robotex commented Nov 25, 2022

@reneeotten I bumped the revision here too

@reneeotten reneeotten merged commit a14ec99 into macports:master Nov 25, 2022
@Robotex Robotex deleted the patch-2 branch November 26, 2022 15:23
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: enhancement
Development

Successfully merging this pull request may close these issues.

5 participants