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

feat: App-specific ApplicationData on Skia #12596

Merged

Conversation

MartinZikmund
Copy link
Member

@MartinZikmund MartinZikmund commented Jun 9, 2023

GitHub Issue (If applicable): related to #6420, closes #8910

Replaces #12314

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Copilot Summary

🤖 Generated by Copilot at b807adc

This pull request adds support for the Skia backend to the SamplesApp project, refactors the rendering and extension logic of the Uno.UI.Composition and Uno.UI.Runtime.Skia.Gtk projects, and aligns the app manifest of the SamplesApp.UWP project with the Uno Platform branding. It also renames, deletes, or updates some files and namespaces to improve the code organization and consistency.

Nope, you didn't get it this time 😂

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

@MartinZikmund MartinZikmund self-assigned this Jun 9, 2023
@github-actions github-actions bot added area/automation Categorizes an issue or PR as relevant to project automation area/skia ✏️ Categorizes an issue or PR as relevant to Skia platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform labels Jun 9, 2023
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from b807adc to 18ff98c Compare June 19, 2023 19:28
@github-actions github-actions bot removed platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform labels Jun 19, 2023
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from 18ff98c to 99b7eeb Compare June 19, 2023 19:34
@github-actions github-actions bot added platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform labels Jun 20, 2023
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from 0f042ba to f664560 Compare June 21, 2023 06:09
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from 67c45ac to a030f4b Compare June 22, 2023 15:28
@jeromelaban jeromelaban changed the base branch from feature/5x to master June 29, 2023 15:32
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch 2 times, most recently from 67ec303 to c1f93c3 Compare July 7, 2023 20:39
@MartinZikmund MartinZikmund enabled auto-merge July 7, 2023 20:47
@MartinZikmund MartinZikmund disabled auto-merge July 10, 2023 13:47
@MartinZikmund MartinZikmund enabled auto-merge July 10, 2023 13:48
@MartinZikmund MartinZikmund marked this pull request as ready for review July 25, 2023 13:14
@MartinZikmund
Copy link
Member Author

@jeromelaban @Youssef1313 The application data path is now correct and user specific on Windows as well as Unix-based platforms, but my concern is with the Temp folder - as Path.GetTempPath() just returns /tmp/ on Linux, which is not really user-specific (as compared to Windows) - https://learn.microsoft.com/en-us/dotnet/api/system.io.path.gettemppath?view=net-7.0&tabs=windows . Also /tmp is potentially very temporary, so it could get cleaned up even while the app is running. Is that acceptable? I provided feature flag to allow overriding the path manually, so the developer is still in control

@Youssef1313
Copy link
Member

@MartinZikmund I think /var/tmp would be better on Linux. Tagging @ramezgerges for his view as well.

@ramezgerges
Copy link
Contributor

@MartinZikmund I think /var/tmp would be better on Linux. Tagging @ramezgerges for his view as well.

I'm not sure about the temp directory requirements for Uno specifically, but the vast majority of linux apps/scripts use /tmp directly. Most distros clear /tmp on reboot, and that should be enough for most scenarios. If it isn't, something more persistent like the local folder should probably be used.

@MartinZikmund MartinZikmund enabled auto-merge July 26, 2023 15:19
@github-actions github-actions bot added area/build Categorizes an issue or PR as relevant to build infrastructure kind/documentation labels Jul 27, 2023
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from a079700 to 35d9770 Compare July 27, 2023 12:30
@MartinZikmund MartinZikmund force-pushed the dev/mazi/packageid-skia branch from 5258477 to 389c5df Compare July 27, 2023 13:02
@MartinZikmund MartinZikmund merged commit 4cad602 into unoplatform:master Jul 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/automation Categorizes an issue or PR as relevant to project automation area/build Categorizes an issue or PR as relevant to build infrastructure area/skia ✏️ Categorizes an issue or PR as relevant to Skia kind/documentation platform/android 🤖 Categorizes an issue or PR as relevant to the Android platform platform/ios 🍎 Categorizes an issue or PR as relevant to the iOS platform platform/macos 🍏 Categorizes an issue or PR as relevant to the macOS platform platform/wasm 🌐 Categorizes an issue or PR as relevant to the WebAssembly platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Skia] ApplicationData.Current.LocalFolder redirects to %appdata%
5 participants