#2351: Fixed GUI not launching with local dev build - #2352
Conversation
samuelkos17
added
GUI
Coverage Report for CI Build 32831392063Coverage increased (+0.03%) to 73.681%Details
Uncovered ChangesNo uncovered changes found. Coverage Regressions15 previously-covered lines in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
|
Hi, i checked fixes, they work, well done. The only thing i want to recommend is to do both steps (the build script and ide gui) inside the same ide shell. That way both use the same cache folder. Because maven keeps downloaded libraries in a local folder, and which folder it uses is decided by an environment variable (M2_REPO) that is only set inside an ide shell (via conf/mvn/settings.xml). So if the two steps happen in different shells, the fix can silently not work. Then ide gui doesn't find your local GUI and quietly falls back to the remote snapshot. The GUI opens anyway but so it looks like the fix works, but it's launching the wrong build. |
|
@krystynaShatkovska did you test that and experienced that problem or are you just saying that it could happen? If you did experience this, then the fix isn't really that useful |
|
Yes, I tested it. The fix works with a local install, ide gui launches the locally built GUI, not the remote one. About next, the two shell types really point Maven at two different folders (ide shell to conf/mvn/repository, plain shell to ~/.m2). If the build runs in a plain shell and ide gui in an ide shell, the two use different repos, so ide gui silently falls back to the remote GUI. So maybe you can think about adding just one line in the testing instructions: run both steps in the same ide shell, so a tester on a fresh shell doesn't get a false |
|
@krystynaShatkovska I couldn't reproduce your problem, could you please explain step by step how you've encountered this issue? I've run the |
|
@samuelkos17 I think we have just misunderstood each other. You tested matching shells, but the bug only shows when you build in one shell type and run ide gui in the other. Then it silently uses the remote snapshot, not your build. I will explain. There are two separate local Maven folders, and they don't automatically know about each other: Local build (build-local-dev.sh) does mvn install, which drops newly built GUI into whichever folder that shell uses: When it finds nothing, and because version is a -SNAPSHOT, the GUI launcher quietly adds -U (force-update) and just downloads the remote snapshot from the internet instead. So the GUI opens, but it s not your code. That's the bug. Maybe in your test you always built and launched in the same kind of shell (normal ->normal or IDE->IDE), which always agree on the folder. Could you try my case? How I proved it: |
|
Thanks again for the details, the problem was not the shells but that you had to be in the same project directory, otherwise it wouldn't work. This could've been merged anyways and documented, but I believe that wouldn't be right so I've reworked the gui handling in the local dev build: I've tested every combination we discussed and it always worked. |
|
Thank you for your rework. It is really a good fix. Making the local-dev install self-contained and launching offline removes the only path by which the remote Sonatype snapshot could be selected, so ide gui now works from any folder. I alsoverified 3 new GuiTest cases pass, and the two seed commands produce a self-contained .m2 that resolves the base -SNAPSHOT ide-gui jar offline with no remote snapshot present. Just two things that are not blocking but just some cosmetic: 1. the execute() catch error message still references the old "M2 repo enabled in your project" story and should point at the self-contained repo (In Gui.execute, the catch block still says "check whether the maven M2 repo is enabled in your project." That was the old M2_REPO story. Now, for a local-dev install, the realistic failure is an incomplete self-contained .m2 (like script interrupted before the seed steps). The message should point at the self-contained repo / re-running the build.). 2. and buildMvnArgs recomputes installationPath.resolve("gui/pom.xml") even though execute already computed pomPath and verified it. |
…ttps://github.com/samuelkos17/IDEasy into feature/2351-gui-not-launching-with-local-dev-build
|
@krystynaShatkovska thanks for the suggestions! I've added them and will move this to In Review now 😄 |
hohwille
left a comment
hohwille
left a comment
There was a problem hiding this comment.
@samuelkos17 thanks for this story and PR. You created an elaborate solution including the local repository that you populate during the build with all required artifacts. Great job 👍
My major suggestion for improvement is to still keep the current release version included in the local dev build as it is currently due to several aspects and effects I tried to motivate in my review comments.
In my last comment I suggested the easiest solution to finally address this.
Please have a look.
BTW: Why do you think this PR is urgently required as you said today? To launch and test the GUI I can just start it in Intellij. Do not get me wrong. This feature is great for testing changes of the GUI launcher logic itself under realistic conditions but such changes are IMHO very rare.
| private static final Logger LOG = LoggerFactory.getLogger(Gui.class); | ||
|
|
||
| /** The value of {@link IdeContext#FILE_SOFTWARE_VERSION} written by the local-dev build script to mark a local-dev installation. */ | ||
| private static final String LOCAL_DEV_VERSION = "local-dev-version"; |
There was a problem hiding this comment.
IMHO also the local dev version should be indicated as SNAPSHOT version like it is before merging this PR.
Otherwise IDEasy will think that it is an official release version and does not treat it as snapshot.
For SNAPSHOT versions e.g. ide status will warn me that I am currently not working on an official release.
This feature might still be helpful if I forgot to upgrade from a local dev version before I start reporting bugs that may not officially exist.
How about this?
| private static final String LOCAL_DEV_VERSION = "local-dev-version"; | |
| static final String LOCAL_DEV_VERSION = "local-dev-version"; |
BTW: Removing the private visibility allows access from the JUnit avoiding duplication.
There was a problem hiding this comment.
Another thought: Is it really required to have a static version for this?
This way we are loosing the information from what release version this build was created from.
Esp. in cases where I want to create an (internal) bug issue and include ide -p status this can be confusing if I look at the ticket after a long time and cannot relate it to a release.
We could just have a boolean method IdeVersion.isLocalDev() that can give us the information from matching the version format (endsWith("-SNAPSHOT") + no timestamp included).
There was a problem hiding this comment.
This is actually just a marker to identify if it's a local-dev-build, I didn't change anything about the naming of the local-dev-version (it's still identified as a SNAPSHOT version). ide -p status after running build-local-dev.sh:
Updates are available for the settings repository. If you want to apply the latest changes, call "ide update"
IDE_ROOT is set to ~/projects
IDE_HOME is set to ~/projects/project
Your version of IDEasy is 2026.08.002-SNAPSHOT.
You are using a SNAPSHOT version of IDEasy. For stability consider switching to a stable release via 'ide upgrade --mode=stable'
Your version of IDEasy is 2026.08.002-SNAPSHOT but version 2026.08.002-20260825.030842-7 is available. Please run the following command to upgrade to the latest version:
ide upgrade
Your operating system is windows(10.0)@x64 [Windows 11@amd64]
You are online.
Found bash executable at: C:/Program Files/Git/usr/bin/bash.***
Found git executable at: C:/Program Files/Git/mingw64/bin/git.***
Your settings are not up-to-date, please run 'ide update'.
Successfully completed ide (status)
| /** Version marker written into a local-dev installation by {@code build-local-dev.sh}. */ | ||
| private static final String LOCAL_DEV_VERSION = "local-dev-version"; |
There was a problem hiding this comment.
Now you can remove this and just reference Gui.LOCAL_DEV_VERSION.
| if (installationPath == null) { | ||
| return false; | ||
| } | ||
| Path versionFile = installationPath.resolve(IdeContext.FILE_SOFTWARE_VERSION); |
There was a problem hiding this comment.
We are currently reworking our way to determine the current installed edition and version of tools.
Maybe for several scenarios this file (IdeContext.FILE_SOFTWARE_VERSION) might even disapear.
Therefore it would be good to avoid adding more and more places that have knowledge about it.
IMHO this entire method could be simplified to this:
return IdeVersion.getVersionString().equals(Gui.LOCAL_DEV_VERSION);
BTW: The LOCAL_DEV_VERSION is IMHO not really a feature of Gui so I would consider it slightly misplaced there.
I would instead suggest to have this constant in IdeVersion and you could even provide a boolean isLocalDevVersion() method there just like isUndefined() and isSnapshot().
|
|
||
| note right of guiCmd | ||
| For a local-dev installation | ||
| (.ide.software.version = local-dev-version) |
There was a problem hiding this comment.
Spreading too deep implementation details makes documentation hard to maintain.
| (.ide.software.version = local-dev-version) |
4 participants
This PR fixes #2351
Implemented changes:
$[project.version]in the installedgui/pom.xmlcleanso stale build output from another branch/checkout can't leak into the local-dev installation.-f <pom>for both Maven invocations instead of cd, so the build is independent of the caller's working directory.Testing instructions
Please add conscise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
./build-local-dev.shide gui. The GUI should open.Checklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.batand notfeature/921 fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internalpom.xmlfiles or otherwise if runtime dependencies changed, you have updated our LICENSE.asciidoc