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: Fixed 'ti sdk install' silent fail when copying new modules to dest #655

Merged
merged 5 commits into from
May 25, 2024

Conversation

cb1kenobi
Copy link
Contributor

@cb1kenobi cb1kenobi commented May 24, 2024

The part of ti sdk install that copies the module files was wrapped in a try/catch, but didn't surface the error.

The try/catch was basically a lazy catch when fs.statSync(destDir).isDirectory() throws for destDir not existing (e.g. new module found). Getting rid of the try/catch shows the error that destDir doesn't exist, which was expected, however throwing was breaking out of all the nested loops when recursively copying the module files to the dest. The solution is to remove the try/catch and use fs.existsSync().

This PR also resolves 2 other related issues.

  1. When installing an SDK that is already installed, it prompts the user if they want to overwrite the existing install, but it wasn't also overwriting the modules.
  2. There are 3 prompts that were incorrectly handling the return value from a prompt just like what happened in fix: properly handle confirm sdk uninstall prompt response, fixes #645 #649:
    1. SDK install overwrite prompt
    2. SDK uninstall version selection prompt
    3. ti setup user author name prompt

@cb1kenobi cb1kenobi added the bug label May 24, 2024
@cb1kenobi cb1kenobi requested a review from m1ga May 24, 2024 16:19
@cb1kenobi cb1kenobi self-assigned this May 24, 2024
@m1ga
Copy link
Contributor

m1ga commented May 25, 2024

  • reinstalling with an existing 12.3.0.GA - override - works
  • reinstalling with an existing 12.3.0.GA - rename - works
  • installing lastest without 12.3.0.GA - fails
Downloading https://github.com/tidev/titanium-sdk/releases/download/12_3_0_GA/mobilesdk-12.3.0.GA-osx.zip
  100%  [========================================]

TypeError: Cannot read properties of undefined (reading 'forceModules')
    at Object.onEntry (file:///Users/miga/titanium-cli/src/commands/sdk.js:649:27)
    at async ZipFile.<anonymous> (file:///Users/miga/titanium-cli/src/util/extract-zip.js:57:8)

@m1ga
Copy link
Contributor

m1ga commented May 25, 2024

normal install works again 👍 Will retest the other ways now, just to be sure
edit: all ways (rename, override, new) works

Copy link
Contributor

@m1ga m1ga left a comment

Choose a reason for hiding this comment

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

working fine now 👍

@cb1kenobi cb1kenobi merged commit 3500ba1 into main May 25, 2024
19 checks passed
@cb1kenobi cb1kenobi deleted the sdk-install-modules-fix branch May 25, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants