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

ResolveMojo should not be part of assemble cycle #3199

Open
maxonfjvipon opened this issue May 23, 2024 · 11 comments
Open

ResolveMojo should not be part of assemble cycle #3199

maxonfjvipon opened this issue May 23, 2024 · 11 comments
Assignees
Labels

Comments

@maxonfjvipon
Copy link
Member

maxonfjvipon commented May 23, 2024

There's a strange reason why ResolveMojo and MarkMojo are in assemble cycle.

ResolveMojo downloads .jar packages from Maven Central. Package contains .class files and directory EO-SOURCES with .eo source files.

MarkMojo scans EO-SOURCES directory and extends foreign-tojos with visible EO objects.

We actually need this MarkMojo inside assembly cycle because of atoms that may create EO objects that are not visible in EO sources.

So what we need:

  1. Move ResolveMojo out of assembly cycle so it downloads and unpack all jars after assembling only once.
  2. Introduce some mechanism of downloading all EO sources by the name of the dependency name:
  • name eo-runtime -> download all sources for eo-runtime,
  • name eo-collections -> download all sources for eo-collections

This mechanism should be placed instead of ResolveMojo in assemble cycle

@yegor256 WDYT?

@maxonfjvipon maxonfjvipon changed the title ResolveMojo should not be part of assemble cycle ResolveMojo and MarkMojo should not be part of assemble cycle May 23, 2024
Copy link

@maxonfjvipon the issue body is empty, please provide more details for this problem.

@maxonfjvipon maxonfjvipon changed the title ResolveMojo and MarkMojo should not be part of assemble cycle ResolveMojo should not be part of assemble cycle May 23, 2024
@yegor256
Copy link
Member

@maxonfjvipon why do we need to scan EO-SOURCES by the way? Shouldn't we get a list of all necessary objects from the objects already downloaded? I mean, from their sources. If my project depends on the org.example.foo object, the pull step will download the org/example/foo.eo file from Objectionary, and then inside this file it will find the list of all dependencies (other objects that need to be downloaded).

Maybe the problem is that org.example.foo object may use the org.example.bar object without the use of +alias meta. This is why we need the resolve step inside the assemble?

@maxonfjvipon
Copy link
Member Author

maxonfjvipon commented May 23, 2024

@yegor256 org.example.foo object may have atom inside and only god know what objects are used inside atom. For example:

+package org.example

[] > foo
  "Hello" > x
  [] > @ /true

Here foo depends only on org.eolang.string (let's imagine org.eolang.string does not depend on anything more) BUT it also has an atom foo$@ which, let's imagine, creates org.eolang.int for some reason and maybe some other objects and returns org.eolang.bool. And we don't see it at the level of EO. That's why we scan EO-SOURCES and downloads all of the objects so they can be used inside atoms

@yegor256
Copy link
Member

@maxonfjvipon maybe let's make it mandatory for every object to mention all dependencies that it has, right in the .eo file with the help of +alias annotation? If this annotation is not there, atoms are not allowed to use that objects (to be checked in runtime during testing, for example).

@maxonfjvipon
Copy link
Member Author

maxonfjvipon commented May 23, 2024

@yegor256 sounds good. Could you please clarify a bit how we should restrict usages of objects inside atoms?

@maxonfjvipon
Copy link
Member Author

@yegor256 maybe we can also introduce some new annotation for such cases to make it different from +alias?

@yegor256
Copy link
Member

@maxonfjvipon actually, +alias won't work, since we demand that all objects mentioned by the +alias are used in the EO code. Thus, maybe it can be +also -- this will mean that this current object "also" requires other objects.

Then, inside Java implementation of atoms we may simply let programmers either 1) use new EOorg.EOlang.EOint() in order to create an object, or use Phi.Φ.get("org").get("eolang").get("int").copy() (which is a preferred way). If they use the second option, we have a chance to catch it during testing and verify whether it's a legal request (the +also meta was present).

@maxonfjvipon
Copy link
Member Author

maxonfjvipon commented May 23, 2024

@yegor256 first option new EOorg.EOlang.EOint() is not really legal because object won't be copied properly and won't have \rho in such case. I think we should prohibit it somehow. Phi.Φ.take("org").take("eolang").take("int") is the only right way.
But I still don't understand how we can really catch it if we're inside atom and don't have access to EO source file

@yegor256
Copy link
Member

@maxonfjvipon when we compile .eo file, we can wrap the place when an atom is declared:

+also xyz
[] > bar
  [] > foo /bytes

becomes in Java:

class EObar {
  public EObar() {
    this.add("foo", new AtAlso(new AtOnce(...), ["xyz"]);
  }
}

Inside this new AtAlso class we have a list of available objects to use.

How will AtAlso know that the object it encapsulates goes to a particular object inside Phi.Φ? Well, let's think...

@maxonfjvipon
Copy link
Member Author

@yegor256 until we figure out how to implement this checking we can:

  1. just introduce +also meta
  2. don't scan EO-SOURCES but scan all +also metas in source code
  3. if some +also meta is missed - object will be just failed to create

@yegor256
Copy link
Member

@maxonfjvipon good plan

@maxonfjvipon maxonfjvipon self-assigned this May 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants