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

deps(common/log): drop env_logger's auto-color and regex features #2373

Merged
merged 3 commits into from
Jan 20, 2025

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jan 17, 2025

#2291 added additional formatting to our log output via env_logger's auto-color feature. The auto-color feature adds the is-terminal crate dependency. is-terminal depends on hermit-abi v0.4.

Pulling latest Neqo v0.12.0 into mozilla-central adds is-terminal and hermit-abi v0.4.0 to the Firefox dependency tree.

In order to keep our dependency footprint low, I suggest not enabling the two env-logger features.

@larseggert thoughts? Alternatively I can also audit the two dependencies and then introduce them into Firefox. That said, is it worth it?

mozilla#2291 added additional formatting to our log
output via `env_logger`'s `auto-color` feature. The `auto-color` feature adds
the `is-terminal` crate dependency. `is-terminal` depends on `hermit-abi`
`v0.4`.

Pulling latest Neqo `v0.12.0` into mozilla-central adds `is-terminal` and
`hermit-abi` `v0.4.0` to the Firefox dependency tree.

In order to keep our dependency footprint low, I suggest not enabling the two
`env-logger` features.
Copy link

github-actions bot commented Jan 17, 2025

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to 108fb8d.

neqo-latest as client

neqo-latest as server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Copy link

codecov bot commented Jan 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (108fb8d) to head (f087727).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2373      +/-   ##
==========================================
- Coverage   95.31%   95.29%   -0.02%     
==========================================
  Files         114      114              
  Lines       36856    36850       -6     
  Branches    36856    36850       -6     
==========================================
- Hits        35128    35116      -12     
- Misses       1724     1728       +4     
- Partials        4        6       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@larseggert
Copy link
Collaborator

Can we condition them on cfg(not(feature = gecko))?

@mxinden
Copy link
Collaborator Author

mxinden commented Jan 20, 2025

@larseggert I don't think hiding these dependencies behind a feature flag will resolve the issue. See same scenario we faced in the past:

neqo-udp is used in Firefox. Firefox uses cargo vet to audit its dependencies. tokio, the dependency of neqo-udp, is not audited as safe-to-deploy. cargo vet will require safe-to-deploy for tokio even when behind a feature flag.

#1988

@larseggert
Copy link
Collaborator

Ah yes. Too bad. I kinda liked the color, but easy upstreaming is more important.

Cargo.toml Show resolved Hide resolved
@mxinden mxinden marked this pull request as ready for review January 20, 2025 15:11
@mxinden
Copy link
Collaborator Author

mxinden commented Jan 20, 2025

🎉 with mozilla/mtu#74 and this patch, we are down to the following changes:

        new file:   third_party/rust/mtu/.cargo-checksum.json
        new file:   third_party/rust/mtu/CODE_OF_CONDUCT.md
        new file:   third_party/rust/mtu/Cargo.lock
        new file:   third_party/rust/mtu/Cargo.toml
        new file:   third_party/rust/mtu/LICENSE-APACHE
        new file:   third_party/rust/mtu/LICENSE-MIT
        new file:   third_party/rust/mtu/README.md
        new file:   third_party/rust/mtu/SECURITY.md
        new file:   third_party/rust/mtu/build.rs
        new file:   third_party/rust/mtu/src/bsd.rs
        new file:   third_party/rust/mtu/src/lib.rs
        new file:   third_party/rust/mtu/src/linux.rs
        new file:   third_party/rust/mtu/src/routesocket.rs
        new file:   third_party/rust/mtu/src/windows.rs
        modified:   third_party/rust/neqo-bin/.cargo-checksum.json
        modified:   third_party/rust/neqo-bin/Cargo.toml
        modified:   third_party/rust/neqo-bin/benches/main.rs
        modified:   third_party/rust/neqo-bin/src/bin/client.rs
        modified:   third_party/rust/neqo-bin/src/bin/server.rs
        modified:   third_party/rust/neqo-bin/src/client/http09.rs
        modified:   third_party/rust/neqo-bin/src/client/http3.rs
        modified:   third_party/rust/neqo-bin/src/client/mod.rs
        modified:   third_party/rust/neqo-bin/src/lib.rs
        modified:   third_party/rust/neqo-bin/src/server/http09.rs
        modified:   third_party/rust/neqo-bin/src/server/http3.rs
        modified:   third_party/rust/neqo-bin/src/server/mod.rs
        modified:   third_party/rust/neqo-bin/src/udp.rs
        modified:   third_party/rust/neqo-common/.cargo-checksum.json
        modified:   third_party/rust/neqo-common/Cargo.toml
        new file:   third_party/rust/neqo-common/benches/decoder.rs
        modified:   third_party/rust/neqo-common/src/codec.rs
        modified:   third_party/rust/neqo-common/src/fuzz.rs
        modified:   third_party/rust/neqo-common/src/header.rs
        modified:   third_party/rust/neqo-common/src/hrtime.rs
        modified:   third_party/rust/neqo-common/src/incrdecoder.rs
        modified:   third_party/rust/neqo-common/src/lib.rs
        modified:   third_party/rust/neqo-common/src/log.rs
        modified:   third_party/rust/neqo-common/src/qlog.rs
        modified:   third_party/rust/neqo-common/src/tos.rs
        modified:   third_party/rust/neqo-common/tests/log.rs
        modified:   third_party/rust/neqo-crypto/.cargo-checksum.json
        modified:   third_party/rust/neqo-crypto/Cargo.toml
        modified:   third_party/rust/neqo-crypto/src/aead.rs
        modified:   third_party/rust/neqo-crypto/src/agent.rs
        modified:   third_party/rust/neqo-crypto/src/agentio.rs
        modified:   third_party/rust/neqo-crypto/src/cert.rs
        modified:   third_party/rust/neqo-crypto/src/ech.rs
        modified:   third_party/rust/neqo-crypto/src/lib.rs
        modified:   third_party/rust/neqo-crypto/src/p11.rs
        modified:   third_party/rust/neqo-crypto/src/result.rs
        modified:   third_party/rust/neqo-crypto/src/secrets.rs
        modified:   third_party/rust/neqo-crypto/src/selfencrypt.rs
        modified:   third_party/rust/neqo-crypto/src/time.rs
        modified:   third_party/rust/neqo-crypto/tests/handshake.rs
        modified:   third_party/rust/neqo-crypto/tests/hp.rs
        modified:   third_party/rust/neqo-http3/.cargo-checksum.json
        modified:   third_party/rust/neqo-http3/Cargo.toml
        modified:   third_party/rust/neqo-http3/src/buffered_send_stream.rs
        modified:   third_party/rust/neqo-http3/src/client_events.rs
        modified:   third_party/rust/neqo-http3/src/connection.rs
        modified:   third_party/rust/neqo-http3/src/connection_client.rs
        modified:   third_party/rust/neqo-http3/src/connection_server.rs
        modified:   third_party/rust/neqo-http3/src/control_stream_local.rs
        modified:   third_party/rust/neqo-http3/src/control_stream_remote.rs
        modified:   third_party/rust/neqo-http3/src/features/extended_connect/tests/webtransport/mod.rs
        modified:   third_party/rust/neqo-http3/src/features/extended_connect/tests/webtransport/negotiation.rs
        modified:   third_party/rust/neqo-http3/src/features/extended_connect/tests/webtransport/sessions.rs
        modified:   third_party/rust/neqo-http3/src/features/extended_connect/tests/webtransport/streams.rs
        modified:   third_party/rust/neqo-http3/src/features/extended_connect/webtransport_session.rs
        modified:   third_party/rust/neqo-http3/src/features/mod.rs
        modified:   third_party/rust/neqo-http3/src/frames/hframe.rs
        modified:   third_party/rust/neqo-http3/src/frames/reader.rs
        modified:   third_party/rust/neqo-http3/src/frames/tests/hframe.rs
        modified:   third_party/rust/neqo-http3/src/frames/tests/mod.rs
        modified:   third_party/rust/neqo-http3/src/frames/tests/reader.rs
        modified:   third_party/rust/neqo-http3/src/frames/wtframe.rs
        modified:   third_party/rust/neqo-http3/src/headers_checks.rs
        modified:   third_party/rust/neqo-http3/src/lib.rs
        modified:   third_party/rust/neqo-http3/src/push_controller.rs
        new file:   third_party/rust/neqo-http3/src/push_id.rs
        modified:   third_party/rust/neqo-http3/src/recv_message.rs
        modified:   third_party/rust/neqo-http3/src/send_message.rs
        modified:   third_party/rust/neqo-http3/src/server.rs
        modified:   third_party/rust/neqo-http3/src/server_events.rs
        modified:   third_party/rust/neqo-http3/src/stream_type_reader.rs
        modified:   third_party/rust/neqo-http3/tests/httpconn.rs
        modified:   third_party/rust/neqo-http3/tests/priority.rs
        modified:   third_party/rust/neqo-http3/tests/send_message.rs
        modified:   third_party/rust/neqo-http3/tests/webtransport.rs
        modified:   third_party/rust/neqo-qpack/.cargo-checksum.json
        modified:   third_party/rust/neqo-qpack/Cargo.toml
        modified:   third_party/rust/neqo-qpack/src/decoder.rs
        modified:   third_party/rust/neqo-qpack/src/decoder_instructions.rs
        modified:   third_party/rust/neqo-qpack/src/encoder.rs
        modified:   third_party/rust/neqo-qpack/src/encoder_instructions.rs
        modified:   third_party/rust/neqo-qpack/src/header_block.rs
        modified:   third_party/rust/neqo-qpack/src/prefix.rs
        modified:   third_party/rust/neqo-qpack/src/qpack_send_buf.rs
        modified:   third_party/rust/neqo-qpack/src/reader.rs
        modified:   third_party/rust/neqo-qpack/src/stats.rs
        modified:   third_party/rust/neqo-qpack/src/table.rs
        modified:   third_party/rust/neqo-transport/.cargo-checksum.json
        modified:   third_party/rust/neqo-transport/Cargo.toml
        modified:   third_party/rust/neqo-transport/src/ackrate.rs
        modified:   third_party/rust/neqo-transport/src/addr_valid.rs
        modified:   third_party/rust/neqo-transport/src/cc/classic_cc.rs
        modified:   third_party/rust/neqo-transport/src/cc/cubic.rs
        modified:   third_party/rust/neqo-transport/src/cc/tests/cubic.rs
        modified:   third_party/rust/neqo-transport/src/cc/tests/mod.rs
        modified:   third_party/rust/neqo-transport/src/cc/tests/new_reno.rs
        modified:   third_party/rust/neqo-transport/src/cid.rs
        modified:   third_party/rust/neqo-transport/src/connection/dump.rs
        modified:   third_party/rust/neqo-transport/src/connection/idle.rs
        modified:   third_party/rust/neqo-transport/src/connection/mod.rs
        modified:   third_party/rust/neqo-transport/src/connection/params.rs
        modified:   third_party/rust/neqo-transport/src/connection/state.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/ackrate.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/cc.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/datagram.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/ecn.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/handshake.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/idle.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/keys.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/migration.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/mod.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/priority.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/recovery.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/resumption.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/stream.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/vn.rs
        modified:   third_party/rust/neqo-transport/src/connection/tests/zerortt.rs
        modified:   third_party/rust/neqo-transport/src/crypto.rs
        modified:   third_party/rust/neqo-transport/src/ecn.rs
        modified:   third_party/rust/neqo-transport/src/events.rs
        modified:   third_party/rust/neqo-transport/src/fc.rs
        modified:   third_party/rust/neqo-transport/src/frame.rs
        modified:   third_party/rust/neqo-transport/src/lib.rs
        modified:   third_party/rust/neqo-transport/src/pace.rs
        modified:   third_party/rust/neqo-transport/src/packet/mod.rs
        modified:   third_party/rust/neqo-transport/src/packet/retry.rs
        modified:   third_party/rust/neqo-transport/src/path.rs
        modified:   third_party/rust/neqo-transport/src/pmtud.rs
        modified:   third_party/rust/neqo-transport/src/qlog.rs
        modified:   third_party/rust/neqo-transport/src/quic_datagrams.rs
        modified:   third_party/rust/neqo-transport/src/recovery/mod.rs
        modified:   third_party/rust/neqo-transport/src/recovery/sent.rs
        modified:   third_party/rust/neqo-transport/src/recv_stream.rs
        modified:   third_party/rust/neqo-transport/src/rtt.rs
        modified:   third_party/rust/neqo-transport/src/send_stream.rs
        modified:   third_party/rust/neqo-transport/src/sender.rs
        modified:   third_party/rust/neqo-transport/src/server.rs
        new file:   third_party/rust/neqo-transport/src/shuffle.rs
        modified:   third_party/rust/neqo-transport/src/stats.rs
        modified:   third_party/rust/neqo-transport/src/stream_id.rs
        modified:   third_party/rust/neqo-transport/src/streams.rs
        modified:   third_party/rust/neqo-transport/src/tparams.rs
        modified:   third_party/rust/neqo-transport/src/tracking.rs
        modified:   third_party/rust/neqo-transport/src/version.rs
        modified:   third_party/rust/neqo-transport/tests/common/mod.rs
        modified:   third_party/rust/neqo-transport/tests/connection.rs
        modified:   third_party/rust/neqo-transport/tests/retry.rs
        modified:   third_party/rust/neqo-transport/tests/server.rs
        modified:   third_party/rust/neqo-udp/.cargo-checksum.json
        modified:   third_party/rust/neqo-udp/Cargo.toml
        new file:   third_party/rust/neqo-udp/build.rs
        modified:   third_party/rust/neqo-udp/src/lib.rs

@mxinden mxinden added this pull request to the merge queue Jan 20, 2025
Merged via the queue into mozilla:main with commit 51ce9d6 Jan 20, 2025
66 of 73 checks passed
@mxinden mxinden deleted the env-logger-features branch January 20, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants