Skip to content

Conversation

@piotrrak
Copy link

This cleans up meson warning:

WARNING: You should add the boolean check kwarg to the run_command call.
It currently defaults to false,
but it will default to true in meson 2.0.
See also: mesonbuild/meson#9300

Self evaluation:

  1. Build test: [ ]Passed [ ]Failed [*]Skipped
  2. Run test: [ ]Passed [ ]Failed [*]Skipped

How to evaluate:
N/A

Copy link
Contributor

@baek2sm baek2sm left a comment

Choose a reason for hiding this comment

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

Good modification! It seems useful for debugging when issues occured, too.

Copy link
Collaborator

@jijoongmoon jijoongmoon left a comment

Choose a reason for hiding this comment

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

LGTM

@jijoongmoon jijoongmoon changed the title Meson run command check [Wait for #2963] Meson run command check Mar 8, 2025
@piotrrak piotrrak force-pushed the meson_run_command_check branch from 2f614b6 to df26d50 Compare March 10, 2025 11:13
@piotrrak piotrrak marked this pull request as ready for review March 10, 2025 11:14
@piotrrak
Copy link
Author

Updated with regards to windows/nproc changes.

@piotrrak
Copy link
Author

piotrrak commented Mar 10, 2025

Invalid command that was exposed by this change is:

Ubuntu:
Applications/MixedPrecision/jni/meson.build:5:0: ERROR: Command "/usr/bin/cp -lr /home/runner/work/nntrainer/nntrainer/Applications/MixedPrecision/jni/../res /home/runner/work/nntrainer/nntrainer/build/res/app/MixedPrecision" failed with status 1.

Windows:
test\unittest\meson.build:35:4: ERROR: Command C:\Windows\system32\cmd.exe /C mkdir D:\a\nntrainer\nntrainer\builddir\res\test` failed with status 1.`
@gkisalapl could you please take a look why this mkdir is failing here? Maybe this directory is already created somewhere else?

@piotrrak
Copy link
Author

Rebased on top of #2999 and #3005

@piotrrak
Copy link
Author

Android failure:

  ...
  Run-time dependency capi-ml-common found: NO (tried pkgconfig and cmake)
  Message: preparing blas
  
  meson.build:313:4: ERROR: Include dir openblas/include does not exist.
  
  A full log can be found at /home/runner/work/nntrainer/nntrainer/builddir/meson-logs/meson-log.txt
  Error: Process completed with exit code 1.

Honestly no idea

This change removes copy run_command that was always failing without us noticing.
This was part of change 931c586
"[Application] Add Mixed Precision Application"
Path Application/MixedPrecision/res has never existed.

Signed-off-by: Piotr Rak <[email protected]>
Since result.status() is rarely tested one should always specify it.
This also fixes meson warning.

Signed-off-by: Piotr Rak <[email protected]>
@piotrrak piotrrak force-pushed the meson_run_command_check branch from dd940e1 to 03c879f Compare March 12, 2025 11:05
@piotrrak
Copy link
Author

Since PR #3005 was already merged rebased only on main + PR #2999

Copy link
Contributor

@mwlasiuk mwlasiuk left a comment

Choose a reason for hiding this comment

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

Would like to have it

Copy link
Contributor

@djeong20 djeong20 left a comment

Choose a reason for hiding this comment

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

LGTM!

@piotrrak
Copy link
Author

@jijoongmoon Hi, rebase is not required for this change

@jijoongmoon jijoongmoon merged commit bc8560b into nnstreamer:main Mar 18, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants