Skip to content

[RAPPS] Add ExeInZip installer type to support running installers in zip files #7866

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

Merged
merged 3 commits into from
Apr 29, 2025

Conversation

whindsaks
Copy link
Contributor

This feature is to help streamline the installing experience. It will extract the downloaded .zip file to a subfolder in %temp% and execute the the installer. After the installer has completed, the temporary directory is deleted.

For a package like Hxd, simply adding Installer = ExeInZip to the main section is enough because Rapps will default to the first .exe it finds in the root of the extracted zip. For .zip files with a folder structure or multiple .exe files, the file to execute needs to be specified.

For the Royale theme (PR waiting), it would look something like this:

[Section]
Name = Royale
...
Installer = ExeInZip

[ExeInZip]
Exe = Royale Theme for Win XP (2)\Royale Theme for Win XP.exe

Notes:

  • WinGet calls this simply zip installer type but they have the concept of NestedInstallerType which we don't.

Testbot runs (Filled in by Devs)

  • KVM x86:
  • KVM x64:

@whindsaks whindsaks added the enhancement For PRs with an enhancement/new feature. label Apr 5, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

@learn-more
Copy link
Member

Sorry, I don't work on rapps anymore.

@HBelusca HBelusca self-requested a review April 17, 2025 19:54
Comment on lines +848 to +849
CAppDB db(CAppDB::GetDefaultPath());
db.UpdateAvailable();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need yet another instance of the CAppDB? There is one that has been already created when RAPPS has been started. Isn't it possible to use that one instead?

Copy link
Contributor Author

@whindsaks whindsaks Apr 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. The "main" instance is lost in loaddlg.cpp (the download dialog).
  2. The download stuff happens on a background thread and CAppDB is probably not thread safe.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm ok... It may be of interest in the future to make this appdb a singleton or something like that (so that there's only one running globally for the given rapps instance).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if making all its methods thread safe is worth the effort of doing that.

Using a global lock instead would slow down the normal use case just to deal with this rare instance where we want to ask about a single property during install.

Copy link
Contributor

@HBelusca HBelusca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment, but lgtm otherwise.


if (!GetTempPathW(_countof(TempDirBuf), TempDirBuf))
return E_FAIL;
wsprintfW(UniqueDir, L"~%s-%u", RAPPS_NAME, GetCurrentProcessId());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer using wsprintf instead.

@whindsaks whindsaks merged commit 37e2c59 into reactos:master Apr 29, 2025
34 checks passed
@whindsaks whindsaks deleted the RappsExeInZipInst branch April 29, 2025 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For PRs with an enhancement/new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants