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

Standalone app doesn't work #2823

Closed
shaman-apprentice opened this issue May 24, 2022 · 15 comments · Fixed by #2881
Closed

Standalone app doesn't work #2823

shaman-apprentice opened this issue May 24, 2022 · 15 comments · Fixed by #2881
Assignees
Labels
bug Only issues that describe bugs. difficulty:medium The difficulty to solve this is not super complex but not easy either priority:urgent Should be addressed ASAP

Comments

@shaman-apprentice
Copy link
Contributor

shaman-apprentice commented May 24, 2022

Bug

Expected Behavior

Given a new version is released
When downloading the assets
Then we get executable for Windows, Mac and Linux.

Actual Behavior

See ci output of job "Build visualization" and available assets of release 1.96.0:

  • Windows version is not build due to missing wine in build image
  • Mac x64 is not build due to a bug in nwjs-builder-phoenix; maybe related to this bug and this bug
  • When I download Linux x64 version and want to run it in my Ubuntu 20.04 WSL I get the following error: (codecharta-visualization:293): Pango-ERROR **: 17:54:03.066: Harfbuzz version too old (1.2.1)

Open questions

  • Do we need any standalone version and if so do we need Windows, Mac and Linux? -> erstmal ja
  • For Linux: Is it the user responsibility to have the right version of Harfbuzz or out responsibility to use a not outdated version?
  • nwjs-builder-phoenix isn't maintained anymore. Do we want to migrate away from it?

Specifications

  • is released in online-demo: yes
  • CodeCharta Version: 1.96.0
  • OS: Windows, Mac, Linux
@shaman-apprentice shaman-apprentice added the bug Only issues that describe bugs. label May 24, 2022
@BridgeAR BridgeAR added the priority:urgent Should be addressed ASAP label May 31, 2022
@IhsenBouallegue
Copy link
Collaborator

If I understood correctly, we need to fix the standalone builds.
But in this issue it was mentioned that we need to remove them and keep the web version (which works on all platforms) #2573.
I don't see a lot of benefit from the windows linux and mac versions. It's a lot more maintainability for little benefit.

@BridgeAR
Copy link
Member

BridgeAR commented Jun 1, 2022

We might have different understandings when it comes to this: #2573 was about simplifying the build process for the standalone version. We used to have two types of builds: one for the web that caused different javascript code to be executed than in the standalone version. Now both builds are identical but we still have to be able to execute the standalone version properly.

So if I understood this issue correct it's only about not being able to properly start the standalone version at the moment and that should definitely be the case.

@IhsenBouallegue
Copy link
Collaborator

So if I understood this issue correct it's only about not being able to properly start the standalone version at the moment and that should definitely be the case.

I see. Thanks for the clarification @BridgeAR.
And why do we have other standalone versions than web in the first place? (Mac, Windows, Linux…) are they still relevant? Shouldn't be web enough for all platforms?

@BridgeAR
Copy link
Member

BridgeAR commented Jun 1, 2022

We require a desktop application that runs on any OS. The reason is mainly that Internet is not always available.

@IhsenBouallegue
Copy link
Collaborator

IhsenBouallegue commented Jun 1, 2022

But the web build doesn't require internet. And it works on all operating systems without any additional dependencies. Doesn't that already meet the requirements? Plus it's already available for every version we release link
grafik

@shaman-apprentice
Copy link
Contributor Author

That's new to me, that the browser loads and executes JavaScript from file system, when opened from file system. It also works on my machine.

I am not sure, if every request to the internet is enforced to be blocked. Is this relevant for a user, that he is 100% that no request is sent? On the other side a standalone app could also sent requests, couldn't it?

If this approach is fine, it would save same maintenance for sure :)

@BridgeAR
Copy link
Member

BridgeAR commented Jun 2, 2022

It is correct, that it works by loading the main file in a browser. Some computers that CC should run upon might not have a browser though. We just have to make sure we need nothing on a computer than CC to be run properly. What tool is used for it does not matter. Let's please look into what has broken in nw.js soon. It could be a breaking change in a recent version (that's what I would verify first).

@ce-bo ce-bo added this to the automation-basics milestone Jun 7, 2022
@luelista luelista self-assigned this Jun 8, 2022
@IhsenBouallegue IhsenBouallegue added the difficulty:medium The difficulty to solve this is not super complex but not easy either label Jun 8, 2022
luelista added a commit that referenced this issue Jun 15, 2022
luelista added a commit that referenced this issue Jun 22, 2022
luelista added a commit that referenced this issue Jun 22, 2022
luelista added a commit that referenced this issue Jun 22, 2022
luelista added a commit that referenced this issue Jun 22, 2022
luelista added a commit that referenced this issue Jun 22, 2022
* fix: nw for mac packaging needs exact nwVersion

issue #2823

* feat: check for nw.js version via renovate

issue #2823

* fix: install wine in release workflow to fix windows packaging

issue #2823
@luelista
Copy link
Contributor

luelista commented Jun 23, 2022

Okay, the fix for macOS worked, for Windows it still got problems. The packaging step reports this error:

Error: rcedit.exe failed with exit code 123
    at ChildProcess.<anonymous> (/home/runner/work/***/***/visualization/node_modules/rcedit/lib/rcedit.js:60:16)
    at ChildProcess.emit (node:events:527:28)
    at maybeClose (node:internal/child_process:1092:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5)

(https://github.com/MaibornWolff/codecharta/runs/7002261326?check_suite_focus=true)

luelista added a commit that referenced this issue Jun 28, 2022
@luelista
Copy link
Contributor

luelista commented Jul 5, 2022

Ich habe den nwjs Build unter Windows untersucht mit folgendem Ergebnis: Es läuft aktuell nicht, da der nwjs-builder-phoenix das Modul node-rcedit in einer sehr alten Version referenziert, die nur die 32-bit Version der "rcedit.exe" mitbringt. Daher läuft es in unserer 64-bit Buildpipeline nicht. Ein 32-bit kompatibles Wine dort zu installieren ist nicht trivial. Sinnvoller wäre es, die neuere node-rcedit zu nutzen, die unter 64-bit läuft. Mit folgendem snippet in der package.json kann ich zwar eine neuere node-rcedit erzwingen, allerdings läuft dann der builder nicht mehr durch, da sich das API von node-rcedit geändert hat:

"overrides": {
  "nwjs-builder-phoenix": {
    "rcedit": "^3.0.1"
  }
}

Wir müssten also unsere eigene, angepasste Version von nwjs-builder-phoenix pflegen.

Stattdessen würde ich empfehlen, erstmal das alternative Build-Tool nw-builder (https://github.com/nwutils/nw-builder) zu testen. Das ist zwar nur die zweite Wahl laut Doku (https://docs.nwjs.io/en/latest/For%20Users/Package%20and%20Distribute/), aber dafür deutlich aktueller (letztes Update 2022 statt 2018) - vielleicht ist die Doku veraltet. Es scheint auch ein sehr ähnliches Featureset zu haben.

@BridgeAR
Copy link
Member

BridgeAR commented Jul 5, 2022

+1 on switching to nw-builder.

luelista added a commit that referenced this issue Jul 5, 2022
MW-Friedrich added a commit that referenced this issue Jul 12, 2022
* fix: switch to nw-builder

issue #2823

* add changelog item

* Change package expression to work with windows (admin privileges)

* Change flavor from sdk to normal (production)

* Remove cleanup-call (is now included in build-nwjs.js)

* Remove test release entries

* Remove zip option, add clean up of build and package dirs, change to bestzip for zip process

* Refactor package process to be in one file

* Remove old parts of package process, add zip for bestzip

* refactor build-nwjs.js
#2881

* refactor build-nwjs.js to avoid a void return as a promise
#2881

* Update visualization/script/build-nwjs.js

Co-authored-by: Cedrik Bormann <26900540+ce-bo@users.noreply.github.com>

Co-authored-by: phanlezz <se.wo96@gmx.de>
Co-authored-by: MW-Friedrich <108457762+MW-Friedrich@users.noreply.github.com>
Co-authored-by: Cedrik Bormann <26900540+ce-bo@users.noreply.github.com>
@phanlezz
Copy link
Collaborator

Fixed by #2881

@phanlezz phanlezz reopened this Aug 3, 2022
@phanlezz
Copy link
Collaborator

phanlezz commented Aug 3, 2022

Currently the web package and the global npm install don't work of the visualization part.

@ce-bo
Copy link
Collaborator

ce-bo commented Aug 3, 2022

Would you please provide some More error details?

@phanlezz
Copy link
Collaborator

phanlezz commented Aug 3, 2022

Would you please provide some More error details?

After installing via npm i -g codecharta-virtualization and running it codecharta-virtualization under mac, only the command nw . pops up in the command line and process is finished. Nothing else happens.

The zip file of the -web version can't be extracted, because it is corrupted. Im already working on this second problem.

phanlezz added a commit that referenced this issue Aug 9, 2022
* Change zip path to not contain a directory up order

* Add entry for zip fix

* Remove temporary test file

#2823
@phanlezz
Copy link
Collaborator

phanlezz commented Aug 9, 2022

Closed via #2934, standalone applications should work now. There is still an issue with the MacOS npm-install version: #2967

@phanlezz phanlezz closed this as completed Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Only issues that describe bugs. difficulty:medium The difficulty to solve this is not super complex but not easy either priority:urgent Should be addressed ASAP
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants