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

[meta] Blog: technical changes to PP source code #1914

Open
buchen opened this issue Dec 28, 2020 · 49 comments
Open

[meta] Blog: technical changes to PP source code #1914

buchen opened this issue Dec 28, 2020 · 49 comments

Comments

@buchen
Copy link
Member

buchen commented Dec 28, 2020

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 buchen pinned this issue Dec 28, 2020
@buchen
Copy link
Member Author

buchen commented Dec 28, 2020

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.

@chrisaut
Copy link

chrisaut commented Dec 28, 2020

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 :-(

@buchen
Copy link
Member Author

buchen commented Dec 28, 2020

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

@chrisaut
Copy link

chrisaut commented Dec 28, 2020

  • 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.

@buchen
Copy link
Member Author

buchen commented Dec 28, 2020

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 #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.

@chrisaut
Copy link

image

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

@buchen
Copy link
Member Author

buchen commented Dec 28, 2020

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?

@chrisaut
Copy link

chrisaut commented Dec 28, 2020

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

@inv-trad
Copy link
Contributor

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

@damarvin
Copy link
Contributor

damarvin commented Dec 28, 2020

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.

@chrisaut
Copy link

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?

@buchen
Copy link
Member Author

buchen commented Dec 28, 2020

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).

@damarvin
Copy link
Contributor

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?

@buchen
Copy link
Member Author

buchen commented Dec 30, 2020

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?

@damarvin
Copy link
Contributor

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: quarkusio/quarkus#6335 :(
So I raised https://issues.apache.org/jira/browse/MENFORCER-370.
And thank You for the -Denforcer.skip=true hint.

@damarvin
Copy link
Contributor

damarvin commented Dec 31, 2020

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.

@buchen
Copy link
Member Author

buchen commented Jan 3, 2021

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

@buchen
Copy link
Member Author

buchen commented May 1, 2021

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
Copy link
Member Author

buchen commented Aug 1, 2021

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
Copy link
Member Author

buchen commented Sep 25, 2021

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
Copy link
Member Author

buchen commented Apr 23, 2022

With commit 9f695b3 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

@Nirus2000
Copy link
Member

Nice 👍🏻
I have tested once...
grafik

@tquellenberg
Copy link
Contributor

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

Looks fine with MacOS and Wndows 10.

@buchen
Copy link
Member Author

buchen commented Dec 18, 2022

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

@Nirus2000
Copy link
Member

Nirus2000 commented Jan 2, 2023

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

@buchen
Copy link
Member Author

buchen commented Jan 2, 2023

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"

@Nirus2000
Copy link
Member

Nirus2000 commented Jan 2, 2023

There are no embedded projects in the workspace...

grafik

Edit:
Fix with ffcb20d

@buchen
Copy link
Member Author

buchen commented Apr 8, 2023

FYI: With f663328, 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
Copy link
Member Author

buchen commented Jun 1, 2023

@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?

Bildschirmaufnahme.2023-06-01.um.17.27.38.mp4

@OnkelDok
Copy link
Member

OnkelDok commented Jun 1, 2023

Can one of you test how this behaves on Windows?

Here a video of my test:

2023-06-01.19-57-55.mp4

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.

@buchen
Copy link
Member Author

buchen commented Jun 3, 2023

Thanks @OnkelDok for testing. I'll try to fix the style in dark mode.

The delay is necessary because the mouse move listener is attached to multiple widgets (the chart, the plot area, the title, the composite). If I do not add the delay, it would hide the button when moving from the plot to the chart - and that results in flickering.

@buchen
Copy link
Member Author

buchen commented Jun 5, 2023

Eclipse Core has been updated to 4.27 (March 2023) version.

Same procedure as every time:

  • open target platform in target platform editor, let it load (takes time), click "reload target platform" on the top right
  • open the launch configuration view, right-click "re-generate Eclipse launch configuration"

@buchen
Copy link
Member Author

buchen commented Jun 16, 2023

I have updated to the 4.28 version (release 14 June 2023).

Same procedure as every time:

  • open target platform in target platform editor, let it load (takes time), click "reload target platform" on the top right
  • open the launch configuration view, right-click "re-generate Eclipse launch configuration"

I also updated my Eclipse installation to 2023-06 version to match the platform.

When running the unit tests, I get this error message. When clicking "continue" the tests run fine. The mentioned plugins are about Junit5 (but PP is using Junit4). I have no idea (yet) how to get rid of this error dialog other than adding all plugins of the target platform to the launch configuration.

Bildschirmfoto 2023-06-16 um 09 52 35

@buchen
Copy link
Member Author

buchen commented Jul 22, 2023

Please checkout #3458 about transferring the repository to the portfolio-performance organization:

After 11 years, the project has outgrown my personal GitHub account. Transferring it to an organization allows better management of collaborators, projects, issues, help pages, etc. Moreover, it offers a fresh start for the list of open issues, resolving past mismanagement.

@buchen
Copy link
Member Author

buchen commented Jul 29, 2023

The repository has been transferred (noticed the new URL in your browser). I updated my local clone by running

git remote set-url origin https://github.com/portfolio-performance/portfolio.git

Depending on your setup, you'll need to replace the remote name with the one you use in your copy.

@buchen
Copy link
Member Author

buchen commented Aug 1, 2023

FYI, I have updated the Orbit dependencies (from the March 2022 to the May 2023 repository) with commit e30de80

Same procedure as every time:

  • open target platform in target platform editor, let it load (takes time), click "reload target platform" on the top right
  • open the launch configuration view, right-click "re-generate Eclipse launch configuration"

@buchen
Copy link
Member Author

buchen commented Aug 5, 2023

With merge #3487, PP is using Tycho 4.x.
To build locally with Maven, you need Maven 3.9.x

@buchen
Copy link
Member Author

buchen commented Sep 15, 2023

I have updated to the 4.29 Eclipse version (release 3 September 2023).

Same procedure as every time:

  • open target platform in target platform editor, let it load (takes time), click "reload target platform" on the top right
  • open the launch configuration view, right-click "re-generate Eclipse launch configuration"

@buchen
Copy link
Member Author

buchen commented Sep 15, 2023

Out of an forum conversation with @OnkelDok, I have create a new tag to categorize issues: [meta]. The idea is to use for all meta discussions (how do we want to work? How to prioritize certain changes? Does it make sense to implement something like this?) which are not on a level of a bug or concrete feature.

@buchen buchen added the [meta] label Dec 14, 2023
@buchen
Copy link
Member Author

buchen commented Dec 14, 2023

FYI, with this change 5aac558 the target platform has been updated. You need to reload the target platform and re-generate the launch configurations.

I plan to push the update to Eclipse 4.30 soon (which then will require the same procedures)

@buchen
Copy link
Member Author

buchen commented Dec 15, 2023

With commit 6a10792 the target platform has been updated to Eclipse 4.30.
I also changed the definition to use the new Orbit repositories for (some of the) 3rd party libraries. The new structure should make it easier to keep the dependencies up-to-date and in sync with the Eclipse release train.

As always, you need to reload the target platform and re-generate the launch configurations.

@OnkelDok
Copy link
Member

OnkelDok commented Feb 7, 2024

@buchen: Long time since I compiled and started PP via eclipse.
Today I had a try and it does not work. I updated Eclipse to 2023-12 (4.30.0), Reloaded Target Platform and re-generated the launch configuration.
I see a lot of errors in the Error Log:
grafik

When I try to run I get this message:
grafik

And when I Continue then PP is started, but GUI is corrupt:
grafik
And a lot of errors are logged into console. Here some of their messages:

  • org.osgi.framework.BundleException: Could not resolve module: org.eclipse.ecf.provider.filetransfer.httpclient5 [3267]
  • org.osgi.framework.BundleException: Could not resolve module: org.eclipse.ecf.provider.filetransfer.httpclient5.win32 [3268]
  • org.osgi.framework.BundleException: Could not resolve module: name.abuchen.portfolio [4327]
  • org.osgi.framework.BundleException: Could not resolve module: name.abuchen.portfolio.ui [4329]
  • java.io.IOException: Unable to resolve plug-in "platform:/meta/name.abuchen.portfolio.ui/custom.css"

Any idea what I can do to get this working? Thanks a lot.

@buchen
Copy link
Member Author

buchen commented Feb 21, 2024

Pinging contributors here: I don’t know if you noticed, but I finally published the mobile app. Check out this link with some screenshots: https://www.portfolio-performance.app/

As it turns out, if you only use a noreply email in the commit messages, I cannot reach out to you directly. I would like to give free one year subscriptions to anybody who has contributed code, translation, documentations to the open source (desktop) project. Please drop me an email with your user, your device (iPhone, Android) and the user id from the diagnostics page within the app.

@buchen
Copy link
Member Author

buchen commented Feb 21, 2024

@OnkelDok

The first screenshots are error from the Eclipse installation itself. To be honest, it reads a little like the installation itself is corrupt. If JDT (Java Development Tools) reports issues, then this is not good. Maybe consider a fresh install?

When I try to run I get this message:
Missing constaint: Require-Capability: osgi.serviceloader; (filter=org.slf4j.spi.SLF4JServiceProvider)'

To my understanding, the bundle slf4j.simple should provide this capability. Of course, the generated launch configuration should select this plug-in, but can you check your run configuration.

I think the rest can then easily be follow-up errors - if one bundle is not activated, then some of the other are not either. And then the UI bundle is not active (only the bootstrap bundle which includes the menu).

@buchen
Copy link
Member Author

buchen commented Sep 7, 2024

I have added aggregate rows (header and/or footer) to the Performance -> Securities view. The aggregate rows show the sums of relevant columns (i.e. where a sum of values makes sense). If you can test and provide feedback, I'd be thankful.

@pfalcon
Copy link
Contributor

pfalcon commented Sep 8, 2024

I have added aggregate rows (header and/or footer) to the Performance -> Securities view.

Looks good!

@buchen
Copy link
Member Author

buchen commented Oct 19, 2024

FYI, the Java21 branch is merged.

Java21 is the current long-term support version of Java. With the change, I have also updated the Eclipse Framework to the 2024-09 version (Eclipse 4.33). Eclipse itself requires Java 21 as well.

Make sure you have a Java 21 JDK installed locally for development. Personally, I am using the Azul distribution because that is also the embedded Java Runtime (= the distribution of PP that includes a Java runtime).

If run into issues, please let me know.

@mierin12
Copy link
Contributor

Hello,

With the change, I have also updated the Eclipse Framework to the 2024-09 version (Eclipse 4.33).

Does this mean that we need to update Eclipse to this version ? I have Eclipse 2023-12 (4.30).

Same procedure as every time :

  • open target platform in target platform editor, let it load (takes time), click "reload target platform" on the top right
  • open the launch configuration view, right-click "re-generate Eclipse launch configuration"

Is this also applicable with this change ?

@Nirus2000
Copy link
Member

Yes and yes 👍🏻

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

No branches or pull requests

9 participants