Page MenuHomePhabricator

build: install eo files per default
ClosedPublic

Authored by bu5hm4n on Sep 29 2019, 10:03 AM.

Details

Summary

in the last release we turned that off, because we started to stabelize
API back there, but the .eo file format wasnt ready yet.

Now, the file format is stable. And we stabelized more widgets, which
means, we should also install the .eo files per default.

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bu5hm4n created this revision.Sep 29 2019, 10:03 AM
bu5hm4n requested review of this revision.Sep 29 2019, 10:03 AM
zmike added a comment.Sep 29 2019, 1:53 PM

Should this be restricted to only installing stable eo class files?

I do not think so, that would mean we need to pull apart the lists beforehand.

I am a bit unsure if we really put this into this release or if we should apply it for the next.
@q66 Any comment from you on this?

zmike requested changes to this revision.Oct 28 2019, 6:05 AM

I think the option here should be changed: we should always install eo files for stable classes, and this option should be to control installing eo files for beta classes.

This revision now requires changes to proceed.Oct 28 2019, 6:05 AM

@zmike @stefan_schmidt can we get back to this ?

@zmike this is sadly not possible the state of a .eo file is not known to meson, addtionally, *all* .eo files are needed in order to check the integrity of the .eo files on the system.

bu5hm4n requested review of this revision.Apr 21 2020, 7:50 AM
zmike resigned from this revision.Apr 21 2020, 8:02 AM
zmike edited reviewers, added: stefan_schmidt; removed: zmike.

yea whatever

One step back. Why do we want to have the eo files in the system? Developing against our API would not need it.
I assume it for external bindings?

Not a big fan of dumping them all into the system either I have to say. If we split up the list by having one for stabilized APIs and one for the rest? And only installing the stable list this should work, no?
The extra work would be creating the lists but there is nothing technical that prevents from doing so or am I missing something?

We want them for external bindings, clouseau, my efl-ui editor is depending on it as well.

Splitting stable and beta up is not working, for the reasons i have given in my last reply ...

You mean you the "in order to check the integrity of the .eo files on the system." statement?

Why would stable eo files need non-stable eo files for integrity? It might be clear to you but its honestly not to me.
I would imagine you would need all stable eo files to work together as set, agreed, but where comes the dependency on the other files?

If you could explain why this is not possible I would be less resistant in dumping them all in the system.

Sure :)

  1. The "problem" is on the one side, that one .eo files can contain stable structs as well as beta classes or the other way arround.
  2. for methods that are beta, the parameters also can be beta. However, we still validate them when using this externally.
  3. Right now the process of "unbetaing" something is simply removing the @beta tag, i am not sure if we also want to move the file there in the buildsystem for that...
  4. We allow beta interfaces to be implemented by none beta classes, hence for the validation of the implements { } block, we *need* to have them available ...
stefan_schmidt accepted this revision.Apr 22 2020, 8:35 AM

Sigh, seems we are way less strict on this than I thought we are. While I think moving files when declaring things stable would be ok, the situation in 4 makes the idea to have them split a bit useless.

I am not happy to have this all dumped into the system, but I see the need and no chance to get this sorted out without a *major* surgery. :/

This revision is now accepted and ready to land.Apr 22 2020, 8:35 AM
Closed by commit rEFLe6a62186971d: build: install eo files per default (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyApr 28 2020, 5:44 AM
This revision was automatically updated to reflect the committed changes.