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

elmPackages: Refactor lib and expression for related packages #85324

Merged
merged 1 commit into from Jun 26, 2020

Conversation

turboMaCk
Copy link
Member

@turboMaCk turboMaCk commented Apr 15, 2020

Refactoring elmPackages.lib and related packages:

  • elmPackages.coverage - now reuses new lib function to patch npm instalation of elm
  • elmPacages.create-elm-app - same as above + there is a fix for bug where elm-create-app tries to download elm binary when certain command is run (which fails due to read only FS)

I recomend looking at new file for review (diff is a bit hard to read)

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

'';
});

patchNpmElm = pkg:
Copy link
Member Author

Choose a reason for hiding this comment

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

this is pretty stupid name. Ideas are welcome.

Copy link
Member

Choose a reason for hiding this comment

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

patchNodeModules

Copy link
Member Author

Choose a reason for hiding this comment

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

this is specific to elm package as a npm dependecy. Essentially just this https://www.npmjs.com/search?q=elm. I think calling it nodeModules might be too vague.

what about patchNodeElm?

@turboMaCk
Copy link
Member Author

@domenkozar please have a look. I think I've managed to remove the biggest hacks from recent additions to elm tooling.

@turboMaCk
Copy link
Member Author

@GrahamcOfBorg build elmPackages.elm-coverage

@turboMaCk
Copy link
Member Author

@GrahamcOfBorg build elmPackages.create-elm-app

@turboMaCk turboMaCk changed the title elmPackages: Refactor lib and expression for related packages WIP: elmPackages: Refactor lib and expression for related packages Apr 15, 2020
@turboMaCk
Copy link
Member Author

marking as WIP due to FAILURE of OfBorg. I probably disabled sandboxing on my machine and forgot about it, sorry.

@turboMaCk
Copy link
Member Author

@GrahamcOfBorg build elmPackages.elm-coverage

@domenkozar
Copy link
Member

@GrahamcOfBorg build elmPackages

@domenkozar
Copy link
Member

Seems to fail on darwin:

post-installation fixup
rewriting symlink /nix/store/xip6y50237a9izbj45fr8pj6wjnd0wvb-node_elm-xref-4.1.0/bin to be relative to /nix/store/xip6y50237a9izbj45fr8pj6wjnd0wvb-node_elm-xref-4.1.0
patching script interpreter paths in /nix/store/xip6y50237a9izbj45fr8pj6wjnd0wvb-node_elm-xref-4.1.0
error: build of '/nix/store/j28s5jgyqpilk8c36f7ksq5k93c34rny-node_create-elm-app-4.2.4.drv' failed
``

@turboMaCk
Copy link
Member Author

turboMaCk commented Apr 20, 2020

For the record failure on darwin seems to be related to gyp.

OfBorg log:

> node-gyp rebuild
Traceback (most recent call last):
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py", line 50, in <module>
    sys.exit(gyp.script_main())
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 554, in script_main
    return main(sys.argv[1:])
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 547, in main
    return gyp_main(args)
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/__init__.py", line 532, in gyp_main
    generator.GenerateOutput(flat_list, targets, data, params)
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 2215, in GenerateOutput
    part_of_all=qualified_target in needed_targets)
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 845, in Write
    mac_bundle_deps, extra_outputs, part_of_all)
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/generator/make.py", line 1484, in WriteTarget
    lambda p: Sourceify(self.Absolutify(p)))
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 821, in GetLdflags
    archs = self.GetActiveArchs(self.configname)
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 427, in GetActiveArchs
    xcode_archs_default = GetXcodeArchsDefault()
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 122, in GetXcodeArchsDefault
    xcode_version, _ = XcodeVersion()
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 1274, in XcodeVersion
    version_list = GetStdoutQuiet(['xcodebuild', '-version']).splitlines()
  File "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/gyp/pylib/gyp/xcode_emulation.py", line 1323, in GetStdoutQuiet
    job = subprocess.Popen(cmdlist, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
  File "/nix/store/17i7namf6rlb4vqbrsl156y53mcq0g69-python-2.7.17/lib/python2.7/subprocess.py", line 394, in __init__
    errread, errwrite)
  File "/nix/store/17i7namf6rlb4vqbrsl156y53mcq0g69-python-2.7.17/lib/python2.7/subprocess.py", line 1047, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory
gyp ERR! configure error
gyp ERR! stack Error: `gyp` failed with exit code: 1
gyp ERR! stack     at ChildProcess.onCpExit (/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/lib/configure.js:351:16)
gyp ERR! stack     at ChildProcess.emit (events.js:198:13)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:248:12)
gyp ERR! System Darwin 16.7.0
gyp ERR! command "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/bin/node" "/nix/store/pqvaqbkn0y1idnaycwdqv2924b1k05ap-nodejs-10.20.1/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /nix/store/4qjghk1nj093lh6lkgml6la4lv96ahba-node_create-elm-app-4.2.4/lib/node_modules/create-elm-app/node_modules/watchpack/node_modules/fsevents
gyp ERR! node -v v10.20.1
gyp ERR! node-gyp -v v5.1.0
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! fsevents@1.2.12 install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the fsevents@1.2.12 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /private/tmp/nix-build-node_create-elm-app-4.2.4.drv-0/.npm/_logs/2020-04-16T09_10_28_402Z-debug.log

Which might be related to #84797

@turboMaCk
Copy link
Member Author

I think it might be time to update to node 12 anyway (newest LTS). Elm compiler itself uses nodejs package which seems to be still 10 version. This upgrade concerns only packages using node2nix. I'm keeping it in a separate commit. See commit message

@turboMaCk
Copy link
Member Author

I'm too lazy to fiddle with all the OS updates and then installation of nix on Catalina but please try this again and if it fails I'll do it over the weekend on my old macbook.

@domenkozar
Copy link
Member

@GrahamcOfBorg build elmPackages

@domenkozar
Copy link
Member

It seems like it wants to run GetStdoutQuiet(['xcodebuild', '-version']).splitlines()

@turboMaCk
Copy link
Member Author

turboMaCk commented Jun 23, 2020

@domenkozar sorry it took so long for me to revisit it. I got a new job and wanted to get on speed with work related stuff.

I just rebased this PR and tested it on both NixOS and MacOS and it seems to build now on both machines.

@domenkozar
Copy link
Member

@GrahamcOfBorg build elmPackages

@turboMaCk turboMaCk changed the title WIP: elmPackages: Refactor lib and expression for related packages elmPackages: Refactor lib and expression for related packages Jun 24, 2020
@turboMaCk
Copy link
Member Author

🙌 seems MacOS is passing now. It must have been something unrelated to this.

@domenkozar domenkozar merged commit 38227a6 into NixOS:master Jun 26, 2020
@turboMaCk turboMaCk deleted the refactor-elm-node-packages branch February 2, 2021 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants