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
texlab: init at 1.6.0 #73087
texlab: init at 1.6.0 #73087
Conversation
All tests pass and I manually verified the binary using Emacs as a client. @doronbehar I added you as a maintainer, is that ok with you? |
Looks good! I tried to figure this out last night with no luck. I have 1 comment: What I was trying to do, based on other packages I saw that use node2nix, was writing a script such as this: #!/usr/bin/env nix-shell
#! nix-shell -i bash -p nodePackages.node2nix jq
set -eu -o pipefail
if [ "$#" -ne 1 ] || [[ "$1" == -* ]]; then
echo "Usage: $0 <git release tag>"
exit 1
fi
TEXLAB_WEB_SRC="https://raw.githubusercontent.com/latex-lsp/texlab/$1"
# We use curl and `jq` inorder to make the `devDependencies` normal `dependencies`
curl --silent "$TEXLAB_WEB_SRC/src/citeproc/js/package.json" | \
jq '. + {"dependencies": .devDependencies} | del(.devDependencies)' > package.json
wget "$TEXLAB_WEB_SRC/src/citeproc/js/package-lock.json" -O package-lock.json
node2nix \
--nodejs-10
-i package.json -l package-lock.json \
--composition citeproc-packages.nix \
-e ../../../development/node-packages/node-env.nix However, from the diff of this PR it seems that you added a Right? Maybe we can at least take the idea of this script to automate the Oh and also, the PR doesn't use
Yes! |
I followed the same style used in luma and iosveka for grabbing nodejs build dependencies. The
I didn't include a lock file for a few reasons:
|
I see 👍.
That's a good reason for not including those linting dependencies but according to diff --git c/pkgs/development/tools/misc/texlab/citeproc/update-package.json.sh i/pkgs/development/tools/misc/texlab/citeproc/update-package.json.sh
new file mode 100755
index 00000000000..54fbfaccaff
--- /dev/null
+++ i/pkgs/development/tools/misc/texlab/citeproc/update-package.json.sh
@@ -0,0 +1,14 @@
+#!/usr/bin/env nix-shell
+#! nix-shell -i bash -p jq
+
+set -eu -o pipefail
+
+if [ "$#" -ne 1 ] || [[ "$1" == -* ]]; then
+ echo "Usage: $0 <git release tag>"
+ exit 1
+fi
+
+TEXLAB_WEB_SRC="https://raw.githubusercontent.com/latex-lsp/texlab/$1"
+
+curl --silent "$TEXLAB_WEB_SRC/src/citeproc/js/package.json" | \
+ jq '. + {"dependencies": .devDependencies} | del(.devDependencies)' > package.json |
@doronbehar That's a good point 👍. I will try using your script. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great now!
Need to resolve the merge conflict. Preferably by not applying irrelevant changes to node-packages-vXX.nix. |
@veprbl The |
@MetaDark Yes, that's my advice. This should help to limit the PR bitrot until it's reviewed/merged. |
@veprbl Do you conceive a way to make sure the conflicts will always be resolved without making manual changes to the auto generated "Do not edit!" |
I ran into problems trying to manually remove unneeded changes, so I just decided to rebuild the whole expression again. I also started working on a tool to try to support partial builds. It's not complete yet, but it could be helpful in the future. |
I hope there won't be a need for that, perhaps @marsam would be willing to review and merge this quickly? As this is a bit urgent, it will be harder to merge this once there will be merge conflicts.. |
Thanks for working in this, and sorry for the delay
I would also like to work on this in the future, right now |
@marsam That definitely sounds simpler than what I was originally planning. I was thinking of parsing |
That was my original approach towards packaging this but @MetaDark has superseded me. I don't have a strong opinion as for the matter but you do point out an inconsistency.. |
@veprbl I was following the documentation in https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/node.section.md |
Motivation for this change
Fixes #67934
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @doronbehar