portfolio icon indicating copy to clipboard operation
portfolio copied to clipboard

[meta] Blog: technical changes to PP source code

Open buchen opened this issue 4 years ago • 47 comments
trafficstars

As more people are contributing to the code base of PP, I will use this issue to "blog" about technical changes to the source code. It is not meant to discuss features, but library upgrades, changes to the target platform, etc.

buchen avatar Dec 28 '20 09:12 buchen

Today's changes update the master branch to the 2020-12 release of Eclipse (4.18). Make sure to open and re-set the target platform. And you need Java11

Here are some things to note:

  • This version includes all macOS Big Sur fixes and should re-enable the auto update on the macOS platform.
  • Eclipse now requires Java 11. I have changes the environment to JavaSE-11 in the Manifest files accordingly. However, because I want to be able to downport the name.abuchen.portfolio to the 32bit branch, this bundle remains on Java8
  • With Eclipse 2020-12, we finally have typed Eclipse Databinding API. That allows me to remove a lot of restriction annotations. At the time of writing, the "Problems" view is finally empty again (the "other" category contains SonarLink and Inifitest warnings which I typically only fix if I happen to touch the source code) Bildschirmfoto 2020-12-28 um 10 20 43

A couple of more things are outstanding. For example, the themes have changed and need fixing. If you notice other changes, please let me know.

buchen avatar Dec 28 '20 09:12 buchen

Hi @buchen

Good idea with this mini-blog. Unfortunately with latest master I get tons of errors:

image

Perhaps I just need to update imports somehow, but I honestly don't know how :-(

chrisaut avatar Dec 28 '20 10:12 chrisaut

Unfortunately with latest master I get tons of errors

🤔 I would start looking at three things:

  • First: did the new target platform properly resolve? Open portfolio-target-definition/portfolio-target-definition.target and reload (top left button in the editor), if necessary.
  • Second: Is the right Java library on the path? Right-click on project -> Properties -> Java Build Path -> Libraries (Tab)
  • Third: Is Java11 the default JRE? Open the Preferences -> Java -> Installed JREs -> checkbox next to Java11 ticked

buchen avatar Dec 28 '20 10:12 buchen

  • First: did the new target platform properly resolve? Open portfolio-target-definition/portfolio-target-definition.target and reload (top left button in the editor), if necessary.

Ah ok, yes that fixed a ton of issues, only one remains: image

  • Second: Is the right Java library on the path? Right-click on project -> Properties -> Java Build Path -> Libraries (Tab) image

It says JavaSE-1.8 I guess? Should it be 11?

  • Third: Is Java11 the default JRE? Open the Preferences -> Java -> Installed JREs -> checkbox next to Java11 ticked

Hmm...if I can decypher this correctly I have Java 15? image

Edit: If under Lauch Configurations I start Portfolio Performance(launches.lc) it does start. If I instead try Portfolio Performance Java 11 .... it errors out with:

!ENTRY org.eclipse.osgi 4 0 2020-12-28 12:04:26.009
!MESSAGE Application error
!STACK 1
java.lang.NoClassDefFoundError: org/eclipse/swt/widgets/Display
	at org.eclipse.e4.ui.internal.workbench.swt.E4Application.start(E4Application.java:148)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:64)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:653)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:590)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1461)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1434)
Caused by: java.lang.ClassNotFoundException: org.eclipse.swt.widgets.Display cannot be found by org.eclipse.e4.ui.workbench.swt_0.15.0.v20201103-0952
	at org.eclipse.osgi.internal.loader.BundleLoader.findClass(BundleLoader.java:516)
	at org.eclipse.osgi.internal.loader.ModuleClassLoader.loadClass(ModuleClassLoader.java:171)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:522)
	... 14 more

So I guess this ant thing is only related to tests, but this launch indicates I'm missing Java11 or something. Although one would think if I have a higher version it would be fine.

I apologize for the noise.

chrisaut avatar Dec 28 '20 11:12 chrisaut

If I instead try Portfolio Performance Java 11 .... it errors out with

The problem is: this launch configuration is specific to my macOS platform. You would have to "fix it" for your Windows platform (open the Launch Configuration and on the "Plug-ins" tab select "Add required Plug-ins").

This is actually the reason for https://github.com/buchen/portfolio/issues/1913 - it should create launch configurations independent of the platform.

Alas, that is where the missing org.eclipse.ant.core.antRunner is reported. If I open the launches.lc file, what applications do you get? (Press Ctrl-Space for suggestions) Bildschirmfoto 2020-12-28 um 16 02 13

Your Java Version looks fine.

buchen avatar Dec 28 '20 15:12 buchen

image

Perhaps rename it "... (MacOS)" then? Or are you saying with #1913 this should also work for windows?

chrisaut avatar Dec 28 '20 15:12 chrisaut

Or are you saying with #1913 this should also work for windows?

Yes, that should work for Windows too (as it is dynamically resolving the dependencies).

But first we need to get it working. Very strange that offers antRunner as a proposal, but then errors on it. Which Eclipse version are you running?

buchen avatar Dec 28 '20 15:12 buchen

Which Eclipse version are you running?

Eclipse IDE for Java Developers (includes Incubating components) Version: 2020-12 (4.18.0) Build id: 20201210-1552 OS: Windows 10, v.10.0, x86_64 / win32 Java version: 15.0.1

image

chrisaut avatar Dec 28 '20 16:12 chrisaut

After removing antRunner, typing Ctrl-Space to get suggestions and selecting antRunner once again, the error disappears in my installation.

inv-trad avatar Dec 28 '20 17:12 inv-trad

Hi, And yes, there should be a channel for fast syncing on issues (maybe in addition to "Blog: technical changes..."). I would like to look into the (more general) csv import, and was wondering how to build PP (on Windows), preferably with mvn and Java11 only ⇒ @buchen Is the README.md still correct? Travis is replaced? By Github Actions? Where is the exe built, I only see rm ../../portfolio-product/target/products/*.exe in build_installer.sh.

damarvin avatar Dec 28 '20 17:12 damarvin

After removing antRunner, typing Ctrl-Space to get suggestions and selecting antRunner once again, the error disappears in my installation.

Lol, amazingly this works. Seems like a bug with Ecplise?

chrisaut avatar Dec 28 '20 18:12 chrisaut

I [...] was wondering how to build PP (on Windows), preferably with mvn and Java11 only ⇒ @buchen Is the README.md still correct? Travis is replaced? By Github Actions?

You can build PP fully with mvn only. But I assume you might not be able to build the Linux and macOS versions on Windows. But other than that, I would think you can build the full distribution on Windows as well.

To check the build, I run from the command line

mvn clean verify

This will not build all products - in particular not the "distributions" which includes the Java Runtime. To do so, run:

mvn clean verify -Ppackage-distro

You should find all ZIPs in the portfolio-product/target/products folder.

And, yes, I moved to Github Actions a while ago. I fixed the README file.

If you check the workflow definition, I am running three builds

  • mvn verify (if a pull request is build)
  • mvn verify sonar:sonar (on the master branch - to report to Sonarqube)
  • mvn verify -pl :portfolio-target-definition,:name.abuchen.portfolio,:name.abuchen.portfolio.tests -am -amd (to build and test only the name.abuchen.portfolio bundle with Java8)

There are a couple other flags in the build - which I use locally to sign the build result: with a Java Code Signing certificate and an Apple developer certificate.

build_installer.sh

I am not using this file at the moment. The idea was to have an EXE installer instead of a ZIP on Windows. However, I had troubles with the administration mode and the updates. I could try this again, though. At the moment, the code is checking if the user has write permissions to the install directly and asks the user to run PP "as administrator" if not (only for the update, of course).

buchen avatar Dec 28 '20 19:12 buchen

Thanks a lot! This works nicely in Ubuntu, and I can start the built exe in Windows.

But running mvn under Windows does not get over the <evaluateBeanshell/> <condition/> with ${project.basedir}, as it gives the Windows path with raw backslashes and fails before you have a chance to replace them with doubled ones. @buchen Not sure, what you are testing there, but for maven builds in Windows this check needs to be off, or be fixed by a correct Windows paths evaluation, or...? Is there something else of https://maven.apache.org/enforcer/enforcer-rules/index.html You could use?

damarvin avatar Dec 29 '20 23:12 damarvin

Not sure, what you are testing there, but for maven builds in Windows this check needs to be off, or be fixed by a correct Windows paths evaluation, or...?

@damarvin Well, obviously I am not testing the build on Windows... 🙃 Every once in a while I copy the Windows build result to a virtual machine and run PP there. I just assumed the build runs on Windows as well...

You could try running it with -Denforcer.skip=true to skip that section. Or you can try if we can fix the bean shell condition to something like java.nio.file.Paths.get("${project.basedir}/".replace("\\", "/"),. If that works, can you create a pull request?

buchen avatar Dec 30 '20 18:12 buchen

Unfortunately this seems to be a bug in apache/maven-enforcer. I only found https://issues.apache.org/jira/browse/MENFORCER-100, which was auto-closed, or https://stackoverflow.com/questions/63263586/maven-enforcer-how-to-access-maven-properties-from-beanshell-rule neither with a solution so far.

Already

    <evaluateBeanshell>
        <condition>
            print(java.nio.file.Paths.get("${project.basedir}/".replace("\\", "/"));
            1==1
        </condition>
    </evaluateBeanshell>

fails with Couldn't evaluate condition... as ${project.basedir} is seamingly taken as a Java string directly, w/o escaping the backslashes. While the message part

    <evaluateBeanshell>
        <condition>
            1==0
        </condition>
        <message>condition failed, project.basedir=${project.basedir}</message>
    </evaluateBeanshell>

logs the Windows path correctly:

    [WARNING] Rule 2: org.apache.maven.plugins.enforcer.EvaluateBeanshell failed with message:
    condition failed, project.basedir=C:\Users\me\git\portfolio\portfolio-app

Others gave up here as well: https://github.com/quarkusio/quarkus/pull/6335 :( So I raised https://issues.apache.org/jira/browse/MENFORCER-370. And thank You for the -Denforcer.skip=true hint.

damarvin avatar Dec 31 '20 13:12 damarvin

Just to inform, in Windows, with JAVA_HOME pointing to an 11.0.9.1 OJDK installation: C:\Users\me\git\portfolio>mvn -f portfolio-app\pom.xml -Denforcer.skip=true clean verify -Ppackage-distro works well. Built PortfolioPerformance.exe runs fine.

@buchen You might want to update the main readme.md.

damarvin avatar Dec 31 '20 14:12 damarvin

FYI, with #1932, the precision of shares and quotes would be added to 8 digits.

buchen avatar Jan 03 '21 10:01 buchen

With the latest commit, I have refactored the "information pane" in the views - the lower part where the historical prices, transactions, security chart, etc. are shown. I wanted to make the detail pages re-usable across multiple views. The area can be hidden with Strl-L (or Command-L on macOS).

For example, if you click in the holdings chart on one of the pies, details for the security are to be shown. The same is available in the statement of assets view now.

If you sync your local project to the latest commit, please give it a go and test it. I had to shuffle quite some code around which makes it easy to introduce bugs.

buchen avatar May 01 '21 08:05 buchen

Have a look at #2363 - I have drafted an implementation using protocol buffers to the store the data in a binary format. I knew XML was slow, but I did not expect it to be that slow.

buchen avatar Aug 01 '21 17:08 buchen

PP dependencies are now updated to Eclipse 2021-09 release. This allows us to support the Apple Silicon M1 build. You have to update the target platform in your Eclipse installation

buchen avatar Sep 25 '21 08:09 buchen

With commit 9f695b36474642fb2408bcffa5f5e3b7cd5b7714 I have updated to Eclipse 2022-03.

The basic steps to update your workspace:

  • open the target platform and "reload"
  • re-generate the LCDSL run configurations
  • and (if you build not within Eclipse but with Maven from the CLI), clear the caches: ~/.m2/repository/.cache and ~/.m2/repository/p2

Please let me know if you run into problems. There are new versions of libraries and I had to adopt the Apache HTTP component which is used for downloading

buchen avatar Apr 23 '22 17:04 buchen

Nice 👍🏻 I have tested once... grafik

Nirus2000 avatar Apr 24 '22 06:04 Nirus2000

For me this warnings disappeared after closing and reopening the "portfolio-app" project in eclipse.

Looks fine with MacOS and Wndows 10.

tquellenberg avatar Apr 24 '22 09:04 tquellenberg

I have created a new branch feature_java17 move PP to Java17. It includes updates to Java17, Eclipse 2022-12, and Tycho 3.0.1.

The two big changes are:

  • the target platform can now contain references to Maven artifacts directly --> therefore the separate portfolio-deps projects is not necessary. There are 2 dependencies why we still need our own Maven repository but they hardly ever change (the old SWTChart library and the TreeMap SWT implementation)
  • I had to refactor the code and move some Test helper classes to its own bundle. When updating, you need to add this new project to your workspace

buchen avatar Dec 18 '22 19:12 buchen

Hello @buchen, after the update to build id: 20221201-1913 did not work, I reinstalled Eclipse on my end. (JDK17). After installation, "Reload Target Platform" and so on, I can't get these errors to go away.

Eclipse Log.txt

Nirus2000 avatar Jan 02 '23 17:01 Nirus2000

The error message says:

Der Import name.abuchen.portfolio.junit kann nicht aufgelöst werden

That is exactly the new project I created in order to refactor the test classes. It was necessary, because otherwise the name.abuchen.portfolio bundle is re-exporting test classes which led to errors.

Select in the menu "File" -> "Import..." -> "Existing projects into workspace" -> navigate to your local directory where you have the git checkout -> pick "name.abuchen.portfolio.junit"

buchen avatar Jan 02 '23 18:01 buchen

There are no embedded projects in the workspace...

grafik

Edit: Fix with ffcb20d02e8631d4f21b9b2acc3d0f6b6428d95f

Nirus2000 avatar Jan 02 '23 18:01 Nirus2000

FYI: With f66332813f4e9e0f141afbc1058e687b0298a895, the google Protobuf library has been updated.

Make sure you update your project configuration by

  • reloading the target platform (open the file, let it resolve, then click "reload" at the top right corner)
  • rebuild the run configuration (open the launch configuration and right-click "re-generate launch configuration")

buchen avatar Apr 08 '23 09:04 buchen

@OnkelDok @Nirus2000 - other window testers:

I have extended the dashboard to show a "jump to view" button only if the mouse hovers over the chart. See the short video below. It is only in the latest commit on master and (for now) only for the new dividend charts (add widgets -> earnings -> earnings per month/year/quarter).

Can one of you test how this behaves on Windows?

https://github.com/buchen/portfolio/assets/587976/248b6d74-4bd5-4f6c-abdd-083a37f7cce9

buchen avatar Jun 01 '23 15:06 buchen

Can one of you test how this behaves on Windows?

Here a video of my test:

https://github.com/buchen/portfolio/assets/90478568/9280f225-6421-427a-844c-d1b0e39ed74e

The style in darkmode is not the same as that of the permanent button of the performance diagram. And for me the delay is not necessary. The first time I tested the function, it felt a little strange when there is no delay on hover start and delay after mouse leaves the chart.

OnkelDok avatar Jun 01 '23 18:06 OnkelDok