search

build2 / build2

build2 build system
MIT License
591 stars 15 forks source link

Assertion failed: `(!tp.empty () || t.mtime () == timestamp_unreal` #253

Open helmesjo opened 1 year ago

helmesjo commented 1 year ago
  1. Unzip build2-install-bug.zip
  2. bdep init -C @clang cc config.cxx=clang++
  3. b install: hello/ !config.install.root=./out !config.install.scope=project
Assertion failed: (!tp.empty () || t.mtime () == timestamp_unreal), function perform_install, file rule.cxx, line 1012.
fish: Job 2, 'b install: hello/ !config.ins...' terminated by signal SIGABRT (Abort)

Reproduced on MacOS, haven't tested Linux or Windows.

bdep 0.16.0-a.0.34e232023e8f
libbpkg 0.16.0-a.0.424fcd390192
libbutl 0.16.0-a.0.c6c224a78715
boris-kolpackov commented 1 year ago

The trigger is the custom ad hoc recipe to install exe{hello}:

exe{hello}:
% install
{{
  ...
}}

The cxx module which provides the rule to build exe{hello} actually supplied two rules, to update and to install, which perform a pretty tricky dance in order to be able to link things differently when building for install, etc. So what happens here is that you kept the update rule provided by cxx but replaced install with your own ad hoc recipe which commits all kinds of faux pas during that dance. In this particular case, the update rule from cxx picked one of the lib{} group members (libs{} in this case) when linking the executable. The install rule from cxx also knows to do the same but not your ad hoc recipe, which tries to install both members of lib{} while only libs{} was updated.

I think at this stage you have only two options: either use both rules from cxx or neither because it's not going to be easy to support that dance in an ad hoc recipe written in Buildscript (your could probably pull it off in C++ but I wouldn't recommend it). If you need to "hook" some post-install steps to an executable or library built by cxx, I suggest that you use an alias, for example:

./: alias{hello}: exe{hello}: ...

alias{hello}:
% install
{{
  ...
}}

We will also try to fix this assert and issue proper diagnostics at some point but it's not a high priority right now.

helmesjo commented 1 year ago

Okay, thanks. I gave the example with alias{} a try but still got the built for install error though. Edit. Never mind, I accidentally added it as a prerequisite to the dir target with ./: alias{} instead of just alias{}.