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 cannot render #349444

Closed
carschandler opened this issue Oct 18, 2024 · 21 comments · Fixed by #349683
Closed

quarto cannot render #349444

carschandler opened this issue Oct 18, 2024 · 21 comments · Fixed by #349683
Labels
0.kind: bug Something is broken 9.needs: upstream fix This PR needs upstream to change something

Comments

@carschandler
Copy link

carschandler commented Oct 18, 2024

Describe the bug

quarto render file.qmd gives an unexpected error.

Steps To Reproduce

Steps to reproduce the behavior:

{
  description = "Quarto Devshell";

  inputs = {
    nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable";
  };

  outputs =
    { nixpkgs, ... }:
    let
      forAllSystems =
        function:
        nixpkgs.lib.genAttrs [
          "x86_64-linux"
          "aarch64-darwin"
        ] (system: function nixpkgs.legacyPackages.${system});
    in
    {
      devShells = forAllSystems (pkgs: {
        default = pkgs.mkShell {
          buildInputs = [
            (pkgs.quarto.override {
              extraPythonPackages = ps: [
                ps.numpy
                ps.pandas
                ps.xarray
              ];
            })
            pkgs.texliveFull
          ];
        };
      });
    };
}

Then running quarto render on a trivial qmd file yields:

ERROR: TypeError: Attempted to load JSON module without specifying "type": "json" attribute in the import statement.

Stack trace:
    at async initYamlIntelligenceResourcesFromFilesystem (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:82360:29)
    at async file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:78778:9
    at async initState (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:78786:5)
    at async render (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:82898:5)
    at async Command.actionHandler (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:83077:32)
    at async Command.execute (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:8017:13)
    at async Command.parseCommand (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:7907:20)
    at async quarto (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:118224:9)
    at async file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:118244:9
    at async mainRunner (file:///nix/store/ks6vcvx9iys46wjnwbchqb76bjzp6ffz-quarto-1.5.57/bin/quarto.js:118128:9)

Notify maintainers

@minijackson @MrTarantoga

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

 - system: `"x86_64-linux"`
 - host os: `Linux 6.6.56, NixOS, 24.11 (Vicuna), 24.11.20241014.a3c0b3b`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.24.9`
 - nixpkgs: `/nix/store/xnjw9gmfmpppdj6bxpw6cfkspc3h6xwl-source`

Add a 👍 reaction to issues you find important.

@carschandler carschandler added the 0.kind: bug Something is broken label Oct 18, 2024
@detroyejr
Copy link
Contributor

Seems to be due to something in the flake itself?

For example, the following works:

nix-shell -p 'pkgs.quarto.override { extraPythonPackages = ps: with ps; [numpy pandas xarray];}'

Then use quarto render on the snippet below:

---
title: Render Test
---

# Test that rendering works.


```{python}
import numpy as np
import pandas as pd
import xarray as xr
data = xr.DataArray(np.random.randn(2, 3), dims=("x", "y"), coords={"x": [10, 20]})

data
```

Should give you an html file with the python block rendered correctly.

@carschandler
Copy link
Author

@detroyejr running that nix-shell and document give me the same error unfortunately

@detroyejr
Copy link
Contributor

Got it. Yeah, seems like some change broke something within the last couple of days.

I expect this to work on the QMD snippet you posted.

let 
  pkgs = import (builtins.fetchTarball "https://github.com/nixos/nixpkgs/archive/194846768975b7ad2c4988bdb82572c00222c0d7.tar.gz") {};
in
pkgs.mkShell {
  packages = [ 
    (pkgs.quarto.override {
      extraPythonPackages = ps: [
        ps.numpy
        ps.pandas
        ps.xarray
      ];
    }) 
    pkgs.texliveFull
  ];
}

@carschandler
Copy link
Author

Yeah pinning to that version worked

@Atemu
Copy link
Member

Atemu commented Oct 21, 2024

Given that deno_1 has been dropped, you'll either have to revive it and become its maintainer or get upstream to fix compat with deno_2. Check whether there's already work in that direction.

@gkapfham
Copy link

Hi @carschandler and @Atemu and @detroyejr, I also use Quarto on NixOS and I can confirm that I see exactly this error when I used the newest version of Quarto from Nix unstable. What I am temporarily doing to solve the problem is to revert the version of Quarto from Nix stable.

Although I am not a NixOS expert, I would be glad to help by testing possible solutions to this problem. Do you have a specific plan forward in terms of reviving Deno 1 or finding a way to ensure that Quarto works with Deno 2?

I did a quick search of the issue tracker and discussion forum for quarto_cli and I did not see an mention of this specific problem. Although I saw some issues that might be tangentially connected, I could not find any discussion of this specific rendering error or details about Quarto's upgrade path to Deno 2.

I'm sure that you are all super-busy --- but when you have time, please let me know what are your plans for resolving this issue and if there is any way in which I can help. Thanks in advance!

@carschandler
Copy link
Author

carschandler commented Oct 22, 2024

I don't have much to offer right now either other than my current alternative which is to use pixi (conga-like package manger from prefix.dev) with nix-ld. I am also willing to help with guidance. I do think we should at least find out what the timeline for quarto on deno2 is like if it exists at all.

@Atemu
Copy link
Member

Atemu commented Oct 22, 2024

Please note that I don't use this software or even know what it does.

In order to bring back deno_1, you need to bring back the code that was removed as if you were adding back a package. Given that deno_1 is EOL and likely to quickly accumulate CVEs, bringing it back means some friction as we generally don't want insecure or abandonware in Nixpkgs.

Bringing it back as a regular package this would bring with it some expectations of the package generally working though which you might not be willing/capable to ensure. What would be easier to accept would be to basically "vendor" the package definition in the definition for quarto and not expose it in the regular pkgs; adding it back only for the purpose of making quarto work until it supports deno_2. The easiest way to achieve that would be to restore the deleted Nix files in quarto's package directory and callPackage them from within quarto's package definition.

You should pre-emptively mark the deno_1 package as known vulnerable in any case though.

You must get in contact with upstream about this though. They need to be aware that a critical dependency of theirs is EOL and will therefore be unsupported by any distro worth its salt.

cc @emilazy

@detroyejr
Copy link
Contributor

detroyejr commented Oct 22, 2024

Thanks. Yeah, this all makes sense. I'll open an issue with upstream and see how long it might take to migrate to version 2. If its a ways out, we can consider the vendor route.

@gkapfham
Copy link

Hi, for reference, there is this discussion in the quarto-cli issue tracker about the Deno version: quarto-dev/quarto-cli#10809.

The summary from the final comment is: "1.6.19 runs Deno 1.46.3 now."

However, my impression is that would not be a suitable version for resolving this issue that now NixOS does not support any Deno 1 versions. (I may be interpreting the version number for Deno incorrectly, with that said).

Okay, once there is an issue raised and you want me to comment, I would be glad to do so. I'm also willing to test a new release of Quarto in NixOS unstable if you want me to help. Let me know!

@Atemu
Copy link
Member

Atemu commented Oct 22, 2024

Hi, for reference, there is this discussion in the quarto-cli issue tracker about the Deno version: quarto-dev/quarto-cli#10809.

That's about updating to a not quite as old deno_1 version, not deno_2. Please ask them about deno version 2.

@detroyejr
Copy link
Contributor

I've modified my PR to use the callPackage route. We can go ahead with that if we need a fix sooner.

@mcanouil
Copy link

mcanouil commented Oct 23, 2024

If I may, this build issue is unrelated to Quarto.
Quarto bundles specific and tested dependencies for some supported platforms (Nix is not one of them unfortunately).
The valid dependencies are https://github.com/quarto-dev/quarto-cli/blob/main/configuration for the development version. (The file can be found for other versions as well)

I don’t know Nix, but if it cannot use the proper dependencies, users should be warned that they are running a custom unstable build of Quarto.

Note that the Deno 2 issue is the most visible one as it leads to crashes but Pandoc dependency not being the one used officially by Quarto can lead to much less prominent issues but still important issues.

@emilazy
Copy link
Member

emilazy commented Oct 23, 2024

We really ought not to reintroduce Deno 1 for the 24.11 release cycle. V8’s JIT is a highly complex attack surface by itself and Deno is a huge codebase with many security‐relevant dependencies on top of that. Upstream aren’t going to support it, so we can’t really do so ourselves; for cases like this (browsers, media libraries, complex language runtimes, etc.) we use knownVulnerabilities to mark EOL packages. For #349683 to be merged it would have to mark the vendored Deno 1 package with knownVulnerabilities explaining that the version is EOL and unsupported upstream, like we do with old Node.js releases, but this would mean that everyone using the package would have to opt in to compiling it themselves, which is unlikely to be a pleasant experience. Even when deno_1 was introduced in the original PR, the plan was to remove it before the 24.11 release.

Ultimately, the problem here is that Quarto relies on software that is not supported by the relevant maintainers. As a downstream, there’s not much we can do to make that a sustainable situation. I appreciate @mcanouil commenting about Quarto’s dependency expectations – unfortunately it’s not viable for us to maintain an entire parallel ecosystem of package versions for one package; doing this for every package that wants hard dependency pins would result in an unsustainable combinatorial explosion of maintenance effort in Nixpkgs, and greatly complicate our ability to effectively remediate security vulnerabilities in widely‐used software.

For now, the best approach may be to use the Quarto package from Nixpkgs 24.05, which will be security‐supported until the end of the year (and past that, you’re already accepting some security risk by using Deno 1 anyway, so continuing to use the Nixpkgs 24.05 version with appropriate precautions may be adequate). However, if this kind of issue applies to many of Quarto’s dependency pins and we’re likely to continue to run into these sorts of issues, then it may not be viable to package it in Nixpkgs and users might be better‐served by, say, using the binary distributions with nix-ld or similar.

@mcanouil
Copy link

mcanouil commented Oct 23, 2024

Thanks for the comment about Nix ecosystem!

I totally understand the dependency tree issue. That's also why Quarto has a "slow" dependency update policy (at most once per release). This means Quarto will almost always be behind Nix hence the source of many issues for Nix users using Quarto.

Disclaimer: I'm not one of the maintainers of Quarto CLI, only collaborating on the project.

@emilazy
Copy link
Member

emilazy commented Oct 23, 2024

Yeah, ultimately if there’s no way around a hard requirement on specific pinned versions and it's going to continually lag behind what upstreams support, then the best approach may be for people to volunteer to, say, maintain a Nix flake including derivations for the required dependency versions upstream in the Quarto repository. But I don’t think Nixpkgs itself can really take on that burden, so our choices are to exploit wiggle‐room in the dependency versions and accept that the package might sometimes break, or remove it entirely.

@Atemu Atemu added the 9.needs: upstream fix This PR needs upstream to change something label Oct 27, 2024
@Atemu
Copy link
Member

Atemu commented Oct 27, 2024

Is the package still useful without the render function?

@mcanouil
Copy link

mcanouil commented Oct 27, 2024

Without dependency pinning, the package will only "work" by chance. It's fundamentally broken.

NixOS users using this custom build of Quarto are unlikely to have a great experience and will very likely face issues everytime there is a Deno, Pandoc, etc. update.

I'm commenting as a user and collaborator on the Quarto project but still not a maintainer.

@Atemu
Copy link
Member

Atemu commented Oct 27, 2024

I only care about the current issues of quarto right now, not possible future issues.

I don't use this package or know anything about it. Does it make any sense to have without the render functionality?

If not, we should mark it as broken.

@mcanouil
Copy link

mcanouil commented Oct 27, 2024

I don't use this package or know anything about it. Does it make any sense to have without the render functionality?

The current issue is the same as future issues. All comes down to the same thing, i.e., no dependency pinning.

The main feature is quarto render.
As I said, this package is fundamentally broken without dependency pinning.

In other words, there is no way for this package to work as intended by the Quarto maintainers (which are not the ones behind this NixOS package as the official answer is that Quarto does not support NixOS distribution at least for a foreseeable future).

Edit: in case it's not clear, putting back Deno 1.* won't change anything as Deno is not the only dependency on which Quarto relies on.

@detroyejr
Copy link
Contributor

The PR I've proposed fixes the main issue being discussed in this PR without requiring an older version of deno. I don't think there's necessarily a need to mark as broken just yet as it works well enough.

I appreciate what @mcanouil is saying. We can't guarantee that this won't break in the future. However I'd still lean toward keeping this in if possible. I don't want nix users bothering the quarto team (like I did in quarto-dev/quarto-cli#11169 (comment) 🙃) so if there's a good way to mitigate that, I'm all for it. Given the nature of nix (especially if you're on unstable) I think it's generally understood among our users that if something is wrong it's our problem not yours.

Personally, I use quarto quite a bit and would love to keep benefiting from regular updates even if we're not officially supported, but I'm also happy to do something else if these other concerns take priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug Something is broken 9.needs: upstream fix This PR needs upstream to change something
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants