check vevacious deps

[andrewfowlie]

Don't try to build Vevacious if any of its 3 dependencies (minuit2, phc, hom4ps) were ditched. This fixes #358

[andrewfowlie]

@jose let me know what you think. As well as checking minuit2, I checked for phc and hom4ps.

[patscott]

Unfortunately this will have two unintended side effects that make the ditching behaviour differ from the standard:

a) Vevacious will not be listed amongst the ditched backends printed to cout at cmake time (instead there will be one of the new error messages from this commit)
b) Vevacious will still show up in the list of backends that GAMBIT is configured to use when you do ./gambit backends, but will appear as 'not installed'. However, doing make vevacious will result in a 'target not found' error.

I think the way to achieve what you're after is to put the test a lot earlier. The key thing is that Vevacious needs to be added to the itch variable, and that needs to happen both before the check whether it is ditched in backends.cmake, and also before the backend harvester runs at

gambit/CMakeLists.txt

Line 632 in 4917802

add_gambit_custom(backend_harvest backends_harvested BACKEND_HARVESTER BACKEND_HARVESTER_FILES ${itch_with_commas})

[andrewfowlie]

How about making it a fatal error and forcing user to write -Dtich=minuit2,vevacious etc if that's what they really want?

[patscott]

Yeah, that would do the job too. It's arguably not as neat as automating, but it has the advantage of being more explicit.

[andrewfowlie]

Now with FATAL_ERROR rather than WARNING.

[tegonzalo]

There is a crucial problem here. It is currently not possible to ditch vevacious without breaking the gambit build. The type vevacious_1_0::VevaciousPlusPlus::VevaciousPlusPlus, which is defined only the backend types for vevacious (generated by BOSS), will not be found, so linking will fail.

But even if it would be possible, I don't think the way you've implemented this is the right way. All backends that have dependencies make use of the macro check_ditched_status, and if their dependencies are not met, then they are added to the ditched list. So I would recommend something like this in the vevacious block

set(ditch_if_absent "minuit2;phc;hom4ps")
check_ditch_status(${name} ${ver} ${dir} ${ditch_if_absent})

and then modify the macro in externals.cmake to check if the dependency is already ditched.

I think this would be a good change regardless, but something else must be done to avoid the linking problems. All I can think of right now is to store whether vevacious has been ditched in a cmake variable that is then used to define a preprocessor macro, and add guards around the declaration of the function that uses the types. I can take a crack at that later this week if people agree that it's the right way forward.

[andrewfowlie]

set(ditch_if_absent "minuit2;phc;hom4ps")
check_ditch_status(${name} ${ver} ${dir} ${ditch_if_absent})

Will this approach have the problem Pat mentioned? That the itch arguments are are processed long before the check_ditch_status calls, so adding it to the itch arguments in the above would be too late.

[tegonzalo]

Will this approach have the problem Pat mentioned? That the itch arguments are are processed long before the check_ditch_status calls, so adding it to the itch arguments in the above would be too late.

It will show as being ditched in the cmake output. And when running gambit backends it will show as absent/broken, which is the same for all non-installed backends, and all ditched backends, so I don't think that is a problem.

[andrewfowlie]

Sounds good, why doesn't it have the same problem though?

…

On Tue, 7 Jun 2022 at 20:28, Tomas Gonzalo ***@***.***> wrote: Will this approach have the problem Pat mentioned? That the itch arguments are are processed long before the check_ditch_status calls, so adding it to the itch arguments in the above would be too late. It will show as being ditched in the cmake output. And when running gambit backends it will show as absent/broken, which is the same for all non-installed backends, and all ditched backends, so I don't think that is a problem. — Reply to this email directly, view it on GitHub <#359 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA4VQ4IBOCKHJUOVT4X7Z4TVN455JANCNFSM5YBHIMZA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[tegonzalo]

The check_ditch_status macro makes sure to print to screen everything that is ditched.

[andrewfowlie]

Right, right. Why will e.g. gambit backends show absent/broken though? Pat suggested above that changing the ditch_name_ver this late would not work as itch contents is processed beforehand somewhere else. And that consequently you'll get not installed

[tegonzalo]

I think he just meant that the backend harvester will run before it is known that this backend is ditched. Which is definitely true. But that is always been true for many backends that are ditched at that time, such as python and mathematica backends if either python or mathematica are ditched. I don't really see a problem with that, but maybe Pat can elaborate. Worst case scenario, it may be just a case of swapping the order, provided there are no other consequences.

[patscott]

Humm, I had thought that even when we auto-ditch things because Python or Mathematica is not present, they do not show up in gambit backends. If some ditched things show up differently to others, that is an inconsistency that needs to be fixed up regardless. In principle this should all be remedied in the context of #61, #270 and #276, but I'm not sure what the exact status is on those now (#276 at least is still an open PR). My understanding is that those PRs use the backend ditch list generated from the output of the backend harvester to show ditched backends explicitly as 'ditched', so adding things to itch does indeed need to happen before itch_with_commas is created from itch at

gambit/CMakeLists.txt

Line 562 in 4917802

string (REPLACE ";" "," itch_with_commas "${itch}")

This clearly does not happen on the current master when things are ditched because Python/Mathematica is missing.

On the backend type issue, I think it's fine -- ditching a backend does not remove its associated backend types, so this should not cause any problem, right?

[andrewfowlie]

Tomas, I'm confused about another aspect of your proposal.

set(ditch_if_absent "minuit2;phc;hom4ps")
check_ditch_status(${name} ${ver} ${dir} ${ditch_if_absent})

How does this interact with version numbers of the dependencies? Should I in fact be doing

set(ditch_if_absent "minuit2_6.23.01;phc_2.4.58;hom4ps_2.0")

since those are the version numbers that vevacious wants? I'm envisioning having multiple versions of minuit2 etc.

[patscott]

Should I in fact be doing

set(ditch_if_absent "minuit2_6.23.01;phc_2.4.58;hom4ps_2.0")

Yes, I think so, seeing as those are the versions that this version of Vevacious is pinned to by the DEPENDS statements.

[tegonzalo]

How does this interact with version numbers of the dependencies? Should I in fact be doing

set(ditch_if_absent "minuit2_6.23.01;phc_2.4.58;hom4ps_2.0")

since those are the version numbers that vevacious wants? I'm envisioning having multiple versions of minuit2 etc.

If Vevacious necessarily depends on those specific versions, then you're correct. But I don't think that's necessarily the case. In principle it should work with newer versions too, right @JoseEliel?

Humm, I had thought that even when we auto-ditch things because Python or Mathematica is not present, they do not show up in gambit backends. If some ditched things show up differently to others, that is an inconsistency that needs to be fixed up regardless.

I don't think this is completely correct. Afaik, ditched backends still show up in gambit backends, regardless of them being ditched directly (-Ditch option) or later (e.g. in backends.cmake). Currently the only difference between early ditching and ditching done at the time the backend.cmake file is imported, is that the latter will not affect the itch_with_commas, so it won't be part of the harvesting scripts. To me it seems more logical to ditch the backends where they are defined in backends.cmake, rather than elsewhere, so my suggestion is to just move the line

include(cmake/externals.cmake)

right before the itch_with_commas variable is created. This will set up all the backend targets before the harvester scripts are set. I cannot think of any nasty side-effects this may have off the top of my head. I'll do some tests to confirm that nothing is wrong with this.

On the backend type issue, I think it's fine -- ditching a backend does not remove its associated backend types, so this should not cause any problem, right?

This is strange. Since reading this I've been trying to reproduce the error I saw earlier when ditching vevacious, but it seems to have disappeared. I will try a few things to see if it pops up again, but it's pretty odd.

[tegonzalo]

This is strange. Since reading this I've been trying to reproduce the error I saw earlier when ditching vevacious, but it seems to have disappeared. I will try a few things to see if it pops up again, but it's pretty odd.

Ok, I can confirm that this indeed happens. From a fresh build, if ditching vevacious directly from the cmake command, it will fail to link because of the backend type. It does not happen if vevacious is ditched afterwards (e.g. backends.cmake), so it definitely has to do with the harvester scripts. I don't think this is intentional, backend types should not be ditched when the backend is ditched, so I'll check what's going on with the harvesting.

[patscott]

Thanks for chasing these up @tegonzalo, all your plans sound good.

Going forward (maybe a topic for the Core session in Kelowna), I wonder if we really want/need to provide the possibility to ditch backends at all any longer. Having them not ditched doesn't mean you need to build them, and keeping the ditching facility for them has some pretty non-trivial maintenance overhead, as we're seeing in this PR for example. The only advantage of ditching is that you don't need to compile their frontends and types.

[tegonzalo]

Just a quick follow up, the problem with ditching vevacious is that if it's ditched before the harvesting scripts are set (which at the moment happens before the backends.cmake file is imported), the frontend header of vevacious is ditched and thus the typedefs that make vevacious' types usable in gambit are not built, the types are undefined and linking fails. I suspect this would also happen for any BOSSed backend whose types we use in the declaration of functions (e.g. Pythia event types for the Rivet interface).

I believe for now the preprocessor guards option is the best one, as it can be done right away for vevacious only without affecting much else. Long term we could do what Pat suggests (not ditching), or find an alternative. I'll implement that on this branch asap.

[patscott]

Shouldn't it work to just move the typedefs to the backend types file and include that from the frontend header? This is how the backend_types system was designed to be used as far as I can remember.

[anderkve]

Shouldn't it work to just move the typedefs to the backend types file and include that from the frontend header? This is how the backend_types system was designed to be used as far as I can remember.

I guess that should work, but it will require some BOSS modifications, since the current frontend header is auto-generated by BOSS. So perhaps BOSS should instead generate a backend types header that contains these typedefs.

Long term we could do what Pat suggests (not ditching), or find an alternative.

I also think the option of not ditching backends sounds like a good strategy.

[patscott]

So perhaps BOSS should instead generate a backend types header that contains these typedefs.

Yes, definitely; this would be good practice even if we don't ditch ditching.

[patscott]

Closing as obsolete in favour of #440.