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

Update Eclipse project files #8621

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Pieter12345
Copy link
Contributor

@Pieter12345 Pieter12345 commented Mar 8, 2019

This PR adds missing build path entries to the Java Eclipse IDE specific .classpath files, effectively resolving all compile errors in the project. An outdated license file for jmdns was also updated in the process.
The changes in this PR only affect people who use Java Eclipse to build this project.

This resolves issue #5459.

@Pieter12345
Copy link
Contributor Author

Pieter12345 commented Mar 18, 2019

@facchinm The requested change was made and the PR is up-to-date with master again. Is any additional action required to get this merged?
Edit: I've rebased this onto master, rather than merging master into this branch.

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from 7fa1bb4 to 1e744bd Compare March 18, 2019 16:27
facchinm
facchinm previously approved these changes Mar 20, 2019
@facchinm facchinm added this to the Release 1.8.10 milestone Mar 20, 2019
@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from 1e744bd to 52f1c4b Compare March 22, 2019 05:15
@Pieter12345
Copy link
Contributor Author

Force pushed to c00e50a to also remove references to that removed library from the class path in the launcher config.xml and config_debug.xml files.

@Pieter12345
Copy link
Contributor Author

Pieter12345 commented Mar 22, 2019

These additional changes make the project more clean, but also allow running the Arduino IDE from Java Eclipse on Windows. To do so, create a run configuration for main class processing.app.Base with the following VM arguments:

-splash:"${workspace_loc:Arduino}/build/shared/lib/splash.png"
-Dsun.java2d.d3d=false
-Djna.nosys=true
-Djna.nounpack=true
-Djna.boot.library.name=jnidispatch-4.2.2-win32-x86
-Djna.boot.library.path="${workspace_loc:Arduino}/build/windows/work/lib"
-Djssc.library.path="${workspace_loc:Arduino}/build/windows/work/lib"
-DAPP_DIR="${workspace_loc:Arduino}/build/windows/work"

Note: To be able to run the project from Eclipse, ant build/run/dist has to be executed first to extract 3 required .dll files from library jars. This is also why the Eclipse build configuration options contain build/windows/work. If these .dll files were manually extracted, the project would be able to run without use of Ant, which allows developers to make and test changes without installing any additional software (other than a 32 bits JDK). The downside of this would be that adding these .dll files also adds redundancy. Note that on OSX and Unix, this might already be possible since they do not depend on these .dll files.

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from a9f9d8f to 964c189 Compare March 22, 2019 06:14
@Pieter12345
Copy link
Contributor Author

Force pushed to revert linking to my personal 32 bits JVM by name in the .classpath file.

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from 964c189 to 631ceab Compare March 29, 2019 00:57
@Pieter12345
Copy link
Contributor Author

Rebased this PR onto master.

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from 631ceab to 6ed443e Compare April 4, 2019 14:16
@Pieter12345
Copy link
Contributor Author

Rebased this PR onto master to resolve merge conflicts.

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from 9ee234c to ee3c37b Compare April 4, 2019 14:54
@Pieter12345
Copy link
Contributor Author

In the current PR state, the following setup is possible in Eclipse by importing Arduino (my clone name), Arduino/app and Arduino/arduino-core into Eclipse:
afbeelding

Note that since parent project Arduino has the same source and binary directory as Arduino/app, it is not necessary to import Arduino/app as separate project. It is possible to remove the Arduino project setup so that it becomes just a container for the 2 child projects (or modules), I'd like to have your opinions on this @facchinm and @cmaglie.

@facchinm
Copy link
Member

Hi @Pieter12345 ,
I just tried the complete patchset; there's a problem with 6ed443e (in fact it breaks ant build since the ant projects are no more self contained). A trivial patch is to reference the other folder in the lib search path, like this

diff --git a/arduino-core/build.xml b/arduino-core/build.xml
index cef9fe404..4ddbaea94 100644
--- a/arduino-core/build.xml
+++ b/arduino-core/build.xml
@@ -5,6 +5,9 @@
     <fileset dir="lib">
       <include name="*.jar"/>
     </fileset>
+    <fileset dir="../app/lib">
+      <include name="*.jar"/>
+    </fileset>
     <pathelement path="${env.JAVA_HOME}/lib/tools.jar"/>
   </path>
diff --git a/app/build.xml b/app/build.xml
index cc38670ad..08b57ba31 100644
--- a/app/build.xml
+++ b/app/build.xml
@@ -2,6 +2,9 @@
 <project name="Arduino PDE" default="build">
 
   <path id="class.path">
+    <fileset dir="../arduino-core/lib">
+      <include name="*.jar"/>
+    </fileset>
     <fileset dir="lib">
       <include name="*.jar"/>
     </fileset>

but I'm not a big fan of this approach. Probably unifying the lib folder and have it at the top level would work (not very Java-ish). @cmaglie any hint?

Anyway, I also haven't been able to setup the eclipse project to run; it looks like APP_DIR variable is not being applied, so all files are searched under app/lib/.
Did you find something similar?

@cmaglie
Copy link
Member

cmaglie commented Apr 18, 2019

It is possible to remove the Arduino project setup so that it becomes just a container for the 2 child projects (or modules)

I've successfully setup eclipse without the "parent" Arduino project, so I'm positive to removing it.

but I'm not a big fan of this approach. Probably unifying the lib folder and have it at the top level would work (not very Java-ish). @cmaglie any hint?

+1 to this too, but we should check launchers configuration for the "dist" target.

@facchinm facchinm dismissed their stale review April 18, 2019 10:23

Need to fix ant build before going on

- Replace the HTTP404 license link by a link to the jmdns GitHub repository link (https://github.com/jmdns/jmdns/blob/jmdns-3.5.3/LICENSE).
- Remove the LGPL-2.1 license since the repository only mentions ASL-2.0.
- Add missing dependency references to Eclipse's build path.
- Fix broken relative dependency reference in Eclipse's build path.
Remove commons-httpclient-3.1 dependency since it is not used.
`app/lib` and `arduino-core/lib` contained a different version of `apple.jar`. This copies the latest version (`app/lib/apple.jar`) to `arduino-core/lib/apple.jar`.
- Sync Eclipse classpath file (`.classpath`) by adding exactly all entries that are in the launcher's classpath configuration (`build/windows/launcher/config.xml`).
- Refer to `arduino-core/lib` where possible to avoid having duplicate jars on the classpath (this fixes problems when running Arduino from Eclipse and prepares for having a project without duplicate dependencies).
- Sort the Eclipse classpath files alphabetically (This greatly helps when comparing them).
This way, it will be next to the mxl-apis jar in an alphabetically sorted file system.
Remove libraries from `app/lib` that are already present in `arduino-core/lib`. This removes redundancy and makes it trivial which `.jar` file is actually used.
Remove exclude file entries that do not exist in the project.
Update `app/.classpath` so that it is consistent with `/.classpath`. This file is used when setting up `app` as a module of the main project (which is optional, since the main project and the `app` module have the same source and bin directories).
Remove Eclipse project files that allow a user to setup the project container as the `app` project. The proper way to setup the Arduino IDE (before and after this commit) is to import both the `app` and `arduino-core` project in Eclipse.
@Pieter12345
Copy link
Contributor Author

I just tried the complete patchset; there's a problem with 6ed443e (in fact it breaks ant build since the ant projects are no more self contained). A trivial patch is to reference the other folder in the lib search path, but I'm not a big fan of this approach. Probably unifying the lib folder and have it at the top level would work (not very Java-ish). @cmaglie any hint?

I've tried putting a lib directory in the base directory, but there appears to be no way to link there other than making this lib a separate project as well (the reason it is possible to link to /arduino-core/lib/... from app is that arduino-core is a known project in the workspace). Ideally, I would like to see Apache Ant download and link dependencies like most Maven or Gradle projects do (Maven or Gradle actually replace the need for also having dependencies in the .classpath file, making it impossible to have mismatching dependency versions in config files). I have never worked with Ant before, so support for this in Ant is a gray area for me though.
As for fixing this issue for this PR without fixing the above mentioned technical debt, the two easiest solutions I can come up with are the one you mentioned and introducing duplicate jars again (have the same jar in both projects if both projects use it). I would be in favor of the first option, since the second allows more space for errors and non-deterministic behavior (unknown which version of a dependency is picked when two versions are available by accident) in my opinion. Any thoughts on this in combination with better dependency management @facchinm / @cmaglie ?

Anyway, I also haven't been able to setup the eclipse project to run; it looks like APP_DIR variable is not being applied, so all files are searched under app/lib/.
Did you find something similar?

Perhaps try to set your working directory (under run configurations) to the work directory?

@Pieter12345 Pieter12345 force-pushed the eclipse-project-files branch from ee3c37b to bb8ea29 Compare April 19, 2019 17:54
@Pieter12345
Copy link
Contributor Author

Rebased onto master.

@facchinm
Copy link
Member

Perhaps try to set your working directory (under run configurations) to the work directory?

This may be in fact what In was missing 🙂

About the maven/gradle dependency manager I'm totally positive about adding it. The problem right now is really technical debt since we don't have a proper Java programmer. Any pull request is well accepted as usual, but this time it could be a bit cumbersome to provide test builds since the build environment would change too.

@Pieter12345
Copy link
Contributor Author

About the maven/gradle dependency manager I'm totally positive about adding it. The problem right now is really technical debt since we don't have a proper Java programmer. Any pull request is well accepted as usual, but this time it could be a bit cumbersome to provide test builds since the build environment would change too.

Maven would fully replace Apache Ant, which means that the entire Build.xml files would have to be converted to pom.xml Maven files in an entirely different format. Finding a translation for all functionality and then translating it will likely take quite some time, but would be good for the project (less software to install to start developing (only Maven, which comes with most IDE's) and no more IDE-dependent (.classpath / .project files for Eclipse) files).
This indeed does affect the build environment. I could not find much info on the build environment, but it looks like it is some task that builds the software every hour (whether it changed or not) and does never run tests. What build software is actually used and how is it configured (I did not see a CI setup file in the repository)?

@facchinm
Copy link
Member

The CI infrastructure is based on Jenkins and builds the IDE for every merged commit to compose the hourly build (crosscompiling with ant dist for every OS target). In parallel, some lower priority jobs build the IDE and run the tests on their native OS.

If we replace ant we need a tool that:

  • compiles jars 🙂
  • compresses/uncompresses archives
  • controls checksums
  • can invoke other jars (like appbundler)
  • invokes external binaries
  • runs tests

They all look simple (maybe appbundler is not so easy but there are a couple of projects on github that look ok).
@cmaglie what do you think about that?

@szb512
Copy link

szb512 commented Apr 28, 2019

revapi/testjars can compile jar files

- Access `programmerPid` statically, since it's a static field.
- Add missing override tag.
This duplicates dependency jars that are used by both `app` and `arduino-core` from `arduino-core/lib` into `app/lib`. Reasoning is that the module/project-specific Ant build files point only to the `lib` directory of its project, leading to missing dependencies. Pointing Ant to both `app/lib` and `arduino-core/lib` would work, but include more dependencies than desired to the classpath.
This solution should be seen as temporary since the build system should eventually download dependencies from some repository (Maven / Gradle).
@Pieter12345
Copy link
Contributor Author

Need to fix ant build before going on

I've added a copy of the jars that are used in both app and arduino-core to app/lib, which should fix the Ant build. This might not be the ideal solution, but it should be sufficient for the scope of this PR. Changing the build system to download jars instead of including them as binaries would be the proper fix, but this falls out of scope for this PR.

As of this point, this PR:

  • Fixes Eclipse project setup files (.classpath files and removal of the redundant parent container setup files).
  • Cleans up dependencies (remove unused, update license file).
  • Allows developers to run the Arduino IDE directly from Eclipse.

Can you validate that this change fixes the Ant build @facchinm? If so, is there anything left that prevents this PR from being merged?

@cmaglie cmaglie modified the milestones: Release 1.8.10, Release 1.8.11 Nov 8, 2019
@cmaglie cmaglie modified the milestones: Release 1.8.12, Next Feb 12, 2020
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants