Skip to content

Conversation

@craigfe
Copy link
Contributor

@craigfe craigfe commented Mar 31, 2021

No description provided.

@ghost
Copy link

ghost commented Mar 31, 2021

Thanks for the repro case. Could you use dune-build-info instead of the cache to reproduce this bug? @snowleopard is doing some restructuring of the code for the shared cache and the cache will stop calling git. So this repro case will soon stop working.

@snowleopard
Copy link
Collaborator

Thanks for the repro case. Could you use dune-build-info instead of the cache to reproduce this bug? @snowleopard is doing some restructuring of the code for the shared cache and the cache will stop calling git. So this repro case will soon stop working.

Thanks, indeed.

@craigfe After some upcoming changes, Dune cache will no longer require VCS information. The only reason it currently needs it is to support bulk download of build artifacts corresponding to a given commit but we found that in practice downloading artifacts for individual rules is simpler and fast enough.

@craigfe
Copy link
Contributor Author

craigfe commented Apr 1, 2021

Thanks both. I've altered the reproduction to avoid Dune caching.

I wasn't able to get a reproduction w/ dune-build-info. Interestingly, the following works just fine:

  Initialized empty Git repository in $TESTCASE_ROOT/.git/
  $ echo "(executable (name main) (libraries dune-build-info))" > dune
  $ cat >main.ml <<EOF
  > ;; Printf.printf "internal version: %s\n"
  > ( match Build_info.V1.version () with
  > | None -> "None"
  > | Some v -> Build_info.V1.Version.to_string v )
  > EOF
  $ dune exec ./main.exe
  Info: Creating file dune-project with this contents:
  | (lang dune 2.8)
  internal version: None

(Probably just me missing something obvious.) I had a quick look through the internals, but couldn't work out why the emitted vcs-describe placeholder isn't translated into a failing Git command somewhere. Will take a deeper look at some point when I get the time; dune-build-info seems a more compelling reproduction than dune subst.

@ghost
Copy link

ghost commented Apr 1, 2021

It's because we don't include the actual info when building executables in the _build directory. This is so that dune-build-info doesn't hurt incrementality, otherwise we'd end up rebuilding a bunch of stuff everytime we do a commit. There are two ways of getting the actual info:

  • with dune install
  • by adding (promote (until-clean)) to the executable stanza to make dune promote the exe

@craigfe
Copy link
Contributor Author

craigfe commented Apr 1, 2021

Thanks for the explanation :-) Now using dune-build-info properly.

_
Signed-off-by: Jeremie Dimino <[email protected]>
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@ghost ghost merged commit e8ad49f into ocaml:main Apr 1, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants