carpentries / workbench

Repository for Discussions and Materials about The Carpentries Workbench
https://carpentries.github.io/workbench/
Creative Commons Attribution 4.0 International
17 stars 7 forks source link

including setup.md as only file in learners config field causes Actions workflow to fail #72

Closed tobyhodges closed 10 months ago

tobyhodges commented 10 months ago

Although it does not cause any problem when I build locally, listing setup.md under learners: in config.yaml causes the Build and Deploy workflow to fail on GitHub with the following cryptic error message:

'' does not exist in current working directory ('/home/runner/work/upgraded-engine/upgraded-engine').

I have created a minimal example at https://github.com/tobyhodges/upgraded-engine

Here is the output of sessionInfo() on my local system, where sandpaper::serve() runs without issue:

``` R version 4.3.1 (2023-06-16) Platform: aarch64-apple-darwin20 (64-bit) Running under: macOS Ventura 13.5.2 Matrix products: default BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib LAPACK: /Library/Frameworks/R.framework/Versions/4.3-arm64/Resources/lib/libRlapack.dylib; LAPACK version 3.11.0 locale: [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8 time zone: Europe/Berlin tzcode source: internal attached base packages: [1] stats graphics grDevices utils datasets methods base loaded via a namespace (and not attached): [1] utf8_1.2.3 generics_0.1.3 renv_1.0.2 xml2_1.3.5 tinkr_0.2.0 digest_0.6.33 [7] magrittr_2.0.3 evaluate_0.21 fastmap_1.1.1 jsonlite_1.8.7 rprojroot_2.0.3 processx_3.8.2 [13] whisker_0.4.1 promises_1.2.1 ps_1.7.5 purrr_1.0.2 fansi_1.0.4 cli_3.6.1 [19] rlang_1.1.1 commonmark_1.9.0 xslt_1.4.4 withr_2.5.0 cachem_1.0.8 yaml_2.3.7 [25] tools_4.3.1 sandpaper_0.13.2 memoise_2.0.1 dplyr_1.1.3 httpuv_1.6.11 credentials_1.3.2 [31] assertthat_0.2.1 mime_0.12 vctrs_0.6.3 R6_2.5.1 lifecycle_1.0.3 fs_1.6.3 [37] varnish_0.3.0 pkgconfig_2.0.3 desc_1.4.2 callr_3.7.3 later_1.3.1 pkgdown_2.0.7 [43] pillar_1.9.0 glue_1.6.2 Rcpp_1.0.11 servr_0.27 gert_1.9.3 xfun_0.40 [49] tibble_3.2.1 tidyselect_1.2.0 rstudioapi_0.15.0 sys_3.4.2 knitr_1.44 htmltools_0.5.6 [55] rmarkdown_2.24 pegboard_0.6.1 compiler_4.3.1 downlit_0.4.3 askpass_1.2.0 openssl_2.1.0 ```
zkamvar commented 10 months ago

Is there a reason why you would list setup.md? What would you expect to see from this? It gets stripped from the menus because it's a section in the index and not a separate page.

tobyhodges commented 10 months ago

I can imagine circumstances in which a person would do this accidentally e.g. if they add a bunch of files to learners/ and then decide they want to control the order those pages appear in the dropdown menu. They might forget that setup.md doesn't need to be included, and add it to the config.yaml based on the list of filenames in the directory. I think that's a realistic scenario.

But mostly I wanted to report this because of the inconsistency in behaviour between my local setup and the Actions workflow, which combined with the non-obvious error message and the fact that the issue seemed to spring up spontaneously earlier this week, made it very difficult to debug.

zkamvar commented 10 months ago

Ah, gotcha. That's a very good point. Thank you for finding the root cause of this and creating a MWE.

In particular, this bug was just added in the update to the 404 page that addressed https://github.com/carpentries/sandpaper/issues/498.

The nitty gritty of what's happening is that, when the 404 page is built from a deployment function (one that starts with ci_), then it will convert all of the relative links in the navigation to absolute URLs.

The problem arises when there is nothing to be had in one of the navigation menus for the learners, which can occur in two situations:

  1. the only file in the learners/ folder is setup.md
  2. the only file listed in the learners: item of config.yaml is setup.md

If you have the following config, it will work as expected.

learners:
- reference.md
- setup.md

That being said, this is definitely a bug that should be rectified.

zkamvar commented 10 months ago

Because this is causing existing builds to fail, I will add a quick fix, but it does beg the question: what happens when someone does not want anything to appear in the "more" section? At the moment, there's no good way to do that.

tobyhodges commented 10 months ago

The problem arises when there is nothing to be had in one of the navigation menus for the learners, which can occur in two situations:

  1. the only file in the learners/ folder is setup.md
  2. the only file listed in the learners: item of config.yaml is setup.md

Indeed that makes it much less likely to come up. Thanks for clarifying.

tobyhodges commented 10 months ago

Because this is causing existing builds to fail, I will add a quick fix, but it does beg the question: what happens when someone does not want anything to appear in the "more" section? At the moment, there's no good way to do that.

My immediate thought is that the More dropdown should not be displayed if there's nothing to populate it. If that is challenging to implement, I would suggest some subtly-styled placeholder text that says something like "nothing here".

zkamvar commented 10 months ago

This was fixed in https://github.com/carpentries/sandpaper/pull/520