Skip to content

The version list should be sorted by name #45630

Description

@MrJithil

The version list should be sorted.

Current Behaviour:

process.versions
{
  node: '20.0.0-pre',
  v8: '10.8.168.20-node.8',
  uv: '1.44.2',
  zlib: '1.2.13',
  brotli: '1.0.9',
  ares: '1.18.1',
  modules: '111',
  nghttp2: '1.51.0',
  napi: '8',
  llhttp: '8.1.0',
  cjs_module_lexer: '1.2.2',
  base64: '0.5.0',
  openssl: '3.0.7+quic',
  cldr: '42.0',
  icu: '72.1',
  tz: '2022f',
  unicode: '15.0',
  ngtcp2: '0.8.1',
  nghttp3: '0.7.0'
}

Problem with the current order:

Since the current version list is not sorted, it's hard to compare with the source code (mainly the deps folder).

Solution:

Except the node, sort all other versions by name.
Eg:

process.versions
{
  node: '20.0.0-pre',
  ares: '1.18.1',
  base64: '0.5.0',
  brotli: '1.0.9',
  cjs_module_lexer: '1.2.2',
  cldr: '42.0',
  ...
  ...
  uv: '1.44.2',
  v8: '10.8.168.20-node.8',
  zlib: '1.2.13',
}

Activity

  1. MrJithil commented on Nov 26, 2022

    @MrJithil
    MemberAuthor

    Any easy ways to sort this macro in cpp?

    #define NODE_VERSIONS_KEYS(V)                                                  \
      NODE_VERSIONS_KEYS_BASE(V)                                                   \
      NODE_VERSIONS_KEY_CRYPTO(V)                                                  \
      NODE_VERSIONS_KEY_INTL(V)                                                    \
      NODE_VERSIONS_KEY_QUIC(V)
  2. mscdex commented on Nov 26, 2022

    @mscdex
    Contributor
    Object.keys(process.versions).sort().reduce((r, k) => (r[k] = process.versions[k], r), {});

    or

    Object.fromEntries(Object.entries(process.versions).sort((a,b)=>a<b?-1:1))
  3. MrJithil commented on Nov 27, 2022

    @MrJithil
    MemberAuthor
    ```js
    Object.keys(process.versions).sort().reduce((r, k) => (r[k] = process.versions[k], r), {});

    or

    Object.fromEntries(Object.entries(process.versions).sort((a,b)=>a<b?-1:1))

    Sorry. This is not JS. I was in an expectation like, #define will be sufficient for a programmers to understand that it's a C++ code.

    Editing the above comment.

  4. bnoordhuis commented on Nov 28, 2022

    @bnoordhuis
    Member

    Any easy ways to sort this macro in cpp?

    No, unfortunately. What you could do instead is:

    1. write out the READONLY_STRING_PROPERTY calls in src/node_process_object.cc by hand, or

    2. store the keys and values in an array and qsort/std::sort at runtime, or

    3. write a j2sc-like script that spits out code that is then incorporated in the final build

    (1) is kind of fragile and a maintenance drag. (2) feels meh. (3) increases total build time, possibly by a lot.

    A Sufficiently Smart Compiler would do the sorting in (2) at compile time. Unlikely, though.

    The runtime version string munging in src/node_metata.cc is pretty inefficient. Likely doesn't matter because we usually deserialize from snapshot but if it ran every time, that'd be a good argument for going with (3).

  5. Neustradamus commented on Dec 4, 2022

    @Neustradamus

    @MrJithil: Good point!

  6. himself65 commented on Jan 30, 2023

    @himself65
    Member

    #define V(key) \
    if (!per_process::metadata.versions.key.empty()) { \
    READONLY_STRING_PROPERTY( \
    versions, #key, per_process::metadata.versions.key); \
    }
    NODE_VERSIONS_KEYS(V)
    #undef V

  7. himself65 commented on Jan 30, 2023

    @himself65
    Member

    I think we could do here before it becomes a read-only object

    Subject: [PATCH] src: sort versions
    ---
    Index: src/node_process_object.cc
    IDEA additional info:
    Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
    <+>UTF-8
    ===================================================================
    diff --git a/src/node_process_object.cc b/src/node_process_object.cc
    --- a/src/node_process_object.cc	(revision 2a29df64645a70bbb833298423a29206c4ec6a2e)
    +++ b/src/node_process_object.cc	(date 1675098681432)
    @@ -107,7 +107,6 @@
     
       // process.versions
       Local<Object> versions = Object::New(isolate);
    -  READONLY_PROPERTY(process, "versions", versions);
     
     #define V(key)                                                                 \
       if (!per_process::metadata.versions.key.empty()) {                           \
    @@ -116,6 +115,9 @@
       }
       NODE_VERSIONS_KEYS(V)
     #undef V
    +  // todo: sort the keys except node
    +
    +  READONLY_PROPERTY(process, "versions", versions);
     
       // process.arch
       READONLY_STRING_PROPERTY(process, "arch", per_process::metadata.arch);
    
  8. himself65 commented on Jan 30, 2023

    @himself65
    Member

    See #46428

  9. added a commit that references this issue on Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions