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

support interactive nix-build via rix::nix_build() #22

Merged

Conversation

philipp-baumann
Copy link
Collaborator

@b-rodrigues please note this PR is still in progress; it is a very rough draft.

@philipp-baumann
Copy link
Collaborator Author

addresses #21

@b-rodrigues
Copy link
Owner

I looked at it, seems nice! Could you deal with the CMD check warnings by document the arguments though? Also, I don't know how it could be done, but any ideas on how to unit test the function?

@philipp-baumann
Copy link
Collaborator Author

I looked at it, seems nice! Could you deal with the CMD check warnings by document the arguments though? Also, I don't know how it could be done, but any ideas on how to unit test the function?

on the way :-)

@philipp-baumann
Copy link
Collaborator Author

@b-rodrigues have to briefly think about how to unit-test. Until then, let's keep the PR open. I will ping you later today when ready for a final review.

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 9, 2023

@b-rodrigues have to briefly think about how to unit-test. Until then, let's keep the PR open. I will ping you later today when ready for a final review.

@b-rodrigues At the moment I can only think to do this indirectly via CI/CD pipelining. So not really a unit test in a pure software way, but rather a confirmation it works. What do you think about that? I would then implement like this.

@b-rodrigues
Copy link
Owner

How so? I don't get it

@philipp-baumann
Copy link
Collaborator Author

@b-rodrigues have to briefly think about how to unit-test. Until then, let's keep the PR open. I will ping you later today when ready for a final review.

@b-rodrigues At the moment I can only think to do this indirectly via CI/CD pipelining. So not really a unit test in a pure software way, but rather a confirmation it works. What do you think about that? I would then implement like this.

I am trying to be a bit more clear. We invoke nix-build in this new function. The CLI is written in nix language. How do we test an R function invoking another language apart C or C++?

@philipp-baumann
Copy link
Collaborator Author

@b-rodrigues have to briefly think about how to unit-test. Until then, let's keep the PR open. I will ping you later today when ready for a final review.

@b-rodrigues At the moment I can only think to do this indirectly via CI/CD pipelining. So not really a unit test in a pure software way, but rather a confirmation it works. What do you think about that? I would then implement like this.

I am trying to be a bit more clear. We invoke nix-build in this new function. The CLI is written in nix language. How do we test an R function invoking another language apart C or C++?

we could test via a hash of the build artefact maybe.

keep up with current upstream master
@b-rodrigues
Copy link
Owner

Would be nice if an informative error msg was returned if no default.nix was found

@philipp-baumann
Copy link
Collaborator Author

Would be nice if an informative error msg was returned if no default.nix was found

agree :-). Thinking also to echo a message like
"starting nix-build ..."

@philipp-baumann
Copy link
Collaborator Author

we get the PID even in "non-blocking" mode there we go:

> pid <- nix_build(nix_file = "inst/extdata/default.nix", exec_mode = "non-blocking")
warning: Nix search path entry '/nix/var/nix/profiles/per-user/philipp/channels' does not exist, ignoring
these 8 derivations will be built:
  /nix/store/4a1hv5i38b0pfi5iax5wx262dihj0rha-rix-34f4715.drv
  /nix/store/hgaw5s4712bg2xbd76qkr3zjcgj5gkak-r-openssl-2.0.5.drv
  /nix/store/iq5wnhgbhihsf30g52v1fl051khmyrg4-r-curl-4.3.3.drv
  /nix/store/vwmqcynp95iz261ndfawqkg8pq4q7zrz-r-mime-0.12.drv
  /nix/store/r6v3blld1rza0g2my6ccsnzqws0ak9m7-r-httr-1.4.4.drv
  /nix/store/h2gzp0jvz990k3apbzgjgr5m3hyzp8sa-r-rix.drv
  /nix/store/n9280qkmgw99x2846592g8c8nz2421ad-R-4.2.2-wrapper.drv
  /nix/store/5a6wlark0l0m747pkd7zli4g21d0rz3j-nix-shell.drv
these 104 paths will be fetched (565.70 MiB download, 1260.18 MiB unpacked):
  /nix/store/03bcgkqn4hsf2l2n0ny5xx2c1z0ga6gy-libxkbcommon-1.5.0
  /nix/store/09yg78bcs6vibbk7py1k1l9qf1im9lwj-harfbuzz-7.0.0
  /nix/store/0z70flbcxfvkp3qap9jdbi6mjjadwwkx-gtk+3-3.24.36
  /nix/store/152gsygyjagsirvdg3icmz4px39wddwj-glib-2.74.5-dev
  /nix/store/1gzzdbvsfk7ykqmpwahgpbw987igsh05-openssl-3.0.8-bin
  /nix/store/1xalzkbzfrrw5j09qh7246khbshlgrdv-curl-7.88.0
  /nix/store/1zi1xfjyx1dljqcc8p3y059x256v94ww-kmod-30
  /nix/store/2jq0rdhc7wb8fj0q82whsj9p50sdmvcv-freetype-2.12.1
  /nix/store/36z48v5b6x165cpp43z64mrfyv09vzv4-pango-1.50.12-dev
  /nix/store/3rr3axz5mvrsk32yripf78g9cbg3f3zg-gfortran-12.2.0
  /nix/store/3vnrc3jdf83kx42hb8mdghv8lqddfhxy-libXft-2.3.6
  /nix/store/3yxdzgn2hxxpr73wzwx9rggh864x8cib-tracker-3.4.2
  /nix/store/45h24hh02xmzbzdfxfgmrflklswqzvqb-json-glib-1.6.6
  /nix/store/463cn57ci7xxwr28c1d68zwbr001ka8h-libpng-apng-1.6.39-dev
  /nix/store/4b07b75qbvhy6xbcpqpr83i48hgfd4fi-libmng-2.0.3
  /nix/store/4fysv3vbr08qimwrvn04fbdl8nypkl0j-zstd-1.5.4-bin
  /nix/store/4qf8shwiq3nhrsdsfs5763q14w40k0dy-libaom-3.5.0
  /nix/store/56yv217y8fgg2vx6nzhckc90i7gvj5dz-r-survival-3.4-0
  /nix/store/5j2p1ly0jh7f4h7r9j34dhpw2nrnpffr-stdenv-linux
  /nix/store/5xf33crx3fjn3bq8993d58vmzig98d0y-glib-2.74.5-bin
  /nix/store/63937ka6h2167hljggd8xx3gw602pbqr-texinfo-6.8
  /nix/store/6bdds6632r9fp7mrh27qxjv90d7lzh0y-fontconfig-2.14.0-dev
  /nix/store/798vyxdl129xqwqcbfpn27dzvamw397z-perl-5.36.0
  /nix/store/7b37aqqsxbhvs6ilv0sbdx0xi39j6jhr-gts-0.7.6
  /nix/store/7h9f215fx7kgmin2k35nlkfknk701dyl-gdk-pixbuf-2.42.10
  /nix/store/7zqg9hafk1g8z103ps6fywl89w189nz3-pango-1.50.12
  /nix/store/81zlx45lba8qsfqykbqn0bm2nnvaf8kh-r-Matrix-1.5-3
  /nix/store/90n3cy31f8qlrzs4hmhdwnxawddpb1a9-libtiff-4.5.0
  /nix/store/9362c5116g87zgxafc9lbpvf8g86v83c-gamin-0.1.10
  /nix/store/9c8zpqq2rnj37k8j84g68adljs6kdqmi-gnutls-3.8.0
  /nix/store/9im79gr4kgkb2p3axynvlrpivjw5c2cx-libdevil-1.7.8
  /nix/store/9z8phylynabg15f250i4sfrw3mriw0b9-r-nlme-3.1-160
  /nix/store/bfbp3ypd9nm3fapz634gvvs738blrl0y-gcc-wrapper-12.2.0
  /nix/store/bmj9w4ygx2q19dmawhr19zzqrvkfl97a-r-cluster-2.1.4
  /nix/store/bw8rcdd4w3h6pksh62yhfc24117mpw3s-nghttp2-1.51.0-dev
  /nix/store/c3f4jdwzn8fm9lp72m91ffw524bakp6v-stdenv-linux
  /nix/store/c651qa8kqilglkx8fhxz0ibc4yfndlm8-sqlite-3.40.1
  /nix/store/c6bakr5dqws3k1qdk0qxzlysbg9gka1p-libjxl-0.7.0
  /nix/store/cbnq7h85i8i12krmc5n93y3cspnfgcar-r-class-7.3-20
  /nix/store/ck3mrb8yzak4hilan4cgipxpjsc7fv54-at-spi2-core-2.46.0
  /nix/store/ckvyb7jxzmnvp1pjhm7y871zxwv4m9xq-libwebp-1.3.0
  /nix/store/clln53sqbfn8i0z8x0ji3x3fz6zchqml-zstd-1.5.4
  /nix/store/d7gnh1s7qxmj6w065l9n81xhfw69cdbx-tk-8.6.11-dev
  /nix/store/dd7ggq65m50kl9lxyibaivkwlnnl3pwj-r-spatial-7.3-15
  /nix/store/dy41cc107f3mgprggqj7dxbg4212jhpq-util-linux-minimal-2.38.1-bin
  /nix/store/fyiwac98bi86nd5qzz9bhvs1bvmmwgad-openssl-3.0.8-dev
  /nix/store/g8fxvd4d661hap855aq2l7b3m42gcllw-polkit-122
  /nix/store/g98ykbyrxrl2hilf46f08yp246bmnn02-avahi-0.8
  /nix/store/gd51gknpxqaxyd0gycmszm8ckrvwvs0l-curl-7.88.0-dev
  /nix/store/gfpmwjza4k8y6624s2yw0pqjk2aybls9-git-minimal-2.39.2
  /nix/store/gmr0laikghcg8dfphvsv25lclcrhf3l6-systemd-minimal-252.5
  /nix/store/gwz6xwzk790zslava2az846fphakni27-r-jsonlite-1.8.4
  /nix/store/h6smkld8irnav6yha088dn9h4cihzqz5-openexr-2.5.8
  /nix/store/h6yjxgxlchk7kc16jwydjdv1gpdqrqvp-dbus-glib-0.112
  /nix/store/hmh8yakhrp3r95b7r085m4xnzp8crz3b-libxml2-2.10.3
  /nix/store/i34pk6gv02zsvs3kkajggp5hw505nbyx-r-MASS-7.3-58.1
  /nix/store/i9lc9vqhy928cmwv9msfgkcla8sdz46f-kexec-tools-2.0.25
  /nix/store/idx4lcdd1p91dpib06d5mwyhszs6w8xw-libpng-apng-1.6.39
  /nix/store/iw1vmh509hcbby8dbpsaanbri4zsq7dj-python3-3.10.10
  /nix/store/k4kqrvizzdalhnzfvmladvi6024vp823-libsoup-2.74.3
  /nix/store/k6vlq06f9ad07z368mjd7dwd9llxx20c-gd-2.3.3
  /nix/store/k9hi1f417ph87q830g61z1gl791jixbb-gconf-3.2.6
  /nix/store/lj858aj10sw6kbhhshlqlq258kj2p1kc-r-sys-3.4.1
  /nix/store/lqfkh8w1c9sh3978znw45kx9zaqjp1b8-r-askpass-1.1
  /nix/store/lsg80qf1a3fq0rdp7np33w4naimhbcqi-nghttp2-1.51.0-bin
  /nix/store/mgv2qfkir2w084vwlk5ss6pq2x6gh7n3-libtiff-4.5.0-dev
  /nix/store/mn35g17v0pwnb14k778fap8k31x7waxp-fontconfig-2.14.0-bin
  /nix/store/mv56qyvfin02c96mxrdh60p3c0j05dvn-fontconfig-2.14.0-lib
  /nix/store/myyp93g4lxwlbpgrgfyi5glsr1k7cdaq-libtiff-4.5.0-bin
  /nix/store/n0rh9f06228k6vmvpkzhnj1qarh2a64p-gnome-vfs-2.24.4
  /nix/store/n1zypljp4ijxwwhx03wl03lv8kdqy4c7-libXft-2.3.6-dev
  /nix/store/na6z7fcv842yz7z1d9xgblm67laphlm5-R-4.2.2
  /nix/store/nld5mmzhbw7nbyscdn0i0drvi356av4n-tk-8.6.11
  /nix/store/nn8fbiqk8kv5hj5q33z12m1kg6904cz2-libssh2-1.10.0
  /nix/store/p9q8bgh0qav78niqxjbrnzg8x8cwnmbn-cairo-1.16.0-dev
  /nix/store/pvhlw7lpfik90h22d6xgq6bacxfkin0n-r-mgcv-1.8-41
  /nix/store/q36hdg6hw9vlls7abq0y08gksqr2m2z2-dbus-1.14.4-lib
  /nix/store/q91aw6w9q0ki6zq9881hkkaxk87ssdqy-ORBit2-2.14.19
  /nix/store/qg5ymq815fgvw8cwa6ciri9ijsx2hw13-pango-1.50.12-bin
  /nix/store/qgqsyg7gmqy3fz6shvzhcc7m7pkfw2ig-zlib-1.2.13-dev
  /nix/store/r5rrqp30db9d0g25kjm6l4i3l9m3wf3v-graphviz-7.0.2
  /nix/store/rcwsvm3zmcpwl71b7r5f9ql599hw6f2b-glib-2.74.5
  /nix/store/rhyv9z3js5n14ms7rv0g4s0wkb3xxc6h-r-rpart-4.1.19
  /nix/store/ryxz1bi7axniywwp930zsnfvq3ma85gb-cups-2.4.2-lib
  /nix/store/s7b0fqgczrdsjxbvmbl2iacdcgv8yygf-r-nnet-7.3-18
  /nix/store/s8axys9hpvwxn5718iwrnyvwms4djz96-zstd-1.5.4-dev
  /nix/store/s9rviv9h9j40l0pb717mm0mf8jxc9nis-kmod-30-lib
  /nix/store/sarmffgvwncpdhgzrn631285f54wkpgy-dconf-0.40.0-lib
  /nix/store/sp4j2dyc7rfmx2yk0bw9x4dfhhmhs8qy-libIDL-0.8.14
  /nix/store/v5311yx5lpr60if91rd4vh4n2wxqvxxk-libssh2-1.10.0-dev
  /nix/store/v95glgkqm30pvls1g2s2v9v1ks6dvysj-r-foreign-0.8-84
  /nix/store/vddrjv0wnknv73njnwzj3in0467dk9qa-harfbuzz-7.0.0-dev
  /nix/store/w3wsn4890l6j84pnig7z1lm80slgzwm5-libsoup-3.2.2
  /nix/store/wiis8krqx5hy77j62qxdsc8gnj65grab-gobject-introspection-wrapped-1.74.0
  /nix/store/wj70ar7dgf4ynywqmhxqys5p2rvbhabd-libavif-0.11.1
  /nix/store/wvx5g4kjdb87lvc49s82gcrh59pffv66-openjdk-19.0.2+7
  /nix/store/xzpxjggwfiiw15yxq8rqr8k4si0fqk5i-gobject-introspection-1.74.0
  /nix/store/y0rsgn19b1pnayb5nn4vbliskc9jcw7g-r-lattice-0.20-45
  /nix/store/y49xhcizzj2zi6liajqf6d5cv8p4lpxd-kbd-2.5.1
  /nix/store/y4fkkgbylpqgadgq3n50x7g614sjbrmv-gfortran-wrapper-12.2.0
  /nix/store/ya38n92jwdv6545zj0asli1kqdgcfili-cairo-1.16.0
  /nix/store/ybd7brnb5kavanvz40mg2p5pmgnczi85-freetype-2.12.1-dev
  /nix/store/yl319c7cyd6jb3ssizbdaknwv0543986-curl-7.88.0-bin
  /nix/store/z3a25fpvzdb6xsrjk2y847ji2zdrkb1n-r-KernSmooth-2.23-20
copying path '/nix/store/bfbp3ypd9nm3fapz634gvvs738blrl0y-gcc-wrapper-12.2.0' from 'https://cache.nixos.org'...
copying path '/nix/store/3rr3axz5mvrsk32yripf78g9cbg3f3zg-gfortran-12.2.0' from 'https://cache.nixos.org'...
> pid
[1] 60898

@philipp-baumann
Copy link
Collaborator Author

> library(rix)
> nix_build(nix_file = "inst/extdata/default.nix", exec_mode = "non-blocking")
Launching `nix-build` in non-blocking mode ===> Process ID (PID) is 79524.
warning: Nix search path entry '/nix/var/nix/profiles/per-user/philipp/channels' does not exist, ignoring
/nix/store/b2y1b4c7ggzrdra2r0klkazjvn9hzjqv-nix-shell

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 10, 2023

short todo status list:

  • gracefully stop when file given in nix_file does not exist.
  • document the function
  • more advanced messaging
  • derive sha256 SRI build hash in base64 for the current inst/extdata/default.nix config, to test this against the corresponding TOFU hash via testthat.

@b-rodrigues anything else I forgot about regarding roadmap? ;-)

@b-rodrigues
Copy link
Owner

short todo status list:

  • gracefully stop when file given in nix_file does not exist.
  • document the function
  • more advanced messaging
  • derive sha256 SRI build hash in base64 for the current inst/extdata/default.nix config, to test this against the corresponding TOFU hash via testthat.

@b-rodrigues anything else I forgot about regarding roadmap? ;-)

sounds right to me!

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 10, 2023

one more:

  • signal success via on.exit() hook

=> have to dive a bit deeper, notes to myself: https://jeroen.r-universe.dev/sys/doc/manual.html#exec

@philipp-baumann
Copy link
Collaborator Author

  • also need to automatically kill zombies

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 10, 2023

ATM aiming to capture and signal nix-build special exit codes, too. see https://nixos.org/manual/nix/stable/command-ref/nix-build.html#special-exit-codes-for-build-failure

@philipp-baumann
Copy link
Collaborator Author

> library(rix)
> pid <- nix_build(nix_file = "inst/extdata/default.nix", exec_mode = "blocking")
Launching `nix-build` in blocking mode

==> /nix/store/b2y1b4c7ggzrdra2r0klkazjvn9hzjqv-nix-shell
==> `nix-build` succeeded!
> pid <- nix_build(nix_file = "inst/extdata/default.nix", exec_mode = "non-blocking")
Launching `nix-build` in non-blocking mode
==> Process ID (PID) is 117582.
==> Receiving stdout and stderr streams...
warning: Nix search path entry '/nix/var/nix/profiles/per-user/philipp/channels' does not exist, ignoring
/nix/store/b2y1b4c7ggzrdra2r0klkazjvn9hzjqv-nix-shell

@philipp-baumann
Copy link
Collaborator Author

one more:

  • signal success via on.exit() hook

=> have to dive a bit deeper, notes to myself: https://jeroen.r-universe.dev/sys/doc/manual.html#exec

not really using on.exit() since instead of invisible NULL I want the PID for non-blocking/background exec.

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 10, 2023

@b-rodrigues with the latest commit of this PR it is ready for you to test 🚀 looking forward to your feedback!

What I am still unsure about is whether make non-blocking default since the console is freed, which can be nice for longer builds 😎. Downside of background/non-blocking exec is what {sys} describes as follows: "When using exec_background with std_out = TRUE or std_err = TRUE on Windows, separate threads are used to print output. This works in RStudio and RTerm but not in RGui because the latter has a custom I/O mechanism. Directing output to a file is usually the safest option"

=> latter implies we could have an os detection fallback that redirects stderr and stdout to file and reads it again.

@b-rodrigues
Copy link
Owner

This is really looking great! I have two requests: would it be possible to detect if Nix is installed, and if not, redirect users to https://zero-to-nix.com/start/install ? And regarding the error message, could we provide advice to users to help interpret the messages/correct what’s wrong?

@b-rodrigues
Copy link
Owner

Also, I’m wondering, should we have the path to the default.nix file including it, or simply the folder containg the default.nix file? rix() requries the path to the folder requiring the default.nix file only. Should we change how this work for rix(), or change nix_build()? What do you think?

@philipp-baumann
Copy link
Collaborator Author

This is really looking great! I have two requests: would it be possible to detect if Nix is installed, and if not, redirect users to https://zero-to-nix.com/start/install ? And regarding the error message, could we provide advice to users to help interpret the messages/correct what’s wrong?

sure thing.

@philipp-baumann
Copy link
Collaborator Author

Also, I’m wondering, should we have the path to the default.nix file including it, or simply the folder containg the default.nix file? rix() requries the path to the folder requiring the default.nix file only. Should we change how this work for rix(), or change nix_build()? What do you think?

I think it is good to make things consistent, as you say. Since default.nix is the standard that nix-build uses, I would also say it suffices to give a folder path. We ship a standard default.nix with the package in inst/extdata, however since I guess many projects assume default.nix in the first level of folder structure, it would make sense to use current working directory. What is your opinion?

@philipp-baumann
Copy link
Collaborator Author

philipp-baumann commented Aug 11, 2023

note to myself:

  • Add an extra link to the nix official docs for specific exit codes in nix_build_exit_msg()
  • Point to directory and look for default.nix file
  • Check if nix-build is around and gracefully stop with pointing to https://zero-to-nix.com/start/install

@b-rodrigues
Copy link
Owner

however since I guess many projects assume default.nix in the first level of folder structure, it would make sense to use current working directory. What is your opinion?

Yes, current working directory as default makes most sense, and if no default.nix file is found, instruct users to pay attention with a message such "no default.nix file found. Is the path correctly set?" or something like that. As I’m writing this, I’m thinking that the path = argument in rix() should perhaps be renamed to project_path and that could also be the same argument in nix_build().

@philipp-baumann
Copy link
Collaborator Author

however since I guess many projects assume default.nix in the first level of folder structure, it would make sense to use current working directory. What is your opinion?

Yes, current working directory as default makes most sense, and if no default.nix file is found, instruct users to pay attention with a message such "no default.nix file found. Is the path correctly set?" or something like that. As I’m writing this, I’m thinking that the path = argument in rix() should perhaps be renamed to project_path and that could also be the same argument in nix_build().

all of them are very good ideas I think!

@philipp-baumann
Copy link
Collaborator Author

uuh quite interesting, will use base::system2 for this one. Don't know if this is expected or a bug, will open issue in {sys}.

> out <- system2("command", "-v", "nix-build")
> sys::exec_wait("command", "-v", "nix-build")
Error: Failed to execute 'command' (No such file or directory)
> out
[1] 0

@philipp-baumann
Copy link
Collaborator Author

@b-rodrigues the only thing is the unit test missing.

@philipp-baumann
Copy link
Collaborator Author

ready for review and merge from my side.

@b-rodrigues b-rodrigues merged commit 8380fd5 into b-rodrigues:master Aug 11, 2023
7 checks passed
@philipp-baumann philipp-baumann deleted the 21-interactive-nix-build branch August 11, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants