Skip to content

deps: update ada to 3.1.1 #57190

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

Closed
wants to merge 1 commit into from
Closed

Conversation

nodejs-github-bot
Copy link
Collaborator

This is an automated update of ada to 3.1.1.

@nodejs-github-bot nodejs-github-bot added the dependencies Pull requests that update a dependency file. label Feb 24, 2025
@nodejs-github-bot
Copy link
Collaborator Author

Review requested:

  • @nodejs/security-wg
  • @nodejs/url

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 24, 2025
@anonrig anonrig added the needs-benchmark-ci PR that need a benchmark CI run. label Feb 24, 2025
@anonrig
Copy link
Member

anonrig commented Feb 24, 2025

@nodejs/performance can someone run url benchmarks?

@anonrig anonrig added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Feb 24, 2025
@anonrig
Copy link
Member

anonrig commented Feb 24, 2025

@lemire any idea why we didn't catch this on CI?

@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 24, 2025
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator Author

@aduh95
Copy link
Contributor

aduh95 commented Feb 24, 2025

 ../src/node_config_file.h:35:32: error: 'simdjson' has not been declared
    35 |   ParseResult ParseNodeOptions(simdjson::ondemand::object* node_options_object);
       |                                ^~~~~~~~
 ../src/node_config_file.h:35:58: error: expected ',' or '...' before '*' token
    35 |   ParseResult ParseNodeOptions(simdjson::ondemand::object* node_options_object);
       |                                              

@richardlau
Copy link
Member

 ../src/node_config_file.h:35:32: error: 'simdjson' has not been declared
    35 |   ParseResult ParseNodeOptions(simdjson::ondemand::object* node_options_object);
       |                                ^~~~~~~~
 ../src/node_config_file.h:35:58: error: expected ',' or '...' before '*' token
    35 |   ParseResult ParseNodeOptions(simdjson::ondemand::object* node_options_object);
       |                                              

Should be fixed by #57197 (was a race condition between two separate PR's landing on main).

@anonrig
Copy link
Member

anonrig commented Feb 24, 2025

@richardlau any idea why github 24.04 succeeds but arm doesn't? somehow -Wsign-compare is enabled on github ci for both macos and ubuntu 24.04

../deps/ada/ada.h:9484:48: error: comparison of integers of different signs: 'const value_type' (aka 'const char') and 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
 9484 |   if (!token->value.empty() && token->value[0] != value) {
      |                                ~~~~~~~~~~~~~~~ ^  ~~~~~
      ```

@richardlau
Copy link
Member

@anonrig Some of the CI runs with configure --error-on-warn.

@richardlau
Copy link
Member

@anonrig Some of the CI runs with configure --error-on-warn.

hmm but that is set on both the arm and non-arm Ubuntu 24.04 GitHub workflows. I can only assume either:

  • code paths differ for x64 vs arm64
  • types are declared differently for the archs
  • compiler issue? (clang 18 in both cases)

@anonrig anonrig closed this Feb 25, 2025
@anonrig anonrig deleted the actions/tools-update-ada branch February 25, 2025 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. dependencies Pull requests that update a dependency file. needs-benchmark-ci PR that need a benchmark CI run. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants