search

conda-forge / quarto-feedstock

A conda-smithy repository for quarto.
BSD 3-Clause "New" or "Revised" License
4 stars 9 forks source link

1.5.55 #52

Closed mfisher87 closed 3 months ago

mfisher87 commented 3 months ago

Closes #44

Checklist

conda-forge-webservices[bot] commented 3 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/meta.yaml) and found it was in an excellent condition.

mfisher87 commented 3 months ago

@conda-forge-admin, please rerender

github-actions[bot] commented 3 months ago

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

I tried to rerender for you but ran into some issues. Please check the output logs of the latest webservices GitHub actions workflow run for errors. You can also ping conda-forge/core for further assistance or you can try rerendeing locally.

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

mfisher87 commented 3 months ago

@cderv I got us past a few failures to a point where the build completes but fails to test with (placeholder dir shortened for readability):

ERROR: Error executing '/home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/tools/x86_64/typst': No such file or directory (os error 2)

Stack trace:                                                                                   
    at execProcess (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:4717:15)
    at checkVersions (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:101118:37)
    at eventLoopTick (ext:core/01_core.js:153:7)
    at async check (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:101066:13)
    at async Command.actionHandler (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:101389:5)
    at async Command.execute (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:8017:13)
    at async Command.parseCommand (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:7907:20)
    at async quarto (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:118219:9)
    at async file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:118239:9
    at async mainRunner (file:///home/conda/feedstock_root/build_artifacts/quarto_1722356633693/_test_env_placehold/bin/quarto.js:118123:9)

I only had a little bit of time to focus on this so I'm going to set it down for now.

mfisher87 commented 3 months ago

@conda-forge-admin, please rerender

cderv commented 3 months ago

@mfisher87 this is quite cryptic error to me.

First is the long path with placehold multiple times expected ? Just to be sure.

Then, what is quarto_1722356633693/_test_env_(...truncated...) ? Is this where Quarto is installed ?

I don't know how this works for other tooling bundled, but checkVersions() is the place in Quarto's code where each tool is run with --version to see if we get what is expected.

For typst, the binary used is determined this way 

export function typstBinaryPath() {
  return Deno.env.get("QUARTO_TYPST") ||
    architectureToolsPath("typst");
}

Do we need to set QUARTO_TYPST ? It seems this is done for other tools https://github.com/conda-forge/quarto-feedstock/blob/ffd9cc438828492200aa8eac59a1e663128d9126/recipe/bld.bat#L5-L7

My understanding of conda packaging here is that our downloaded tools are not used but conda-forge equivalent are, and I see you modified this PR to use typst from conda-forge. So maybe just setting QUARTO_TYPST is missing ?

mfisher87 commented 3 months ago

First is the long path with placehold multiple times expected ? Just to be sure.

:laughing: yeah, that's a normal Conda thing. I forgot why, I learned at some point and only kept the knowledge that it's normal :)

Then, what is quarto_1722356633693/_testenv(...truncated...) ? Is this where Quarto is installed ?

This is where Quarto is installed during the build process. It's one of the final steps of installing the package and then running the items under the test: key in meta.yaml.

My understanding of conda packaging here is that our downloaded tools are not used but conda-forge equivalent are, and I see you modified this PR to use typst from conda-forge. So maybe just setting QUARTO_TYPST is missing ?

I think this is a perfect description of what's going on. You've accurately described what I'm trying to do and filled in exactly the gaps that were missing in my mind! Thank you. I should remember this detail by now! :laughing: :bow:

cderv commented 3 months ago

You've accurately described what I'm trying to do and filled in exactly the gaps that were missing in my mind!

😄 Good team work ! 🤝

mfisher87 commented 3 months ago

Getting closer! @cderv any chance you have Windows hardware or experience that could help identify the problem? May be a bit until I can set up a VM.

ERROR: Error executing 'D:/bld/quarto_1722382665224/_test_env/Library/bin\typst.exe': The system cannot find the file specified. (os error 2)

The slash is the thing that jumps out the most, but I followed the pattern in bld.bat. :thinking:

cderv commented 3 months ago

The slash is the thing that jumps out the most, but I followed the pattern in bld.bat.

I don't think this is it. Usually it is handled ok. Like this, nothing jumped out so I tried on my windows machine to understand how typst conda package was working and I found the problem. 

> conda create -n myenv
> conda activate myenv
> conda install -c conda-forge typst=0.11.0`

See where the typst.exe is now 

❯ (gcm typst.exe).Source
C:\Users\chris\miniconda3\envs\myenv\bin\typst.exe

Not where you would expect.

It has been fixed in 0.11.1 conda forge bundle it seems

> conda install -c conda-forge typst
> (gcm typst.exe).Source
C:\Users\chris\miniconda3\envs\myenv\Library\bin\typst.exe

So you probably need to adapt the path for the Windows recipe is this is possible

We can see they change it recently

mfisher87 commented 3 months ago

Nice catch, thanks @cderv! Do you think there can be an upstream release that upgrades the typst dependency to 0.11.1? That way we won't need to change the recipe once to work around this and then change it again once Quarto upgrades typst.

cderv commented 3 months ago

Do you think there can be an upstream release that upgrades the typst dependency to 0.11.1? That way we won't need to change the recipe once to work around this and then change it again once Quarto upgrades typst.

There won't be a 1.5 release with 0.11.1 IMO. We don't update dependencies in patch release, unless really security issue to fix maybe. Also 0.11.1 has some issues

So I think we'll need to do the change for the 1.5 build, and then when 1.6 is out, re-adapt the code.

mfisher87 commented 3 months ago

Shoot! Thanks for considering it. Let's try this :)

mfisher87 commented 3 months ago

@cderv if things look good to you, feel free to merge! I'll probably click merge sometime tomorrow if I don't hear anything :)

cderv commented 3 months ago

You did good by merging. I was off today - start of summer break for me. Thanks for the update on this ! I am glad we manage to build it !

Just did not have time to warn you that we did a new patch release today 😬 - sorry.

We do one usually first of each month, so that you know.

Thanks again !

mfisher87 commented 3 months ago

Happy to help, but looks like things didn't go quite perfectly :grimacing:

After releasing, looks like #53 also impacts 1.5. When I built it locally, the file was present, but somehow it's not present in the automated build. I'm really confused.

cderv commented 3 months ago

I'll think about how to debug this 🤔 Maybe something is wrong with the copy to what is bundled ?