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.

Sign up for GitHub

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

Jump to bottom

codimd: js-sequence-diagrams: use git source rather than the NPM tarball #59194

Merged
merged 1 commit into from
Apr 12, 2019
Merged

codimd: js-sequence-diagrams: use git source rather than the NPM tarball #59194

merged 1 commit into from
Apr 12, 2019

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Apr 8, 2019 •
edited
Loading

Motivation for this change

It seems as NPM just removed the tarballs of the unpublished package,
hence codimd isn't buildable. The sources for the package are
available on github[1] and fix the build.

For further information about the js-sequence-diagrams workarounds,
please refer to 5feec42.

[1] https://github.com/Moeditor/js-sequence-diagrams

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Ma27
Copy link
Member Author

Ma27 commented Apr 8, 2019

@GrahamcOfBorg test codimd

This should also be backported to release-19.03.

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Apr 8, 2019
@Ma27
Copy link
Member Author

Ma27 commented Apr 10, 2019

ok, so now there's https://github.com/codimd/server/pull/41/files which uses a fork of js-sequence-diagrams in the codimd org. I'll update the PR accordingly.

@Ma27 Ma27 changed the title codimd: js-sequence-diagrams: use git source rather than the NPM tarball [WIP] codimd: js-sequence-diagrams: use git source rather than the NPM tarball Apr 10, 2019
@Ma27
Copy link
Member Author

Ma27 commented Apr 10, 2019

This doesn't seem to be possible right now as node2nix generates mismatching hashes when using the git source of codimd/js-sequence-diagrams (svanderburg/node2nix#139)

@Ma27 Ma27 changed the title [WIP] codimd: js-sequence-diagrams: use git source rather than the NPM tarball codimd: js-sequence-diagrams: use git source rather than the NPM tarball Apr 10, 2019
@Ma27
codimd: js-sequence-diagrams: use git source rather than the NPM tarball
c68bc18 
It seems as NPM just removed the tarballs of the unpublished package,
hence `codimd` isn't buildable. The sources for the package are
available on github[1] and fix the build.

For further information about the `js-sequence-diagrams` workarounds,
please refer to 5feec42.

[1] https://github.com/Moeditor/js-sequence-diagrams
@Ma27 Ma27 force-pushed the fix-js-sequence-diagrams-source branch from 9d688b1 to c68bc18 Compare April 12, 2019 09:01
@Ma27
Copy link
Member Author

Ma27 commented Apr 12, 2019

Updated the commit message to fix a small typo. @marsam are you still okay with this? :)

@Ma27
Copy link
Member Author

Ma27 commented Apr 12, 2019

Just had a short chat with @WilliButz. We both agreed that this is the best solution for now and unless I hear otherwise, I'd merge/backport the fix tonight.

I know that this isn't the best solution, but it should be fine for now. As mentioned in original issue, it's planned to switch to yarn2nix in the long term as it will significantly reduce the complexity of the current packaging situation for codimd.

@Ma27
Copy link
Member Author

Ma27 commented Apr 12, 2019

As announced, I'll merge this for now. As soon as @WilliButz or I have sufficient time, the bump to 1.3 with yarn2nix will be done.

@Ma27 Ma27 merged commit 619c15e into NixOS:master Apr 12, 2019
@Ma27 Ma27 deleted the fix-js-sequence-diagrams-source branch April 12, 2019 21:39
@Ma27
Copy link
Member Author

Ma27 commented Apr 12, 2019

Backported as 0b89775.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marsam marsam marsam approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ma27 @marsam @GrahamcOfBorg