Patchwork [4,of,6] rust-chg: add struct holding information needed to spawn server process

login
register
mail settings
Submitter Yuya Nishihara
Date Oct. 14, 2018, 8:18 a.m.
Message ID <83ae46b307d87085a0a0.1539505101@mimosa>
Download mbox | patch
Permalink /patch/35957/
State Accepted
Headers show

Comments

Yuya Nishihara - Oct. 14, 2018, 8:18 a.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1538828039 -32400
#      Sat Oct 06 21:13:59 2018 +0900
# Node ID 83ae46b307d87085a0a0f8224f021adf7788050d
# Parent  efdf82aa259682f10c9211e283aceffd861f77cd
rust-chg: add struct holding information needed to spawn server process

The Locator will handle the initialization of the connection. It will spawn
server processes as needed.
Gregory Szorc - Oct. 14, 2018, 11:48 a.m.
On Sun, Oct 14, 2018 at 10:29 AM Yuya Nishihara <yuya@tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1538828039 -32400
> #      Sat Oct 06 21:13:59 2018 +0900
> # Node ID 83ae46b307d87085a0a0f8224f021adf7788050d
> # Parent  efdf82aa259682f10c9211e283aceffd861f77cd
> rust-chg: add struct holding information needed to spawn server process
>
> The Locator will handle the initialization of the connection. It will spawn
> server processes as needed.
>
> diff --git a/rust/chg/src/locator.rs b/rust/chg/src/locator.rs
> --- a/rust/chg/src/locator.rs
> +++ b/rust/chg/src/locator.rs
> @@ -6,13 +6,54 @@
>  //! Utility for locating command-server process.
>
>  use std::env;
> +use std::ffi::{OsStr, OsString};
>  use std::fs::{self, DirBuilder};
>  use std::io;
> +use std::os::unix::ffi::{OsStrExt, OsStringExt};
>  use std::os::unix::fs::{DirBuilderExt, MetadataExt};
>  use std::path::{Path, PathBuf};
> +use std::process;
> +use std::time::Duration;
>
>  use super::procutil;
>
> +/// Helper to connect to and spawn a server process.
> +#[derive(Clone, Debug)]
> +pub struct Locator {
> +    hg_command: OsString,
> +    current_dir: PathBuf,
> +    env_vars: Vec<(OsString, OsString)>,
> +    process_id: u32,
> +    base_sock_path: PathBuf,
> +    timeout: Duration,
> +}
> +
> +impl Locator {
> +    /// Creates locator capturing the current process environment.
> +    ///
> +    /// If no `$CHGSOCKNAME` is specified, the socket directory will be
> +    /// created as necessary.
> +    pub fn prepare_from_env() -> io::Result<Locator> {
> +        Ok(Locator {
> +            hg_command: default_hg_command(),
> +            current_dir: env::current_dir()?,
> +            env_vars: env::vars_os().collect(),
> +            process_id: process::id(),
> +            base_sock_path: prepare_server_socket_path()?,
> +            timeout: default_timeout(),
> +        })
> +    }
> +
> +    /// Temporary socket path for this client process.
> +    fn temp_sock_path(&self) -> PathBuf {
> +        let src = self.base_sock_path.as_os_str().as_bytes();
> +        let mut buf = Vec::with_capacity(src.len() + 6);
>

This magic number to hold the formatted pid scared me a bit. But Rust
should guarantee memory safety. I wish this could be expressed in terms of
the size of another variable that will occupy the space. But meh.


> +        buf.extend_from_slice(src);
> +        buf.extend_from_slice(format!(".{}", self.process_id).as_bytes());
> +        OsString::from_vec(buf).into()
> +    }
> +}
> +
>  /// Determines the server socket to connect to.
>  ///
>  /// If no `$CHGSOCKNAME` is specified, the socket directory will be
> created
> @@ -47,6 +88,17 @@ pub fn default_server_socket_dir() -> Pa
>      }
>  }
>
> +/// Determines the default hg command.
> +pub fn default_hg_command() -> OsString {
> +    // TODO: maybe allow embedding the path at compile time (or load from
> hgrc)
> +
> env::var_os("CHGHG").or(env::var_os("HG")).unwrap_or(OsStr::new("hg").to_owned())
> +}
> +
> +fn default_timeout() -> Duration {
> +    let secs = env::var("CHGTIMEOUT").ok().and_then(|s|
> s.parse().ok()).unwrap_or(60);
> +    Duration::from_secs(secs)
> +}
> +
>  /// Creates a directory which the other users cannot access to.
>  ///
>  /// If the directory already exists, tests its permission.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Yuya Nishihara - Oct. 14, 2018, 12:34 p.m.
On Sun, 14 Oct 2018 13:48:31 +0200, Gregory Szorc wrote:
> On Sun, Oct 14, 2018 at 10:29 AM Yuya Nishihara <yuya@tcha.org> wrote:
> > +    /// Temporary socket path for this client process.
> > +    fn temp_sock_path(&self) -> PathBuf {
> > +        let src = self.base_sock_path.as_os_str().as_bytes();
> > +        let mut buf = Vec::with_capacity(src.len() + 6);
> 
> This magic number to hold the formatted pid scared me a bit. But Rust
> should guarantee memory safety. I wish this could be expressed in terms of
> the size of another variable that will occupy the space. But meh.

It's just the initial allocation size, so I didn't care much. Perhaps, I
can add some inline comment.

Patch

diff --git a/rust/chg/src/locator.rs b/rust/chg/src/locator.rs
--- a/rust/chg/src/locator.rs
+++ b/rust/chg/src/locator.rs
@@ -6,13 +6,54 @@ 
 //! Utility for locating command-server process.
 
 use std::env;
+use std::ffi::{OsStr, OsString};
 use std::fs::{self, DirBuilder};
 use std::io;
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
 use std::os::unix::fs::{DirBuilderExt, MetadataExt};
 use std::path::{Path, PathBuf};
+use std::process;
+use std::time::Duration;
 
 use super::procutil;
 
+/// Helper to connect to and spawn a server process.
+#[derive(Clone, Debug)]
+pub struct Locator {
+    hg_command: OsString,
+    current_dir: PathBuf,
+    env_vars: Vec<(OsString, OsString)>,
+    process_id: u32,
+    base_sock_path: PathBuf,
+    timeout: Duration,
+}
+
+impl Locator {
+    /// Creates locator capturing the current process environment.
+    ///
+    /// If no `$CHGSOCKNAME` is specified, the socket directory will be
+    /// created as necessary.
+    pub fn prepare_from_env() -> io::Result<Locator> {
+        Ok(Locator {
+            hg_command: default_hg_command(),
+            current_dir: env::current_dir()?,
+            env_vars: env::vars_os().collect(),
+            process_id: process::id(),
+            base_sock_path: prepare_server_socket_path()?,
+            timeout: default_timeout(),
+        })
+    }
+
+    /// Temporary socket path for this client process.
+    fn temp_sock_path(&self) -> PathBuf {
+        let src = self.base_sock_path.as_os_str().as_bytes();
+        let mut buf = Vec::with_capacity(src.len() + 6);
+        buf.extend_from_slice(src);
+        buf.extend_from_slice(format!(".{}", self.process_id).as_bytes());
+        OsString::from_vec(buf).into()
+    }
+}
+
 /// Determines the server socket to connect to.
 ///
 /// If no `$CHGSOCKNAME` is specified, the socket directory will be created
@@ -47,6 +88,17 @@  pub fn default_server_socket_dir() -> Pa
     }
 }
 
+/// Determines the default hg command.
+pub fn default_hg_command() -> OsString {
+    // TODO: maybe allow embedding the path at compile time (or load from hgrc)
+    env::var_os("CHGHG").or(env::var_os("HG")).unwrap_or(OsStr::new("hg").to_owned())
+}
+
+fn default_timeout() -> Duration {
+    let secs = env::var("CHGTIMEOUT").ok().and_then(|s| s.parse().ok()).unwrap_or(60);
+    Duration::from_secs(secs)
+}
+
 /// Creates a directory which the other users cannot access to.
 ///
 /// If the directory already exists, tests its permission.