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/defaultTargets_updating #1782

Merged
merged 11 commits into from
Nov 26, 2024
Merged

fix/defaultTargets_updating #1782

merged 11 commits into from
Nov 26, 2024

Conversation

ElenaDiachenko
Copy link
Contributor

Description

  • On Android, AndroidTV, fireTV, AndroidWear, Tizen, Tizenwatch platforms defaultTarget object of .rnv/renative.json isn't updated with default platforms

Related issues

Npm releases

n/a

@ElenaDiachenko ElenaDiachenko added this to the 1.7 milestone Nov 12, 2024
@ElenaDiachenko ElenaDiachenko self-assigned this Nov 12, 2024
@GabrieleKaceviciute
Copy link
Collaborator

GabrieleKaceviciute commented Nov 12, 2024

On android platforms (android, androidTV, androidwear, firetv) rnv asks ->

? What target would you like to use? Television_1080p_API_31 | TV 📺  | arch: arm64-v8a | udid: unknown
? Your default target for platform firetv is not defined. Do you want to set it to Television_1080p_API_31  Yes

On ios and tvos ->

? No global or project default simulator or device defined. Please select a supported simulator to use iPhone 16 | Phone 📱 | v: 1
8.1 (22B81) | udid: CCD32DDF-CC9D-44C8-8E4A-0CA1CD4623A7
warn: ○ run: Your default target for platform ios is set to undefined.
? What to do next? (Use arrow keys)
❯ Update project default target for platform ios
  Update global default target for platform ios
  Don't update

Note: On webos -> didn't ask to choose target (I have only one webos simulator installed)

info: Found simulator webOS_TV_24_Simulator_1.4.1 in ../../../../../webOS_TV_SDK/Simulator
? Your default target for platform webos is not defined. Do you want to set it to webOS_TV_24_Simulator_1.4.1  (Y/n)

Is it expected that android target is set on global but ios you could choose?

@pauliusguzas
Copy link
Collaborator

pauliusguzas commented Nov 12, 2024

tizen, tizenwatch isn't installed when user chooses any answer. And also same as on android, no way to choose where to set default target
Screenshot 2024-11-12 at 11 58 03
Screenshot 2024-11-12 at 11 59 40

return true;
} else {
await executeAsync(
`${c.cli[CLI_TIZEN_EMULATOR]} launch --name ${name}`,

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input Medium

This string concatenation which depends on
library input
is later used in a
shell command
.

Copilot Autofix AI about 1 month ago

To fix the problem, we should avoid constructing the shell command using string concatenation with untrusted input. Instead, we can use the child_process.execFile method, which allows us to pass arguments as an array, thus avoiding shell interpretation of the input.

  • Replace the use of executeAsync with execFile from the child_process module.
  • Ensure that the name parameter is passed as an argument in the array format to execFile.
Suggested changeset 1
packages/sdk-tizen/src/deviceManager.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/sdk-tizen/src/deviceManager.ts b/packages/sdk-tizen/src/deviceManager.ts
--- a/packages/sdk-tizen/src/deviceManager.ts
+++ b/packages/sdk-tizen/src/deviceManager.ts
@@ -139,6 +139,12 @@
             } else {
-                await executeAsync(
-                    `${c.cli[CLI_TIZEN_EMULATOR]} launch --name ${name}`,
-                    ExecOptionsPresets.SPINNER_FULL_ERROR_SUMMARY
-                );
+                const { execFile } = require('child_process');
+                await new Promise((resolve, reject) => {
+                    execFile(c.cli[CLI_TIZEN_EMULATOR], ['launch', '--name', name], ExecOptionsPresets.SPINNER_FULL_ERROR_SUMMARY, (error, stdout, stderr) => {
+                        if (error) {
+                            reject(error);
+                        } else {
+                            resolve(stdout);
+                        }
+                    });
+                });
                 return true;
EOF
@@ -139,6 +139,12 @@
} else {
await executeAsync(
`${c.cli[CLI_TIZEN_EMULATOR]} launch --name ${name}`,
ExecOptionsPresets.SPINNER_FULL_ERROR_SUMMARY
);
const { execFile } = require('child_process');
await new Promise((resolve, reject) => {
execFile(c.cli[CLI_TIZEN_EMULATOR], ['launch', '--name', name], ExecOptionsPresets.SPINNER_FULL_ERROR_SUMMARY, (error, stdout, stderr) => {
if (error) {
reject(error);
} else {
resolve(stdout);
}
});
});
return true;
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@GabrieleKaceviciute
Copy link
Collaborator

  • webos -> need to ask to choose target before where to set default target
  • tizen -> app doesn't installed on simulator whatever you choose
    error on CLI:
? which emulator or device would you like to launch? T-8.0-x86_64


warn: ○ run: Your default target for platform tizen is not defined.
? What to do next? Update project default target for platform tizen
√ Executing: "C:\tizen-studio\tools\sdb.exe" devices
√ Executing: C:\tizen-studio\tools\emulator\bin\em-cli.bat launch --name T-8.0-x86_64
× Can't connect to the running emulator. Try restarting it.
? Could not find or connect to the specified target (T-8.0-x86_64

). Would you like to start an emulator? Yes
error: ⨯ ○ run: No default target found for tizen. please provide one using -t option

NOTE: project/global default target updates like this -> "defaultTargets": { "tizen": "T-8.0-x86_64\r" } but simulator name is "T-8.0-x86_64" (without "\r")

Tizenwatch -> works as expected.
Tizen, tizenwatch tested on windows

@pauliusguzas
Copy link
Collaborator

on mac both tizen and tizenwatch work as expected

@pauliusguzas pauliusguzas added e2e and removed e2e labels Nov 15, 2024
@Marius456 Marius456 marked this pull request as draft November 15, 2024 08:01
@GabrieleKaceviciute
Copy link
Collaborator

GabrieleKaceviciute commented Nov 15, 2024

windows

  • Tizen -> fails the first time after yarn bootstrap ( doesn't matter what I choose (update global / update project / don't update)) -> tizen simulator opens, but app doesn't installed, and simulator closes when this error occurs on CLI
? which emulator or device would you like to launch? T-8.0-x86_64
warn: ○ run: Your default target for platform tizen is not defined.
? What to do next? Update project default target for platform tizen
√ Executing: "C:\tizen-studio\tools\sdb.exe" devices
√ Executing: C:\tizen-studio\tools\emulator\bin\em-cli.bat launch --name T-8.0-x86_64
√ Waiting for emulator to boot...
√ Executing: "C:\tizen-studio\tools\ide\bin\tizen.bat" build-web -- "C:\Users\dev\Desktop\qa\renative\packages\template-starter\platformBuilds\template_tizen" -out "C:\Users\dev\Desktop\qa\renative\packages\template-starter\platformBuilds\template_tizen\intermediate"
√ Executing: "C:\tizen-studio\tools\ide\bin\tizen.bat" package -- "C:\Users\dev\Desktop\qa\renative\packages\template-starter\platformBuilds\template_tizen\intermediate" -s RNVanillaCert -t wgt -o "C:\Users\dev\Desktop\qa\renative\packages\template-starter\platformBuilds\template_tizen\output"
√ Executing: "C:\tizen-studio\tools\ide\bin\tizen.bat" uninstall -p NkVRhWHJSX.RNVanillaTV -t T-8.0-x86_64
√ Executing: "C:\tizen-studio\tools\ide\bin\tizen.bat" install -- "C:\Users\dev\Desktop\qa\renative\packages\template-starter\platformBuilds\template_tizen\output" -n RNVanillaTV.wgt -t T-8.0-x86_64
× FAILED: "C:\tizen-studio\tools\ide\bin\tizen.bat" run -p NkVRhWHJSX -t T-8.0-x86_64
This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason:
No emulator -t target name specified!
No emulator -t target name specified!
PS C:\Users\dev\Desktop\qa\renative\packages\template-starter>

NOTE - project/global default target updates correctly

macOS ->
If you choose "update global" -> app installed to simulator but doesn't launches. If you try to launch manually - crashes.

@pauliusguzas
Copy link
Collaborator

Not implemented on kaios

@ElenaDiachenko ElenaDiachenko marked this pull request as ready for review November 24, 2024 14:56
@Marius456 Marius456 added e2e and removed e2e labels Nov 25, 2024
@GabrieleKaceviciute GabrieleKaceviciute removed their request for review November 25, 2024 08:14
@pauliusguzas
Copy link
Collaborator

tizen, tizenwatch, webos works without issues. Kaios never asks to choose default target (I currently have only one kaios sim, so maybe there is connection?). Also kaios sim is searched on weird path, see picture, there is "false" directory added
image

@pauliusguzas pauliusguzas merged commit d2b4189 into main Nov 26, 2024
3 checks passed
@ElenaDiachenko ElenaDiachenko deleted the fix/defaultTargets_updating branch December 3, 2024 06:52
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.

4 participants