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

[Manager][linux] Search for skins in /usr/(local)/share/boinc-manager/skins or ./skins folders. #4811

Merged
merged 1 commit into from Jun 28, 2022

Conversation

AenBleidd
Copy link
Member

@AenBleidd AenBleidd commented Jun 24, 2022

This fixes #4809.

Signed-off-by: Vitalii Koshura lestat.de.lionkur@gmail.com

@AenBleidd AenBleidd force-pushed the vko_add_skins_support_of_linux branch 3 times, most recently from 8689efa to ca91c9b Compare Jun 24, 2022
@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 24, 2022

@LocutusOfBorg, @Germano0, please review

@AenBleidd AenBleidd self-assigned this Jun 24, 2022
@AenBleidd AenBleidd added this to In Progress in Client Release 7.20.1 via automation Jun 24, 2022
@AenBleidd AenBleidd added this to the Client Release 7.20.1 milestone Jun 24, 2022
@AenBleidd AenBleidd marked this pull request as ready for review Jun 24, 2022
@AenBleidd AenBleidd moved this from In Progress to In Review in Client Release 7.20.1 Jun 24, 2022
@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

Please add screen shots

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 25, 2022

@talregev, done.
image

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

Thank you.
Also we should add to the CI that it will copy the skin folder along the binary.
Can you add that?

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 25, 2022

Also we should add to the CI that it will copy the skin folder along the binary.
Can you add that?

We don't do this for any other flow, so definitely not in this PR since it has nothing with the bug that is fixed with this PR

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

We should do this for all other flow. We will add this also for windows and mac. Also locale folder.

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

@AenBleidd Where is the skin folder? I don't see it on our repo.

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 25, 2022

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

We should do this for all other flow. We will add this also for windows and mac. Also locale folder.

Can you make a PR after this PR will merge?

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 25, 2022

We should do this for all other flow. We will add this also for windows and mac. Also locale folder.

Can you make a PR after this PR will merge?

@talregev, I'll add it to the backlog and will decide if that is necessary

@talregev
Copy link
Contributor

talregev commented Jun 25, 2022

We should do this for all other flow. We will add this also for windows and mac. Also locale folder.

Can you make a PR after this PR will merge?

@talregev, I'll add it to the backlog and will decide if that is necessary

We should. Usually I am testing the binary from CI. That why I added the c files for WebAssembly debug.

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 25, 2022

why aren't such skins installed in Makefile.am? How can the packager know they are needed if the build system is not installing them?

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 25, 2022

That's a very good question! Thanks, @LocutusOfBorg, I'll fix that

@AenBleidd AenBleidd marked this pull request as draft Jun 25, 2022
@talregev
Copy link
Contributor

talregev commented Jun 26, 2022

@LocutusOfBorg Please answer on discord. Thank you :)

@AenBleidd AenBleidd force-pushed the vko_add_skins_support_of_linux branch from ca91c9b to a2d0795 Compare Jun 27, 2022
@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 27, 2022

Any subsequent build after boinc https://code.launchpad.net/~costamagnagianfranco/+recipe/boinc-upstream-daily
7.22.0+dfsg+git20220627+r25142~r115
or https://code.launchpad.net/~costamagnagianfranco/+recipe/boinc-daily
7.20.0+dfsg+202206271354

Should have skins installed, but please install them from Makefile.am

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 27, 2022

Should have skins installed, but please install them from Makefile.am

This is exactly what I am working on right now.

Thanks, @LocutusOfBorg

@AenBleidd AenBleidd force-pushed the vko_add_skins_support_of_linux branch from a2d0795 to d85be5c Compare Jun 27, 2022
@AenBleidd AenBleidd marked this pull request as ready for review Jun 27, 2022
@AenBleidd AenBleidd requested a review from talregev Jun 27, 2022
@AenBleidd AenBleidd assigned talregev and unassigned AenBleidd Jun 27, 2022
…/skins or ./skins folders.

This fixes BOINC#4809.

Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
@AenBleidd AenBleidd force-pushed the vko_add_skins_support_of_linux branch from d85be5c to 4dba080 Compare Jun 27, 2022
Copy link
Contributor

@talregev talregev left a comment

I download the manger with webview artifact, run it on wsl.
locale and skins folder are working.

image

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 28, 2022

Ok nice thanks! I committed the change in my packaging system to use the upstream installation path. Now the build is broken in boinc-upstream-daily until this PR is merged :)

@talregev
Copy link
Contributor

talregev commented Jun 28, 2022

Ok nice thanks! I committed the change in my packaging system to use the upstream installation path. Now the build is broken in boinc-upstream-daily until this PR is merged :)

Can you test your build on this PR before merge?
Build on vitalii branch?

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 28, 2022

Can you test your build on this PR before merge?
yes

Build on vitalii branch?
no

I can fetch different branches from BOINC/boinc, but I don't want to add new sources. Instead, I extracted the PR as patch on this link https://github.com/BOINC/boinc/pull/4811.patch and added it to the Debian packaging

https://git.launchpad.net/~costamagnagianfranco/+git/boinc-upstream/commit/?id=ee0fec1474d100a905ad111d6724fca4ae5a45ca

So, this patch will break the build once this PR is merged, and I'll just remove it.

G.

@AenBleidd AenBleidd merged commit 0838608 into BOINC:master Jun 28, 2022
39 checks passed
Client Release 7.20.1 automation moved this from In Review to Done Jun 28, 2022
@talregev
Copy link
Contributor

talregev commented Jun 28, 2022

Can you test your build on this PR before merge?
yes

Build on vitalii branch?
no

I can fetch different branches from BOINC/boinc, but I don't want to add new sources. Instead, I extracted the PR as patch on this link https://github.com/BOINC/boinc/pull/4811.patch and added it to the Debian packaging

https://git.launchpad.net/~costamagnagianfranco/+git/boinc-upstream/commit/?id=ee0fec1474d100a905ad111d6724fca4ae5a45ca

So, this patch will break the build once this PR is merged, and I'll just remove it.

G.

Thank you!

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 29, 2022

Screenshot from 2022-06-29 10-03-17
Screenshot from 2022-06-29 10-07-01

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 29, 2022

On simple view, click "notices" brings an empty page and then the program exits...

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 29, 2022

@LocutusOfBorg, is looks like this issue: #4784

@AenBleidd AenBleidd deleted the vko_add_skins_support_of_linux branch Jun 29, 2022
@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 29, 2022

mmm but I didn't switch between simple and advanced view, just clicked on "notices"

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 29, 2022

@LocutusOfBorg, yeah because the real crash is happens inside wxWebView component that is used on Notices windows and Notices tab

@LocutusOfBorg
Copy link
Contributor

LocutusOfBorg commented Jun 29, 2022

ok so the skin "issues" is fixed and working properly!

@AenBleidd
Copy link
Member Author

AenBleidd commented Jun 29, 2022

@LocutusOfBorg, thank you for testing this!

@AenBleidd AenBleidd moved this from Done to Merged to Release in Client Release 7.20.1 Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Client Release 7.20.1
  
Merged to Release
Development

Successfully merging this pull request may close these issues.

4 participants