chore: add missing bench task config to complex/float64/base/assert/is_equal#10235
chore: add missing bench task config to complex/float64/base/assert/is_equal#10235Om-A-osc wants to merge 1 commit intostdlib-js:developfrom
complex/float64/base/assert/is_equal#10235Conversation
…-equal
---
type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes.
report:
- task: lint_filenames
status: passed
- task: lint_editorconfig
status: passed
- task: lint_markdown
status: na
- task: lint_package_json
status: na
- task: lint_repl_help
status: na
- task: lint_javascript_src
status: na
- task: lint_javascript_cli
status: na
- task: lint_javascript_examples
status: na
- task: lint_javascript_tests
status: na
- task: lint_javascript_benchmarks
status: na
- task: lint_python
status: na
- task: lint_r
status: na
- task: lint_c_src
status: na
- task: lint_c_examples
status: na
- task: lint_c_benchmarks
status: na
- task: lint_c_tests_fixtures
status: na
- task: lint_shell
status: na
- task: lint_typescript_declarations
status: passed
- task: lint_typescript_tests
status: na
- task: lint_license_headers
status: passed
---
Coverage Report
The above coverage report was generated for the changes in this PR. |
|
@Planeshifter Please have a look whenever possible. |
Planeshifter
left a comment
There was a problem hiding this comment.
That's very weird: the benchmark task in the manifest is used to compile native C code for benchmark/benchmark.native.js and benchmark/c/benchmark.c, but this package has neither of those files. I don't understand how you might have encountered an error due to this configuration being missing. Please elaborate; otherwise we should go ahead and close this PR.
|
Thanks for looking into this. @Planeshifter The failure I encountered is not related to this package having its own What actually failedRunning: failed while compiling the zgemv C benchmark because the header could not be found. Why it failedThe stdlib build system uses When resolving dependencies, the resolver requires an exact task From the debug output during dependency resolution: This shows:
Why this only appears during benchmarksNode addon builds succeed because they do not compile the C benchmark The failure appears only when compiling pure C benchmarks, which #include "stdlib/complex/float64/base/assert/is_equal.h"Header resolution for these builds depends entirely on the manifest Why the absence of benchmark files is unrelatedThe It is also used when:
So even though Minimal reproduction logicGiven: is-equal manifest
zgemv benchmark task
Why the change fixes the issueMaking the manifest task-agnostic (or adding a Why this may not have surfaced earlierThis only occurs when: • a C benchmark includes a dependency header This combination is relatively uncommon.
|
|
@Planeshifter @kgryte I tried to explain the issue I am facing as simply as possible. Please correct me if I am understanding anything wrong. |
|
The reported issue is legitimate; however, @Planeshifter is right that adding this configuration feels incorrect. The underlying issue is actually more general. Namely, we need Currently, we "over"-resolve dependencies. E.g., when compiling a native add-on, we resolve the native add-on dependencies for all dependencies of a perspective package when, in fact, we only need to resolve the native add-on dependencies for the top-level package which is being compiled. For all the dependencies, we only need the list of source files and their immediate dependents. Fixing this will require effectively updating all |
|
@kgryte While working on zgemv, I somehow managed to stumble into this rare edge case. I’m starting to think I have a special talent for attracting complex build issues. However For this PR, I see a few options: Add a C benchmark to the package as a workaround. Merge with a benchmark config but no benchmarks. Introduce the proposed base manifest configuration (the correct long-term fix, but requires widespread manifest updates). modify library-manifest to gracefully fallback to the "build" task for any dependency that does not satisfy the current conditions (like benchmark or test). ( I was trying this and it seems to work nicely ) Let me know which direction we should proceed for now. |
This is not something we should do. |

type: pre_commit_static_analysis_report
description: Results of running static analysis checks when committing changes. report:
Resolves #N/A.
Description
This pull request:
complex/float64/base/assert/is_equal.Related Issues
This pull request has no related issues.
Questions
No.
Other
No.
Checklist
AI Assistance
Disclosure
N/A.
@stdlib-js/reviewers