This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8357

rust-chg: add brief comment about initial capacity of temp_sock_path()
ClosedPublic

Authored by yuja on Apr 2 2020, 7:12 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGc11d98cff883: rust-chg: add brief comment about initial capacity of temp_sock_path()
Summary

I don't know if it can be expressed as a compile-time constant, so it's
a comment for now.

About this series:
This is quite old patches for rust-chg. I heard from Octobus people that
there's a plan to do an experiment on merging hgcli + chg + some Rust?,
so I decided to respin the rust-chg series.

Maybe we'll rewrite the core to leverage the recent async/await functionality,
but I want to first make my old patches in so the rust-chg can be a drop-in
replacement for the chg of C. Compiler warnings will be removed later, and
the codebase will be upgraded to the 2018 edition later.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yuja created this revision.Apr 2 2020, 7:12 AM
Alphare accepted this revision.Apr 3 2020, 8:18 AM
Alphare added a subscriber: Alphare.

(I'll be re-pasting all of my comments that got lost in emails for this series)

Maybe I'm completely off-base here, but pid length varies with the OS, the architecture and user preferences. On Linux 64 bits it can be 4194304, 32768 for 32bits and 99999 for FreeBSD. We could add constants, but I feel like it would be a lot of work for not a lot of gain.

This patch looks good no matter what.

Alphare added a comment.Apr 3 2020, 8:25 AM

This series is great.

In case anyone wants to give their opinion, I have suggested to Yuya to start using async_std now that it's post-1.0 instead of tokio. It's made by tokio devs and is supposedly the better designed of the two as it gained hindsight from the different tokio/futures versions of the past few years.

yuja added a commit: rHGc11d98cff883: rust-chg: add brief comment about initial capacity of temp_sock_path().Apr 3 2020, 3:17 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGc11d98cff883: rust-chg: add brief comment about initial capacity of temp_sock_path() (authored by yuja). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/chg/src/locator.rs (2 lines)

Diff 20986

rust/chg/src/locator.rs

base_sock_path: prepare_server_socket_path()?, base_sock_path: prepare_server_socket_path()?,
timeout: default_timeout(), timeout: default_timeout(),
}) })
} }
/// Temporary socket path for this client process. /// Temporary socket path for this client process.
fn temp_sock_path(&self) -> PathBuf { fn temp_sock_path(&self) -> PathBuf {
let src = self.base_sock_path.as_os_str().as_bytes(); let src = self.base_sock_path.as_os_str().as_bytes();
let mut buf = Vec::with_capacity(src.len() + 6); let mut buf = Vec::with_capacity(src.len() + 6); // "{src}.{pid}".len()
buf.extend_from_slice(src); buf.extend_from_slice(src);
buf.extend_from_slice(format!(".{}", self.process_id).as_bytes()); buf.extend_from_slice(format!(".{}", self.process_id).as_bytes());
OsString::from_vec(buf).into() OsString::from_vec(buf).into()
} }
} }
/// Determines the server socket to connect to. /// Determines the server socket to connect to.
/// ///