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

fix(bundler/macos): clean up .app bundle if only .dmg is enabled #7081 #7116

Merged
merged 5 commits into from
Jun 5, 2023

Conversation

fcfangcc
Copy link
Contributor

@fcfangcc fcfangcc commented Jun 2, 2023

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

fix #7081. clean up .app bundle if only .dmg is enabled

@fcfangcc fcfangcc requested a review from a team as a code owner June 2, 2023 06:51
@wusyong wusyong added the status: needs review A maintainer must review this code label Jun 2, 2023
@FabianLars
Copy link
Member

Thanks for contributing! ❤️

2 things:

  1. We require all commits to be signed across Tauri's repositories. Follow this guide to set up commit signing. Once all the commits in this PR are signed we can merge your contribution. Thanks for being part of our community!
  2. I think we also need to check if the Updater bundle is being build (which will happen after the .dmg bundle is being build) so that it doesn't have to rebuild&resign the .app bundle. -> The "after the .dmg bundle" thing is important because that means we can't just extend the existing package_type == MacOsBundle check, you could reset the vec in a separate check or something idk

@fcfangcc
Copy link
Contributor Author

fcfangcc commented Jun 5, 2023

Only in the platform::xx::bundle_project method can get tmp files.
My idea is change platform::xx::bundle_project method return like

pub struct BuildBundle {
  bundles: Vec<PathBuf>,
  trashes: Vec<PathBuf>,
}
// pub fn bundle_project(settings: &Settings) -> crate::Result<Vec<PathBuf>>
pub fn bundle_project(settings: &Settings) -> crate::Result<BuildBundle> {
...
}

Then in bundle::bundle_project like

let trashes = vec![]
for package_type in &package_types {
  let bundle = match package_type {
    #[cfg(target_os = "macos")]
    PackageType::MacOsBundle => macos::app::bundle_project(&settings)?,
    #[cfg(target_os = "macos")]
    PackageType::IosBundle => macos::ios::bundle_project(&settings)?,
    // dmg is dependant of MacOsBundle, we send our bundles to prevent rebuilding
    ...
    _ => {
      warn!("ignoring {:?}", package_type);
      continue;
    }
  };

  trashes.extend_from_slice(&bundle.trashes)
}
// clean up trashes after all bundle is build.

@FabianLars If it's suitable, I will implement it 😃

@FabianLars
Copy link
Member

Changing function signatures is a bit tricky because it's technically a breaking chance the the bundler is also released as a normal crate 🤔

Does my idea not work? To be clear, what i meant is to add something like this right above the for app_bundle_path in app_bundle_paths {} line

if !bundles
    .iter()
    .filter(|bundle| bundle.package_type == PackageType::Updater) {
      app_bundle_paths.clear();
    }

@fcfangcc
Copy link
Contributor Author

fcfangcc commented Jun 5, 2023

emm...
I misunderstand your meaning

@lucasfernog lucasfernog self-assigned this Jun 5, 2023
lucasfernog
lucasfernog previously approved these changes Jun 5, 2023
@FabianLars
Copy link
Member

Just in case: It's missing a changefile

@lucasfernog
Copy link
Member

Totally missed it, nice catch :)

@lucasfernog lucasfernog merged commit 3327dd6 into tauri-apps:dev Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs review A maintainer must review this code
Projects
Status: 🔎 In audit
Development

Successfully merging this pull request may close these issues.

[bug] bundler doesn't clean up .app bundle if only .dmg is enabled
4 participants