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

use :include in install stanza #256

Closed
timbertson opened this issue Sep 16, 2017 · 11 comments · Fixed by #6139 or ocaml/opam-repository#22317
Closed

use :include in install stanza #256

timbertson opened this issue Sep 16, 2017 · 11 comments · Fixed by #6139 or ocaml/opam-repository#22317
Assignees

Comments

@timbertson
Copy link
Contributor

I have a package which installs a server program, but also a bunch of resource files (fonts, images, CSS styles etc). I can happily use jbuilder to install the server binary, but I can't figure out how to install the resources without listing them explicitly (not much fun since this includes a large number of files that I didn't write, copied from twitter-bootstrap).

My first thought was that maybe I could write a rule to generate resource-files.sexp and then (:include resource-files.sexp) that in my (files ...) stanza, but that isn't allowed. Is it possible to add this?

@ghost
Copy link

ghost commented Sep 22, 2017

I was thinking of allowing globs in (install ...) stanzas. Would that be enough for your case?

@timbertson
Copy link
Contributor Author

I don't think so - in this case I'd like to:

  • include everything under res/ (recursively)
  • follow symlinks in this traversal
  • drop the res from the destination path (e.g. res/images/foo becomes images/foo)

I could work around any of those being missing (particularly the third, it's just a cosmetic thing), but it doesn't seem like a great fit given that none of them are currently supported by globs.

Aside: it would be pretty neat from an extensibility point of view if pretty much everything supported (:include) - giving an escape hatch which is less severe than generating the whole file. Are you trying to keep the interface simple by not allowing that, or is it difficult / costly to add support each place it's allowed?

@ghost
Copy link

ghost commented Sep 29, 2017

(:include) introduces dynamism in the jbuilder build graph, so it can only be used where jbuilder supports it. At the moment, jbuilder doesn't support dynamic targets. In particular, this mean that after reading all the jbuild files, jbuilder is able to answer the following question without running any command: is X a target?.

jbuilder uses install stanzas for two things: generating .install files and maintaining a tree of symlinks under _build/install that is an image of what we'll be installed. The later is used to make it easier to have dependencies that are part of the same workspace. For instance, in jbuild files you can write ${lib:foo:bar} which means file bar of library foo. If library foo is not available in the current workspace then ${lib:foo:bar} will be resolved to a path where foo is installed on the system; for instance /usr/local/lib/foo/bar. Otherwise it will be resolved to something like _build/install/default/lib/foo/bar, which would be a symlink to _build/default/foo/src/bar for instance.

If we allow arbitrary (:include ...) in install stanzas, then we can no longer answer the quesion is _build/install/X a target without running commands.

There is a prototype to extend jbuilder to support dynamic targets, but it's not ready yet.

To come back to your use case, are files under res generated? Otherwise, I think something equivalent to (files_recursively_in X) would work well.

@timbertson
Copy link
Contributor Author

Ahh, I see. That does make it tricky.

Couldn't you assume that if someone references ${lib:foo:bar} then foo needs to be located, but not go any further than that? In particular, I guess it would mean that any requested file for an external library is assumed to be buildable (and you know its path because you know where foo is to be installed), and it's a runtime error if it fails to exist after installation (since I assume installation is atomic; you wouldn't just install specific files which were needed from external libraries).

To rephrase, I guess what I'm suggesting is to terminate the "is X buildable" at the library layer, not the file layer, since jbuilder can't look inside non-jbuilder libraries anyway. But I have no idea if that would work, so it's up to you whether that's a useful suggestion :)

To come back to your use case, are files under res generated?

Not individually, and not by jbuilder - they're externally-built (npm packages, multiple size variants of input images)

Otherwise, I think something equivalent to (files_recursively_in X) would work well.

Yeah, that would work well for my use case, as long as it followed symlinks (or could be made to). Would it have src and dest prefixes (i.e. "install files from res/ into share/www")? That would be useful, to prevent the source code layout restricting the installed layout.

@ghost
Copy link

ghost commented Jan 29, 2018

Sorry, I was away for a while and I forgot to reply.

Couldn't you assume that if someone references ${lib:foo:bar} then foo needs to be located, but not go any further than that?

That's what happens already. However, when you do want to build the file in question, you need to find out what produces it. This can be worked out and I think it eventually this will be possible, but this requires some work on the internals of jbuilder and it's not a quick change.

BTW, until we have a better solution for this issue, with the tip of jbuilder you can do the following:

(include jbuild.inc)

(rule (with-stdout-to jbuild.inc.gen (run ./gen-list-of-files-to-install.exe)))

(alias
 ((name update-jbuild)
  (action (diff jbuild.inc jbuild.inc.gen))))

The jbuild.inc file will be in the source tree (rather than in _build). You'll have to create an empty one initially, then whenever you want to update it, you can do:

$ jbuilder build @update-jbuild --auto-promote

If you only want to check that it is up-to-date, you can do:

$ jbuilder build @update-jbuild

@timbertson
Copy link
Contributor Author

Thanks for the idea and the diff action! I've (finally ;)) tried this out now, and hit a couple of snags:

Firstly, when I pass --auto-promote the command still exits with 1 when the generated file differs. So I end up having to run:

jbuilder build @src/www/jbuild-res.inc --auto-promote || jbuilder build @src/www/jbuild-res.inc --auto-promote

Which is awkward, but I'm assuming this should be an easy fix.

(I also chose to name the alias the same as the file in question, not sure if that's more or less confusing ;))

But secondly, I can't get my .gen file to be regenerated. If I change some file which affects the list of generated resources (but which isn't ./gen-list-of-files-to-install.exe itself), I can't get jbuilder to ever regenerate the file, even if I ask for it explicitly (with jbuilder build @src/www/jbuild-res.inc.gen). It just doesn't bother running, because none of its known dependencies are out of date. I think this is essentially another case of #255 (but maybe slightly easier, since I'm explicitly requesting it be built).

Since the whole point of this is to include files which were built outside jbuilder, having jbuilder refuse to regenerate the file makes it difficult. I can at least use an external build tool to just build jbuild-res.inc.gen myself explicitly, but it's quite a dance to get right and make sure the file actually gets regenerated.

@ghost
Copy link

ghost commented Mar 8, 2018

For the exit code, the idea is that because the file has changed, you might need to build again to get a fix point.

Running a command every time is a bit complicated as it's not clear what to do once you support polling builds. Maybe the generator could generate the dependencies? For instance the rule that generate the file depends on an alias that you extend in the generated file?

@timbertson
Copy link
Contributor Author

timbertson commented Mar 8, 2018

For the exit code, the idea is that because the file has changed, you might need to build again to get a fix point.

Hmm, fair enough. I still think it should be 0 on --auto-promote though, since if you need to distinguish that case couldn't you just not use --auto-promote? i.e.:

if ! jbuilder build @diff-target; then
  jbuilder build @diff-target --auto-promote
  jbuilder build all-the-other-things
fi

The current workaround is fine though 🤷‍♂️

Maybe the generator could generate the dependencies?

Maybe. This particular target would be affected by:

  • updating my bootstrap dependency version (this is effectively depending on package.json)
  • me manually adding another image into the source location where res/images is generated from (i.e. depend on all files in images/)
  • me modifying any of the build scripts which control what paths are created in res/, which currently is:
gup/_build/bootstrap.gup
gup/src/www/res/all.gup
gup/src/www/res/fonts.gup
gup/src/www/res/images.gup

...but if I add to that set or any of these scripts start sourcing other scripts, I'd need to manually add those to the list of dependencies which should cause a rebuild (I have no automated way to get this list).

So it could be done, but it's quite a lot of effort to maintain an accurate dependency list. And if I miss something, the result is an incorrect build which I can't force jbuilder to rebuild. For me, the time wasted by running the command on every build would be well worth the reduced effort in accurately tracking dependencies, since it takes ~0.03s.

@ghost
Copy link

ghost commented Mar 9, 2018

BTW, currently you can always use the OCaml syntax as a workaround. The code will be executed on each invocation of jbuilder.

@mseri
Copy link
Member

mseri commented Sep 24, 2018

I was discussing with @rgrinberg the possibility to use :include or a glob in the install stanza. For me glob would be enough, I'd be happy to give a shot to the feature if you point me to the right direction

@rgrinberg
Copy link
Member

rgrinberg commented Sep 24, 2018

Actually, I think that we should just finish off #962 first. Let's support static variables first before considering (:include ..). @mseri would you like to finish off that PR, I believe the original author likely abandoned it anyway. Note that (:include ..) is incompatible with the current model of the install dir so it wouldn't be so easy to support

@rgrinberg rgrinberg self-assigned this Jul 28, 2022
emillon added a commit to emillon/opam-repository that referenced this issue Oct 19, 2022
…ne-site, dune-rpc, dune-rpc-lwt, dune-private-libs, dune-glob, dune-configurator, dune-build-info, dune-action-plugin and chrome-trace (3.5.0)

CHANGES:

- macOS: Handle unknown fsevents without crashing (ocaml/dune#6217, @rgrinberg)

- Enable file watching on MacOS SDK < 10.13. (ocaml/dune#6218, @rgrinberg)

- Sandbox running cinaps actions starting from cinaps 1.1 (ocaml/dune#6176, @rgrinberg)

- Add a `runtime_deps` field in the `cinaps` stanza to specify runtime
  dependencies for running the cinaps preprocessing action (ocaml/dune#6175, @rgrinberg)

- Shadow alias module `Foo__` when building a library `Foo` (ocaml/dune#6126, @rgrinberg)

- Extend dune describe to include the root path of the workspace and the
  relative path to the build directory. (ocaml/dune#6136, @reubenrowe)

- Allow dune describe workspace to accept directories as arguments.
  The provided directories restrict the worskpace description to those
  directories. (ocaml/dune#6107, fixes ocaml/dune#3893, @esope)

- Add a terminal persistence mode that attempts to clear the terminal history.
  It is enabled by setting terminal persistence to
  `clear-on-rebuild-and-flush-history` (ocaml/dune#6065, @rgrinberg)

- Disallow generating targets in sub direcories in inferred rules. The check to
  forbid this was accidentally done only for manually specified targets (ocaml/dune#6031,
  @rgrinberg)

- Do not ignore rules marked `(promote (until-clean))` when
  `--ignore-promoted-rules` (or `-p`) is passed. (ocaml/dune#6010, fixes ocaml/dune#4401, @emillon)

- Dune no longer considers .aux files as targets during Coq compilation. This
  means that .aux files are no longer cached. (ocaml/dune#6024, fixes ocaml/dune#6004, @Alizter)

- Cinaps actions are now sandboxed by default (ocaml/dune#6062, @rgrinberg)

- Allow rules producing directory targets to be not sandboxed (ocaml/dune#6056,
  @rgrinberg)

- Introduce a `dirs` field in the `install` stanza to install entire
  directories (ocaml/dune#5097, fixes ocaml/dune#5059, @rgrinberg)

- Menhir rules are now sandboxed by default (ocaml/dune#6076, @rgrinberg)

- Allow rules producing directory targets to create symlinks (ocaml/dune#6077, fixes
  ocaml/dune#5945, @rgrinberg)

- Inline tests are now sandboxed by default (ocaml/dune#6079, @rgrinberg)

- Fix build-info version when used with flambda (ocaml/dune#6089, fixes ocaml/dune#6075, @jberdine)

- Add an `(include <file>)` term to the `include_dirs` field for adding
  directories to the include paths sourced from a file. (ocaml/dune#6058, fixes ocaml/dune#3993,
  @gridbugs)

- Support `(extra_objects ...)` field in `(executable ...)` and `(library
  ...)` stanzas (ocaml/dune#6084, fixes ocaml/dune#4129, @gridbugs)

- Fix compilation of Dune under esy on Windows (ocaml/dune#6109, fixes ocaml/dune#6098, @nojb)

- Improve error message when parsing several licenses in `(license)` (ocaml/dune#6114,
  fixes ocaml/dune#6103, @emillon)

- odoc rules now about `ODOC_SYNTAX` and will rerun accordingly (ocaml/dune#6010, fixes
  ocaml/dune#1117, @emillon)

- dune install: copy files in an atomic way (ocaml/dune#6150, @emillon)

- Add `%{coq:...}` macro for accessing data about the configuration about Coq.
  For instance `%{coq:version}` (ocaml/dune#6049, @Alizter)

- update vendored copy of cmdliner to 1.1.1. This improves the built-in
  documentation for command groups such as `dune ocaml`. (ocaml/dune#6038, @emillon,
  ocaml/dune#6169, @shonfeder)

- The test suite for Coq now requires Coq >= 8.16 due to changes in the
  plugin loading mechanism upstream (which now uses `Findlib`).

- Starting with Coq build language 0.6, theories can be built without importing
  Coq's standard library by including `(stdlib no)`.
  (ocaml/dune#6165 ocaml/dune#6164, fixes ocaml/dune#6163, @ejgallego @Alizter @LasseBlaauwbroek)

- on macOS, sign executables produced by artifact substitution (ocaml/dune#6137, ocaml/dune#6231,
  fixes ocaml/dune#5650, fixes ocaml/dune#6226, @emillon)

- Added an (aliases ...) field to the (rules ...) stanza which allows the
  specification of multiple aliases per rule (ocaml/dune#6194, @Alizter)

- The `(coq.theory ...)` stanza will now ensure that for each declared `(plugin
 ...)`, the `META` file for it is built before calling `coqdep`. This enables
 the use of the new `Findlib`-based loading method in Coq 8.16; however as of
 Coq 8.16.0, Coq itself has some bugs preventing this to work yet. (ocaml/dune#6167 ,
 workarounds ocaml/dune#5767, @ejgallego)

- Allow include statement in install stanza (ocaml/dune#6139, fixes ocaml/dune#256, @gridbugs)

- Handle CSI n K code in ANSI escape codes from commands. (ocaml/dune#6214, fixes ocaml/dune#5528,
  @emillon)

- Add a new experimental feature `mode_specific_stubs` that allows the
  specification of different flags and sources for foreign stubs depending on
  the build mode (ocaml/dune#5649, @voodoos)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment