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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

ghost: init at 0.11.9 #24936

Closed
wants to merge 1 commit into from
Closed

ghost: init at 0.11.9 #24936

wants to merge 1 commit into from

Conversation

b123400
Copy link
Contributor

@b123400 b123400 commented Apr 15, 2017

Motivation for this change

Add Ghost, the blogging platform to nixpkgs.

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wit" (New package, no dependent)
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

  • I am new to here, not sure if I have structured the package in a good way馃檹
  • Description and long description are copied from Ghost's package.json and website, maybe there are some better descriptions that I didn't realise.

@b123400 b123400 force-pushed the add-ghost branch 5 times, most recently from 88f12dd to bec4bca Compare April 16, 2017 02:01
@joachifm
Copy link
Contributor

The main problem here is the huge amount of stuff that'd need to be checked into the repository. Now, that's a failing of the tooling as far as I can tell, but even if it can't be helped it's still way too much, IMO.

@Mic92
Copy link
Member

Mic92 commented Apr 16, 2017

It would create less, if it was added to ./pkgs/development/node-packages/node-packages.json instead.

@b123400
Copy link
Contributor Author

b123400 commented Apr 16, 2017

OK, let me try to add it via node-packages.json. I thought it is ok because I saw pump.io is doing something similar.

By the way, I think adding the module alone is not enough to make it usable. The ghost package only comes with the core part, it does not include any theme and the admin panel. To make it work, I have to run grunt init at the package level, what is the best practice of doing that? A new nix derivation that points to the generated node module, with a buildPhase?

@b123400 b123400 force-pushed the add-ghost branch 3 times, most recently from 3f3f649 to cf03c31 Compare April 18, 2017 14:50
@b123400
Copy link
Contributor Author

b123400 commented Apr 18, 2017

Updated to use node-packages.json

@Mic92
Copy link
Member

Mic92 commented Apr 19, 2017

This project seems to use bower to download assets. We have this tool for that: https://github.com/rvl/bower2nix

@b123400
Copy link
Contributor Author

b123400 commented Jun 9, 2017

Hello, sorry for the late follow up, have been busy for a while... After some trial and error, I think using node-packages.json might not be very suitable and decided to revert to the original approach. The reasons are:

  • The existing build system of ghost is impure, for example it depends on codes from other repositories has to be pulled as a submodule without tagging. To workaround this, patch of the build script is needed, which makes it more difficult to maintain.
  • node2nix (which node-packages.json is depending on) is not enough to setup dependencies, the npm package "knex" looks for optional dependencies such as mysql and pq in runtime, and users are expected to install them manually. That does not work in the nix workflow because the dependency graph is generated.
  • A pre-build zip file already exists, why not just use it, for its simplicity?
  • I agree that this PR is big and hard to read every line, but using node-packages.json or bower2nix would also create a big diff, they won't make things better. If it helps, I can write a readme on how is the file generated.

@b123400 b123400 changed the title ghost: init at 0.11.7 ghost: init at 0.11.9 Jun 9, 2017
@joachifm
Copy link
Contributor

I still think 11K lines for a single package is way too much.

The repo is a commons, so there has to be a reasonable payoff for each line added. This proposal falls short of that by far. Consider that 20 package expressions of this size would add more lines than is required to describe the entire open source Haskell ecosystem (thousands of packages).

@b123400
Copy link
Contributor Author

b123400 commented Jun 13, 2017

Right, this is big, but is there any way to make it smaller? If I use node-packages.json, I would need extra packages (devDependencies) to build it, which probably makes the diff even bigger.

@nbp
Copy link
Member

nbp commented Aug 4, 2017

If you cannot add it into Nixpkgs yet, maybe you can provide an overlay for it, such that others can follow this overlays updates, and potentially contribute back.

@disassembler
Copy link
Member

I agree with @nbp. Please provide this in an overlay. I'm closing this out because of the concerns raised earlier.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nixos-installing-services-from-npm-possibly-w-overlays/5479/1

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

8 participants