#1361 Add eo-runtime dependency even if foreign dependencies are absent #1357

volodya-lombrozo · 2022-10-20T18:12:47Z

Closes: #1361

I've added one more step in ResolveMojo. It adds runtime dependency if it is absent.
That solution allows build EO programs without foreing objects.

volodya-lombrozo · 2022-10-21T09:08:36Z

@mximp @yegor256 pls, take a look

mximp · 2022-10-21T09:49:04Z

@volodya-lombrozo can we squash commits?

mximp · 2022-10-21T09:56:37Z

@volodya-lombrozo please run the build with Qulice locally.

volodya-lombrozo · 2022-10-21T10:00:22Z

@volodya-lombrozo please run the build with Qulice locally.

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------

mximp · 2022-10-21T09:54:08Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

+    /**
+     * Check if runtime dependency is absent.
+     * @param deps Dependencies
+     *  fixme: We are using hardcoded version of RuntimeDependency.


@volodya-lombrozo I suppose this needs to be a puzzle?

I've tried to create a puzzle, but it complains that I have to link to some issue. As far as I know, I have to put current issue #35, but we are not in eoc repository. eo repository has it's own #35 issue.

I've tried to create a puzzle, but it complains that I have to link to some issue. As far as I know, I have to put current issue #35, but we are not in eoc repository. eo repository has it's own #35 issue.

In this case we must create new issue within eo repository since it is a fix for EO. And link this PR to EO issue.

New issue #1361 in eo repo

mximp · 2022-10-21T09:56:02Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

@@ -194,6 +194,7 @@ private Collection<Dependency> deps() throws IOException {
            tojo.set(AssembleMojo.ATTR_JAR, coords);
        }
        this.checkConflicts(deps);
+        addRuntimeDependency(deps);


@volodya-lombrozo wrong style - should be called via class name.

mximp · 2022-10-21T10:02:21Z

eo-maven-plugin/src/main/java/org/eolang/maven/RuntimeDependency.java

+ *
+ * @since 0.28.11
+ */
+final class RuntimeDependency {


@volodya-lombrozo Since it's specific to EO runtime - I would call it EoRuntime

Renamed to EoRuntimeDependency

mximp · 2022-10-21T10:02:53Z

eo-maven-plugin/src/main/java/org/eolang/maven/RuntimeDependency.java

+     * @param other Other dependency
+     * @return True if other dependency is the eo-runtime dependency
+     */
+    boolean theSameAs(final Dependency other) {


@volodya-lombrozo Could be simplified to same

mximp · 2022-10-21T10:03:56Z

eo-maven-plugin/src/main/java/org/eolang/maven/RuntimeDependency.java

+     *
+     * @return Maven Dependency
+     */
+    Dependency toDependency() {


@volodya-lombrozo Could be simplified to dependency

fix(objectionary#35): add RuntimeDependency to classpath if it's absent fix(objectionary#35): add test for empty dependencies situation fix(objectionary#35): make all tests pass fix(objectionary#35): add todo mark for hardcoded version of RuntimeLibrary fix(objectionary#35): move all binary expressions to start of a line fix(objectionary#35): don't align chained method calls fix(objectionary#35): fix all linter mistakes in ResolveMojoTest fix(objectionary#35): fix some linter mistakes in MockMavenCentral fix(objectionary#35): fix all linter mistakes in MockMavenCentral fix(objectionary#35): fix all linter mistakes in ResolveMojo fix(objectionary#35): fix all linter mistakes in RuntimeDependency fix(objectionary#35): correct todo for pdd feat(objectionary#35): fix linter problems in comments feat(objectionary#35): fix all codacy complains

volodya-lombrozo · 2022-10-21T10:52:54Z

@volodya-lombrozo can we squash commits?

Done

toDependency -> dependency theSameAs -> same RuntimeDependency -> EoRuntimeDependency RuntimeDependency -> EoRuntimeDependency use classname for static function call

mximp · 2022-10-21T13:19:36Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

+     * @param dependency Dependency
+     * @return True if dependency is runtime dependency
+     */
+    private static boolean isRuntimeDependency(final Dependency dependency) {


@volodya-lombrozo This can be inlined. I believe no need in separate static method.

mximp · 2022-10-21T13:21:03Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

+     * @return True if dependency is runtime dependency
+     */
+    private static boolean isRuntimeDependency(final Dependency dependency) {
+        return new EoRuntimeDependency().same(dependency);


@volodya-lombrozo May we create only one instance of EoRuntimeDependency and reuse it?

mximp · 2022-10-21T13:47:39Z

@volodya-lombrozo looks good to me.
@yegor256 please merge (if no comments from your side).

yegor256 · 2022-10-22T09:39:45Z

@volod looks very good, but I have a few questions. First, why do we need to create a new class EoRuntimeDependency if we already have Dependency class form Maven? In general, keep a simple rule in mind: if your class doesn't implement any interfaces -- you most probably are doing something wrong. It seems that you only need equals from this new class. Make this equals a new class, or just a new method. But EoRuntimeDependency seems to be a pretty procedural bag of data.

Second, MockMavenCentral has two public methods, while it implements an interface. This is again a mistake in OOP. If you implement a type/interface, you DON'T add anything new to the surface of the object. The method assertLastDownloadedDependencyAndPathEquals (aside from the fact that the name is super ugly :) must be taken away from this class and be implemented as a new Hamcrest assertion (instance of Assertion), I believe.

volodya-lombrozo · 2022-10-22T11:51:03Z

@yegor256 pls, take a loot at the last changes. I’ve change MockMavenCentral class as you suggested and removed the EoRuntimeDependency.

As for EoRuntimeDependency, It definitely looks like a “bag with data” for now, I agree. But in the future I think we will Implement something like HttpEoRuntimeDependency or ConstantEoRuntimeDependency or ConfigEoRuntimeDependency - all of them will implement their own way of retrieving a eo runtime dependency from different sources. All of that classes will have one public interface method toMavenDependency and a common interface of cource.

Also, it’s a quite dangerous to use Dependency class directly, because it has a bad architecture (not an OOP style at all) which influences a lot on overall program design (please, take a look on that code). Moreover, we can’t change that class. I think it’s better to encapsulate it and to use something our own with a better design.

yegor256 · 2022-10-23T05:31:26Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

@@ -194,6 +194,7 @@ private Collection<Dependency> deps() throws IOException {
            tojo.set(AssembleMojo.ATTR_JAR, coords);
        }
        this.checkConflicts(deps);
+        ResolveMojo.addRuntimeDependency(deps);


@volodya-lombrozo check this out: https://www.yegor256.com/2015/12/08/temporal-coupling-between-method-calls.html

In order to obrain the issue I had to add a few private static classes. Please, take a look at AllDependencies and UniqueDependencies classes.

yegor256 · 2022-10-23T05:32:10Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

+     *  or from config files.
+     */
+    private static void addRuntimeDependency(final Collection<Dependency> deps) {
+        final RuntimeDependencyEquality runtime = new RuntimeDependencyEquality();


@volodya-lombrozo check this out: https://www.yegor256.com/2015/09/01/redundant-variables-are-evil.html

yegor256 · 2022-10-23T05:43:53Z

@volodya-lombrozo please, forgive me for being picky. I think that MockMavenCentral is over-design in this case. Here is why: The live class Central goes to Maven Central, finds the necessary .jar file, downloads it and unpacks into a given directory. You want to check that your class, that is using Central, is asking it to download and unpack the right .jar. The best and and simplest approach would be to create DummyCentral class, which only creates an empty directory in the required path. Then, you just check the presence of such directory in your tests and that's it.

What you are doing now with encapsulating mutable structures in MockMavenCentral and then checking their content afterwards is "asserting on side-effect" -- not a perfect object design.

volodya-lombrozo · 2022-10-23T18:54:11Z

@volodya-lombrozo please, forgive me for being picky. I think that MockMavenCentral is over-design in this case. Here is why: The live class Central goes to Maven Central, finds the necessary .jar file, downloads it and unpacks into a given directory. You want to check that your class, that is using Central, is asking it to download and unpack the right .jar. The best and and simplest approach would be to create DummyCentral class, which only creates an empty directory in the required path. Then, you just check the presence of such directory in your tests and that's it.

What you are doing now with encapsulating mutable structures in MockMavenCentral and then checking their content afterwards is "asserting on side-effect" -- not a perfect object design.

I've removed MockMavenCentral and added DummyCenral as you suggested. Please, check.

yegor256 · 2022-10-23T19:48:18Z

eo-maven-plugin/src/test/java/org/eolang/maven/LastPathSame.java

+ *
+ * @since 0.28.11
+ */
+final class LastPathSame extends TypeSafeMatcher<Path> {


@volodya-lombrozo this class is redundant now, right?

Yes, I've just forgot to remove the class. Done.

yegor256 · 2022-10-23T19:48:49Z

eo-maven-plugin/src/test/java/org/eolang/maven/LastDependencySame.java

+ *
+ * @since 0.28.11
+ */
+final class LastDependencySame extends TypeSafeMatcher<Dependency> {


@volodya-lombrozo this class is not needed anymore, right?

Yes, I've just forgot to remove the class. Done.

yegor256 · 2022-10-23T19:49:48Z

@volodya-lombrozo check again, please. Looks like these two matchers are not needed anymore.

volodya-lombrozo · 2022-10-25T06:19:25Z

@yegor256 can you merge the PR, please? If everything is fine, of course.

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/ResolveMojoTest.java

# Conflicts: # eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

prepare for refactoring add DcsNoOneTransitive make all tests pass clean ResolveMojo add puzzle for tests for DcsNoOneHasTransitive refactor DcsNoOneHasTransitive fix some linter mistakes fix some linter mistakes fix some linter mistakes fix some linter mistakes

volodya-lombrozo · 2022-10-31T13:30:04Z

Updates for that PR

Sine we implemented Dependencies interface in that PR. It’s convenient for that task to use the same approach with new classes:

DcsWithRuntime - adds Runtime dependency if it absents

DcsWithoutConflicts - checks conflicts (previously that class was a static method in ResolveMojo)

DcsNoOneHasTransitive - decorator to check all dependencies for transitive dependencies (previously it was a static method in ResolveMojo)

@mximp @yegor256 please, check and merge if you don't have any objections.

yegor256 · 2022-10-31T15:19:17Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsNoOneHasTransitive.java

+        final Dependencies delegate,
+        final Func<Dependency, Dependencies> transitive
+    ) {
+        this.ignore = ignore;


@volodya-lombrozo read this, please: https://www.yegor256.com/2016/04/19/object-must-not-be-configurable.html

@yegor256 fixed.

yegor256 · 2022-10-31T15:20:11Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsOf.java

+ *
+ * @since 0.28.11
+ */
+final class DcsOf implements Dependencies {


@volodya-lombrozo what this class is for? Where and why Collection<Dependency> is not good enough?

I wanted to make it more obvious. We can't use it directly in the next construction:

new DcsNoOneHasTransitive( this.ignoreTransitive, dcs, .... )

because Collection<org.apache.maven.model.Dependency> != Dependencies. But, of course, we can use it this way:

new DcsNoOneHasTransitive( this.ignoreTransitive, dcs::iterator, .... )

I will fix it. Thank you.

@yegor256 fixed.

yegor256 · 2022-10-31T15:21:32Z

eo-maven-plugin/src/main/java/org/eolang/maven/RuntimeDependencyEquality.java

+import org.apache.maven.model.Dependency;
+
+/**
+ * RuntimeDependencyEquality is a class for checking if dependency is a runtime dependency.


@volodya-lombrozo are you sure this is an accurate description of functionality here? It looks wrong to me.

@yegor256 fixed.

yegor256 · 2022-10-31T15:23:42Z

eo-maven-plugin/src/test/java/org/eolang/maven/DummyCentral.java

+            );
+        } catch (final IOException ex) {
+            throw new IllegalStateException(
+                String.format("DummyCentral can't save dependency %s by path %s", dependency, path),


@volodya-lombrozo the information about the originator of the exception is in stacktrace. No need to add this noise. Just say "Can't save '%s' to '%s'". In general, the shorter your messages, the better. They must be short, but contain maximum information.

@volodya-lombrozo moreover, if I remove this class tomorrow, I most probably will forget to rename it in the text

@yegor256 fixed

make exception description shorter change the description of RuntimeDependencyEquality remove redundant class don't use configurable objects fix all linter mistakes

yegor256 · 2022-11-01T08:21:22Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsNoOneHasTransitive.java

+    @SuppressWarnings("PMD.AvoidCatchingGenericException")
+    private boolean hasTransitive(final Dependency dep) {
+        try {
+            return new LengthOf(


@volodya-lombrozo use Unchecked from Cactoos, to avoid exception catching

@yegor256 fixed.

yegor256 · 2022-11-01T08:22:03Z

eo-maven-plugin/src/main/java/org/eolang/maven/DcsWithoutConflicts.java

+                )
+            );
+        if (!conflicts.isEmpty()) {
+            final String msg = String.format(


@volodya-lombrozo this is a bad practice: to do both logging and throwing. Pick one.

@yegor256 I've chosen the exception. Done.

yegor256 · 2022-11-01T08:23:49Z

eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

+    private Dependencies dependenciesOf(final Collection<Dependency> all) {
+        final Dependencies dependencies;
+        if (this.ignoreTransitive && this.ignoreVersionConflicts) {
+            dependencies = new DcsWithRuntime(all::iterator);


@volodya-lombrozo looks like code duplication to me. You use DcsWithRuntime everywhere in each if. Maybe it's better to make dependencies mutable?

@yegor256 done.

yegor256 · 2022-11-01T08:25:06Z

eo-maven-plugin/src/test/java/org/eolang/maven/DummyCentral.java

+            Files.createDirectories(path);
+            Files.createFile(
+                path.resolve(
+                    String.format("%s-%s.jar", dependency.getArtifactId(), dependency.getVersion())


@volodya-lombrozo you ignore dependency.getClassifier(), why? maybe it's better to use it too?

@yegor256 added.

yegor256 · 2022-11-01T08:25:43Z

@volodya-lombrozo a few more comments from me, see above

use unchecked classes inside DcsNoOneHashTransitive use unchecked classes inside DcsNoOneHashTransitive remove duplicated logging remove code duplication inside ResolveMojo use classifier for jar name in DummyCentral

volodya-lombrozo · 2022-11-01T09:22:16Z

@yegor256 I've applied all your suggestions above. Please, merge if you don't have any objections.

yegor256 · 2022-11-01T10:57:39Z

@rultor merge

rultor · 2022-11-01T10:58:08Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2022-11-01T11:05:52Z

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 8min)

mximp reviewed Oct 21, 2022

View reviewed changes

volodya-lombrozo force-pushed the runtime_dependency branch from eb19c79 to 0e81929 Compare October 21, 2022 10:52

Fix all review suggestions

3666441

toDependency -> dependency theSameAs -> same RuntimeDependency -> EoRuntimeDependency RuntimeDependency -> EoRuntimeDependency use classname for static function call

volodya-lombrozo changed the title ~~#35 Add eo-runtime dependency even if foreign dependencies are absent~~ #1361 Add eo-runtime dependency even if foreign dependencies are absent Oct 21, 2022

volodya-lombrozo added 3 commits October 21, 2022 15:31

Add puzzle for harcoded runtime dependency

d769ab8

fix puzzle

240ae69

fix puzzle

94c70fd

volodya-lombrozo requested a review from mximp October 21, 2022 12:52

mximp reviewed Oct 21, 2022

View reviewed changes

simplify code in ResolveMojo

9413b25

volodya-lombrozo requested a review from mximp October 21, 2022 13:42

mximp approved these changes Oct 21, 2022

View reviewed changes

volodya-lombrozo added 2 commits October 22, 2022 13:43

fix OOP problems with MockMavenCentral

ecd365d

fix: EoRuntimeDependency -> RuntimeDependencyEquality

3f528df

yegor256 reviewed Oct 23, 2022

View reviewed changes

volodya-lombrozo added 3 commits October 23, 2022 11:43

fix: remove redundant variables

b1cc4a9

fix: use separate classes for checking and adding dependencies

208a4d6

MockMavenCentral -> DummyCentral

f9d9509

volodya-lombrozo requested a review from yegor256 October 23, 2022 18:54

yegor256 reviewed Oct 23, 2022

View reviewed changes

fix: remove unused matchers

f95826d

volodya-lombrozo requested a review from yegor256 October 23, 2022 20:04

volodya-lombrozo and others added 5 commits October 31, 2022 11:54

Merge remote-tracking branch 'origin/master' into runtime_dependency

ed02290

# Conflicts: # eo-maven-plugin/src/test/java/org/eolang/maven/ResolveMojoTest.java

feat(objectionary#1357): solve merge conflicts

9ec81d7

Merge remote-tracking branch 'origin/master' into runtime_dependency

cc433b6

# Conflicts: # eo-maven-plugin/src/main/java/org/eolang/maven/ResolveMojo.java

Merge branch 'objectionary:master' into runtime_dependency

8046a05

yegor256 reviewed Oct 31, 2022

View reviewed changes

Apply all review suggestions.

e46aa73

make exception description shorter change the description of RuntimeDependencyEquality remove redundant class don't use configurable objects fix all linter mistakes

yegor256 reviewed Nov 1, 2022

View reviewed changes

Apply all review suggestions

7ec79c9

use unchecked classes inside DcsNoOneHashTransitive use unchecked classes inside DcsNoOneHashTransitive remove duplicated logging remove code duplication inside ResolveMojo use classifier for jar name in DummyCentral

rultor merged commit a2a898d into objectionary:master Nov 1, 2022

volodya-lombrozo mentioned this pull request Dec 15, 2022

maven-javadoc-plugin fails because eo-runtime.jar is absent in the list of dependencies #1547

Closed

#1361 Add eo-runtime dependency even if foreign dependencies are absent #1357

#1361 Add eo-runtime dependency even if foreign dependencies are absent #1357

Conversation

volodya-lombrozo commented Oct 20, 2022 • edited Loading

volodya-lombrozo commented Oct 21, 2022

mximp commented Oct 21, 2022

mximp commented Oct 21, 2022

volodya-lombrozo commented Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mximp Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

volodya-lombrozo Oct 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

volodya-lombrozo commented Oct 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mximp commented Oct 21, 2022

yegor256 commented Oct 22, 2022 • edited Loading

volodya-lombrozo commented Oct 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yegor256 commented Oct 23, 2022 • edited Loading

volodya-lombrozo commented Oct 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yegor256 commented Oct 23, 2022

volodya-lombrozo commented Oct 25, 2022

volodya-lombrozo commented Oct 31, 2022

Updates for that PR

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yegor256 commented Nov 1, 2022

volodya-lombrozo commented Nov 1, 2022

yegor256 commented Nov 1, 2022

rultor commented Nov 1, 2022

rultor commented Nov 1, 2022

volodya-lombrozo commented Oct 20, 2022 •

edited

Loading

volodya-lombrozo commented Oct 21, 2022 •

edited

Loading

mximp Oct 21, 2022 •

edited

Loading

volodya-lombrozo Oct 21, 2022 •

edited

Loading

yegor256 commented Oct 22, 2022 •

edited

Loading

yegor256 commented Oct 23, 2022 •

edited

Loading