Quarto 1.4.549 - Githubissues

SaravananSathyanandhaQC commented 7 months ago

Removes a patch as per https://github.com/conda-forge/quarto-feedstock/pull/33

Checklist

[x] Used a personal fork of the feedstock to propose changes
[ ] Bumped the build number (if the version is unchanged)
[x] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[ ] Ensured the license file is being packaged.

conda-forge-webservices[bot] commented 7 months ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

mfisher87 commented 7 months ago

@SaravananSathyanandhaQC would you like to be added as a maintainer?

mfisher87 commented 7 months ago

I haven't seen this before, I think this is new now that we're trying to build 1.4.

It looks like copyPandocScript function may need to be updated to ensure the target directory exists before making the symbolic link. I'm not 100% sure of that and I've forgotten some of my bag of tricks; it's been a while since I've built a conda package :) If that is the problem, we can patch, but I'd like to get upstream involved if the fix is that simple. We can probably get a quick turnaround on an upstream PR and avoid a patch :rocket:

Are you on the Conda Forge Matrix server? We could chat there to work through some of the issues in a more real-time way if you'd like!

mfisher87 commented 7 months ago

:thought_balloon: While we're in here, should we consider updating these patches so they don't require offsets or fuzzing?

[[ RA-MD1LOVE ]] - [[                             patches/0001-win-conda-path-overrides.patch ]]
[[ RA-MD1L-VE ]] - [[                              patches/0001-log-deno-bundle-command.patch ]]
[[ RA-MD1L--E ]] - [[             patches/0001-remove-QUARTO_VERSION-from-configuration.patch ]]

mfisher87 commented 7 months ago

This patch gets me past the error, but now I get a warning.

diff --git a/package/src/common/configure.ts b/package/src/common/configure.ts
index e5f83603e..a77316b26 100644
--- a/package/src/common/configure.ts
+++ b/package/src/common/configure.ts
@@ -154,7 +154,9 @@ export function copyPandocScript(config: Configuration, targetDir: string) {
   }

   if (Deno.build.os !== "windows") {
-    info("> creating pandoc symlink");
+    info("> creating pandoc symlink in " + targetDir);
+    Deno.run({cmd: ["mkdir", "-p", targetDir]});  
     Deno.run({
       cwd: targetDir,
       cmd: ["ln", "-s", linkTarget, "pandoc"]

Warning:

/opt/conda/lib/python3.10/site-packages/conda_build/build.py:1672: UserWarning: file /home/conda/feedstock_root/build_artifacts/quarto_1707866209788/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_plac/bin/tools/pandoc is a symlink with no target
  warnings.warn("file %s is a symlink with no target" % path, UserWarning)

mfisher87 commented 7 months ago

I seem to have also gotten a build without that warning, but not sure how. After installing that locally-built package, I can do quarto create-project just fine, but quarto preview prints a bunch of these:

WARNING: No such file or directory (os error 2): readfile '/path/to/miniconda3/envs/quarto-1.4-test/share/quarto/preview/quarto-preview.js'

Indeed, no such file is there. Although it does actually render a website!

I have to set this down for now, but hopefully the above was helpful :)

mfisher87 commented 7 months ago

@cderv any thoughts on the symlink issue we're seeing and upstreaming the patch?

dragonstyle commented 7 months ago

Are the dependencies for Quarto being downloaded as a part of configure in this case? If not, that is likely the source of the issue as that portion of the configuration is expecting pandoc to have been downloaded and placed in that directory. If that isn't possible in this case, we could consider just making that WARN that the symlink couldn't be created or as you note, just creating the directory.

edit: I'd be in favor of upstreaming whatever you need so you don't have to keep patching Quarto!

mfisher87 commented 7 months ago

edit: I'd be in favor of upstreaming whatever you need so you don't have to keep patching Quarto!

Awesome, thanks :)

Are the dependencies for Quarto being downloaded as a part of configure in this case? If not, that is likely the source of the issue as that portion of the configuration is expecting pandoc to have been downloaded and placed in that directory.

I'm not completely sure! I've been learning as I go and the configuration process has been a bit mystifying to me. Apologies for needing my hand held a little bit here :) Is there any documentation on the configuration process you could point me to, or if not, perhaps we could collaborate to make some?

I see from the following from my test build:

configure.sh is downloading Deno Stdlib, then calling quarto-bld configure:
- quarto-bld configure outputs the following. From reading the code (dependencies.ts), if these were downloaded, a log would have been written to indicate that (info(`Downloading ${dependency.name}`);), but log entry is not present:
```
Preparing deno_dom (linux - x86_64)
Configuring deno_dom
deno_dom complete.

Preparing Pandoc (linux - x86_64)
Configuring Pandoc
Pandoc complete.

Preparing Dart Sass Compiler (linux - x86_64)
Configuring Dart Sass Compiler
Dart Sass Compiler complete.

Preparing esbuild javscript bundler (linux - x86_64)
Configuring esbuild javscript bundler
esbuild javscript bundler complete.

Preparing Typst (linux - x86_64)
Configuring Typst
Typst complete.
```
- Should this step have failed :point_up: ? Have we misconfigured something that causes the downloads to not occur?
- The build continues and eventually completes successfully.
  - In the built env, we have a bin/tools/pandoc symlink pointing at x86_64/pandoc; however, that symlink is the only thing in bin/tools, so the symlink points to nothing!
  - If I do which pandoc, pandoc is present in my env at bin/pandoc, which is not what the symlink expects. How did that happen?

dragonstyle commented 7 months ago

Ok, I think I see what is happening (and you are essentially correct in your comment above). Typically, when we configure Quarto, we place all the various binary dependencies within the bin directory (you'd see progress messages indicating that we are downloading the various dependencies and emplacing them). In this recipe, however, QUARTO_VENDOR_BINARIES=false instructs us not to emplace the binaries, so we essentially skip placing the various dependencies in place.

https://github.com/quarto-dev/quarto-cli/blob/4cc168d3806d36437ec36b32fc924b361df31753/package/src/common/dependencies/dependencies.ts#L74

Because we never place pandoc, the symlink fails.

I think the right fix is to respect the vendoring flag when attempting to make that symlink as well. Does that make sense?

dragonstyle commented 7 months ago

Ok, I've made the upstream fix here:

https://github.com/quarto-dev/quarto-cli/commit/75264062555182478fa37e45040b6cad8e1cec21

this just missed a patch release of Quarto 1.4, so is in our pre-release builds of Quarto 1.5.

For the time being, I think it may make sense to just use that commit to patch 1.4.449 (or .450) until we release our next path release (I can back port this to our 1.4 branch)- does that work?

mfisher87 commented 7 months ago

Right on @dragonstyle, thank you for your expertise here!

I will try and do a test build with a patch from the commit you linked. I'll include information about the expected lifetime of the patch in the patch message, and link back to your comment above so future us can get back on the same page :)

May be a bit later tonight, or even tomorrow. Not sure yet :)

mfisher87 commented 7 months ago

Got a successful build with this change, commit pushing shortly. However, when I test, I still get so much spam when I run quarto preview:

WARNING: No such file or directory (os error 2): readfile '/home/robatt/.local/share/miniconda3/envs/quarto-1.4-test/share/quarto/preview/quarto-preview.js'

Every time a page loads, dozens of these print. But in the browser, everything seems to be working great.

mfisher87 commented 7 months ago

I was able to get this to build with this heavy-handed patch: e86cb4b

I'm not sure how / if this could be addressed upstream. @dragonstyle thoughts?

dragonstyle commented 7 months ago

That's a reasonable patch for the time being - I will make an upstream change that will probably just force rebuilding it when configure is run, but that will have the same issue as the other upstream fix (need to keep the patch for now).

That JS handles auto-reload of previews (e.g. run quarto preview, then modify the qmd and ensure reload happens), just FYI.

mfisher87 commented 7 months ago

Sounds great. I'll try building a couple more of my projects and then I think we can merge this.

mfisher87 commented 7 months ago

I was able to build ~5 different projects, some using Jupyter, some using LaTex, some RevealJS. Let's go!

mfisher87 commented 7 months ago

OSX build had a timeout downloading mamba forge. I think we need some magic incantations to either update the feedstock or just re-run.

mfisher87 commented 7 months ago

@conda-forge-admin, please rerender

github-actions[bot] commented 7 months ago

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/quarto-feedstock/actions/runs/7924564999.

mfisher87 commented 7 months ago

@conda-forge-admin, please restart ci

mfisher87 commented 7 months ago

I'm not sure why some of the builds aren't being restarted. I'm going to wait it out and then restart CI again and see if that works. :weary:

dragonstyle commented 7 months ago

(Upstream fix to preview issue here:

https://github.com/quarto-dev/quarto-cli/commit/78ae90e29e813cdd211fc5763318b88fc61bdfca )

Both fixes are back ported and will be in our next patch release of 1.4 (no ETA at this time)

Thank you for all your work on this @mfisher87 !!!

mfisher87 commented 7 months ago

Amazing! Same to you @dragonstyle , pleasure working with you :)

cderv commented 7 months ago

hey @mfisher87 !

Sorry I was off past week. Thanks a lot for the all the work, and thanks @dragonstyle for helping with this !

Awesome !

mfisher87 commented 7 months ago

:heart: Welcome back, hope you had a restful break! :) It's always a pleasure working with you all.

conda-forge / quarto-feedstock

Quarto 1.4.549 #36