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

quarto: init at 1.1.189 #186697

Merged
merged 3 commits into from
Sep 25, 2022
Merged

Conversation

MrTarantoga
Copy link
Contributor

Description of changes

Quarto® is an open-source scientific and technical publishing system built on Pandoc

The idea is, to add quarto support to RStudio.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

Please also rearrange your commits to have the one adding yourself to the maintainer list first, then squash the rest into a single commit. You can use git rebase -i master to do this easily.

pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
@smancill
Copy link
Contributor

Use the proper format for the maintainers commit message: https://nixos.org/manual/nixpkgs/stable/#var-meta-maintainers

pkgs/development/libraries/quarto/default.nix Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
@smancill
Copy link
Contributor

Do we really need to download precompiled binaries instead of building from source? What's the problem?

@MrTarantoga
Copy link
Contributor Author

Use the proper format for the maintainers commit message: https://nixos.org/manual/nixpkgs/stable/#var-meta-maintainers

Solved in 7363e4d

@MrTarantoga
Copy link
Contributor Author

Do we really need to download precompiled binaries instead of building from source? What's the problem?

Yes, the install phase of the source package does nothing else then downloading precompiled versions and install them. So, there I see no difference between the source and the distribution package.

@das-g das-g mentioned this pull request Aug 22, 2022
@ulysses4ever
Copy link
Contributor

@MrTarantoga you did an amazing job here already, thank you! Do you think you'll get a chance to finish it some time soon?

@ulysses4ever
Copy link
Contributor

I'd also love to have quarto-cli as a separate package, but I can survive with RStudio stuff I guess.

@MrTarantoga
Copy link
Contributor Author

@MrTarantoga you did an amazing job here already, thank you! Do you think you'll get a chance to finish it some time soon?

I am not sure if it will work with all features. It does not find the jupyter or R environment.

I have some struggle with patching the files to work as expected. The last 2 weeks I have no success with adaption.

@leonvonrabenmond
Copy link

I am not sure if it will work with all features. It does not find the jupyter or R environment.

I have some struggle with patching the files to work as expected. The last 2 weeks I have no success with adaption.

I have been playing around with your current patch and I encountered the same issue. Funnily enough, I found that if you add jupyter separately in a nix-shell it works perfectly fine, but when in the enviornment, it somehow can't seem to find it..

@leonvonrabenmond
Copy link

leonvonrabenmond commented Sep 8, 2022

I managed to get Quarto to find jupyter (and other kernels, such as the IJulia one) after installing it as a python package, rather then the standard nix package. With Tex being installed it allows me to render to pdf perfectly, but it completely fails under HTML with

ERROR: NotFound: No such file or directory (os error 2), remove '/home/leon/.cache/quarto/sass/397ef2e52d54cf686e4908b90039e9db.css'

As far as I can tell this comes from Deno, but I have no clue what causes it.

pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
@@ -36449,4 +36449,6 @@ with pkgs;
swift-corelibs-libdispatch = callPackage ../development/libraries/swift-corelibs-libdispatch { };

swaysettings = callPackage ../applications/misc/swaysettings { };

quarto = callPackage ../development/libraries/quarto { };
Copy link
Member

Choose a reason for hiding this comment

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

This should be sorted alphabetically into the libraries section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are not all files sorted alphabetical. Also I do not identify a special library section. I hope that is what you mean: 32c7b9f

pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
@@ -14967,4 +14967,10 @@
github = "yisuidenghua";
githubId = 102890144;
};
mrtarantoga = {
Copy link
Member

Choose a reason for hiding this comment

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

This list is sorted alphabetical.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MrTarantoga
Copy link
Contributor Author

I am not sure if it will work with all features. It does not find the jupyter or R environment.
I have some struggle with patching the files to work as expected. The last 2 weeks I have no success with adaption.

I have been playing around with your current patch and I encountered the same issue. Funnily enough, I found that if you add jupyter separately in a nix-shell it works perfectly fine, but when in the enviornment, it somehow can't seem to find it..

Yes, I think quarto expect the FHS and nix does not support this standard. I am reading currently the documentation to support this, but I am not sure if it is possible. Maybe there must be patched the javscript sources. I struggle with the FHS support in nix and in its packages, it is quite hard to understand how it works.

@SuperSandro2000
Copy link
Member

Did you already try to use buildFHSUserEnvBubblewrap ?

@MrTarantoga MrTarantoga force-pushed the add-quarto-1.0.38 branch 2 times, most recently from 34b719c to bdb1f57 Compare September 10, 2022 10:16
Copy link
Contributor

@sepiabrown sepiabrown left a comment

Choose a reason for hiding this comment

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

Updated src and version. Edited according to comments by @smancill and @SuperSandro2000. Adjusted QUARTO_IMPORT_ARGMAP accordingly. Partially tested with quarto preview.

pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/fix-deno-path.patch Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/default.nix Outdated Show resolved Hide resolved
pkgs/development/libraries/quarto/fix-deno-path.patch Outdated Show resolved Hide resolved
@alienzj
Copy link

alienzj commented Sep 25, 2022

Can we merge it?
Since Rstudio 2022.07.1+554 request new version quarto.

Fix: Sort mrtarantoga alphabetical
Quarto is a library/support package for several pandoc projects.
Fix: Remove trailing whitespace


Add final newline to default.nix


Fix indention and newline errors


Fix: Missed unpackPhase resolved


Add: sourceProvenance

Suggestion of: NixOS#186697 (comment)
Fix: reduce imports


Fix: use version attribute in download string


Fix: reduce path expression


Fix: add runHook {pre/post}Install as common practice


Fix: replace gpl2 with gpl2Plus


Fix: change maintainers description


Fix: do not use symlinks, use the PATH instead


Fix wrong platforms

++ means  concatenation and is not correct in this context.

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Change structure to support fixed output derivations

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Fix


Fix


Apply suggestion

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Fix suggestion


Bundle makeProgram function call

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Update library version from 1.0.38 to 1.1.189

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Add missing dependencies

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
pandoc, deno, esbuild can be hooked into QUARTO_* environment variables, so they are not needed in buildInputs.

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Fix deno-path patch

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Remove wrong spaces


Add python3 jupyter host support


Fix spacing

Co-authored-by: Suwon Park <35622998+sepiabrown@users.noreply.github.com>
Sort quarto in all-packages.nix alphabetical

Remove lib prefix from maintainers

Co-authored-by: Sebastián Mancilla <238528+smancill@users.noreply.github.com>
Simplify mkdir command

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Fix missusage of makeBinPath

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Remove python3Packages -> not used

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Remove punctuation from description

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
Fix review 

Error in NixOS#186697 (comment)
Fix wrong path for import_map.json


Do not strip downloaded binary
@MrTarantoga
Copy link
Contributor Author

I squashed now all commits. You can merge if you want, there no constraints from my side.

@alienzj
Copy link

alienzj commented Sep 26, 2022

Thanks all!

@DPDmancul
Copy link
Member

Is there a way to make the engines dependency optional?
For example if I want to use this with Julia or without an engine it downloads and uselessly installs also R, python and jupyter.

I'm not skilled in Nix yet but maybe changing ${python3... with ${pkgs.python3... (and the same for R) should print the path without ensuring the dependency to be installed. Then maybe some overidable options can be added to install engines (which can be defaulted to true in order to not making a breaking change).

@SuperSandro2000
Copy link
Member

I'm not skilled in Nix yet but maybe changing ${python3... with ${pkgs.python3... (and the same for R) should print the path without ensuring the dependency to be installed. Then maybe some overidable options can be added to install engines (which can be defaulted to true in order to not making a breaking change).

That change wouldn't work. Also making options for everything increases the package complexity and all combinations are never tested. The best option would be if quarto would just pickup the packages from PATH and they are not set in the wrapper.

@leonvonrabenmond
Copy link

The best option would be if quarto would just pickup the packages from PATH and they are not set in the wrapper.

If I recall correctly, we did manage to partially get this to work in parts atleast, but the issue was, among others, that some packages did not set their PATHs correctly.

For example if I want to use this with Julia

Julia uses the IJulia kernel for jupyter, but the sadly, quarto does not find jupyter unless it is installed directly as a python package. We could maybe makes this work if we fix all the PATHs. I believe python3 gets found normally, but I have no clue about R.

We could either try and fix the jupyter pathing issue or just remove all but it (assuming R is found normally) I guess...

@06kellyjac
Copy link
Member

FYI share is full of stuff, should probably be in a subdir

ls ./result/share/
 build   capabilities   conf   COPYING.md ... etc ...

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

10 participants