[ELPA] New package: nspawn-tramp

[ELPA] New package: nspawn-tramp

[Brian Cully]

	This is a package that allows TRAMP to use systemd-nspawn based 
containers (typically used with machinectl).

	I've been dogfooding this myself for about a year now, and I hope that 
others in the community will find it useful.

	The package source can be found at: https://github.com/bjc/nspawn-tramp

	If it gets accepted to ELPA, I'll adjust the README.org to include 
instructions for how to install it from there.

-bjc

Re: [ELPA] New package: nspawn-tramp

[Stefan Monnier]

> 	This is a package that allows TRAMP to use systemd-nspawn based
> 	containers (typically used with machinectl).
>
> 	I've been dogfooding this myself for about a year now, and I hope
> 	that others in the community will find it useful.
>
> 	The package source can be found at: https://github.com/bjc/nspawn-tramp
>
> 	If it gets accepted to ELPA, I'll adjust the README.org to include
> 	instructions for how to install it from there.

I'd be happy to add it to GNU ELPA, but I have a few questions/comments
some of which may affect this decision, so I haven't done it yet:

- It's usually spelled "Tramp" rather than "TRAMP", AFAIK.

- I suggest you clarify that this is not about Tramp using (internally)
  nspawnd to increase its security.  So instead of "allows TRAMP to work
  with containers", I'd say something like "teaches Tramp to access
  nspawnd containers"?

- `nspawnd-tramp` suggests this adds Tramp support to nspawnd rather than
  the reverse.

- Any reason why you want to have it as a separate package rather than
  add it to Tramp?

- `nspawn-tramp-machinectl-path` does not hold a "path" (like $PATH and
  `load-path`) but a "file name", please follow the GNU convention.


        Stefan


diff --git a/nspawn-tramp.el b/nspawn-tramp.el
index e5233406fe..9b1cb0c795 100644
--- a/nspawn-tramp.el
+++ b/nspawn-tramp.el
@@ -1,6 +1,6 @@
 ;;; nspawn-tramp.el -- TRAMP integration for systemd-nspawn containers  -*- lexical-binding: t; -*-
 
-;; Copyright © 2021 Free Software Foundation, Inc.
+;; Copyright © 2021-2022 Free Software Foundation, Inc.
 
 ;; Author: Brian Cully <bjc@kublai.com>
 ;; Maintainer: Brian Cully <bjc@kublai.com>
@@ -39,11 +39,11 @@
 ;;
 ;; Open a file on a running systemd-nspawn container:
 ;;
-;;     C-x C-f /nspawn:user@container:/path/to/file
+;;     C-x C-f /nspawn:USER@CONTAINER:/path/to/file
 ;;
 ;; Where:
-;;     ‘user’          is the user on the container to connect as (optional)
-;;     ‘container’     is the container to connect to
+;;     USER          is the user on the container to connect as (optional)
+;;     CONTAINER     is the container to connect to
 ;;
 ;; ## Privileges
 ;;
@@ -70,7 +70,7 @@
   :link '(emacs-commentary-link :tag "Commentary" "nspawn-tramp"))
 
 (defcustom nspawn-tramp-machinectl-path "machinectl"
-  "Path to machinectl executable."
+  "File name of machinectl executable."
   :type 'string
   :group 'nspawn-tramp)
 
@@ -93,7 +93,7 @@ see its function help for a description of the format."
 
 
 (defun nspawn-tramp--add-method ()
-  "Add TRAMP method handler for nspawn conainers."
+  "Add TRAMP method handler for nspawn containers."
   (push `(,nspawn-tramp-method
           (tramp-login-program ,nspawn-tramp-machinectl-path)
           (tramp-login-args (("shell")

Re: [ELPA] New package: nspawn-tramp

[Andreas Schwab]

On Feb 19 2022, Stefan Monnier wrote:

> - `nspawn-tramp-machinectl-path` does not hold a "path" (like $PATH and
>   `load-path`) but a "file name", please follow the GNU convention.

Or rather a command or program.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [ELPA] New package: nspawn-tramp

[Brian Cully]

On 2/19/22 10:05, Stefan Monnier wrote:
> I'd be happy to add it to GNU ELPA, but I have a few questions/comments
> some of which may affect this decision, so I haven't done it yet:
> 
> - It's usually spelled "Tramp" rather than "TRAMP", AFAIK.

	The info page for it has it spelled as "TRAMP", although the link to it 
from the list in the top-level (from "M-x info") has it as "Tramp". I 
assume the former is correct given its specificity.

> 
> - I suggest you clarify that this is not about Tramp using (internally)
>    nspawnd to increase its security.  So instead of "allows TRAMP to work
>    with containers", I'd say something like "teaches Tramp to access
>    nspawnd containers"?

	What do you think of "... allows TRAMP access to environments provided 
by systemd-nspawn"? I'll admit I'm having a hard time coming up with 
language that addresses your concern and sounds natural.

> - `nspawnd-tramp` suggests this adds Tramp support to nspawnd rather than
>    the reverse.

	I did, initially, call it "tramp-nspawn", but changed it after 
reviewing other packages which added container support to TRAMP, like 
"docker-tramp" and "lxc-tramp". It doesn't matter much to me, so I went 
with what seemed like existing convention.

> - Any reason why you want to have it as a separate package rather than
>    add it to Tramp?

	TRAMP provides a plugin capability, which is being used by other 
packages, and it's already a behemoth. I see no need to include this 
directly within TRAMP itself, as its needs are perfectly addressed without.

> - `nspawn-tramp-machinectl-path` does not hold a "path" (like $PATH and
>    `load-path`) but a "file name", please follow the GNU convention.

	I've renamed the variable to `nspawn-tramp-machinectl-file-name` and 
adjusted its documentation.

	Thank you for your patch. I've applied them and will push them up to my 
repository when the current discussion concludes.

Re: [ELPA] New package: nspawn-tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

> On 2/19/22 10:05, Stefan Monnier wrote:
>> I'd be happy to add it to GNU ELPA, but I have a few questions/comments
>> some of which may affect this decision, so I haven't done it yet:
>> - It's usually spelled "Tramp" rather than "TRAMP", AFAIK.
>
> 	The info page for it has it spelled as "TRAMP", although the
> 	link to it from the list in the top-level (from "M-x info")
> 	has it as "Tramp". I assume the former is correct given its
> 	specificity.

In tramp.texi, we use @value{tramp}. This variable is declared in
trampver.texi as

--8<---------------cut here---------------start------------->8---
@set tramp @sc{Tramp}
--8<---------------cut here---------------end--------------->8---

Therefore, it appears in info pages as "TRAMP". In Elisp docstrings and
comments, we use "Tramp". It looks a little bit inconsistent, but it is
... history. This texinfo variable was declared long before I've ever
heard about Tramp.

>> - Any reason why you want to have it as a separate package rather than
>>    add it to Tramp?
>
> 	TRAMP provides a plugin capability, which is being used by
> 	other packages, and it's already a behemoth. I see no need to
> 	include this directly within TRAMP itself, as its needs are
> 	perfectly addressed without.

I understand your ressentiments. OTOH, living outside the Tramp source
tree let you miss changes. For example, Tramp meanwhile doesn't hardcode
"/bin/sh" any longer, but uses tramp-default-remote-shell instead. Not a
big deal for now, but these little adaptions make a Tramp package more
consistent, because you would get such a change for free if your package
is bundled with Tramp directly.

Best regards, Michael.

Re: [ELPA] New package: nspawn-tramp

[Brian Cully]

On 2/19/22 13:11, Michael Albinus wrote:
> Therefore, it appears in info pages as "TRAMP". In Elisp docstrings and
> comments, we use "Tramp". It looks a little bit inconsistent, but it is
> ... history. This texinfo variable was declared long before I've ever
> heard about Tramp.

	Ok, I can change it to "Tramp". Thanks for filling me in.

>>> - Any reason why you want to have it as a separate package rather than
>>>     add it to Tramp?
>>
>> 	TRAMP provides a plugin capability, which is being used by
>> 	other packages, and it's already a behemoth. I see no need to
>> 	include this directly within TRAMP itself, as its needs are
>> 	perfectly addressed without.
> 
> I understand your ressentiments. OTOH, living outside the Tramp source
> tree let you miss changes. For example, Tramp meanwhile doesn't hardcode
> "/bin/sh" any longer, but uses tramp-default-remote-shell instead. Not a
> big deal for now, but these little adaptions make a Tramp package more
> consistent, because you would get such a change for free if your package
> is bundled with Tramp directly.

	All the same, I think it makes more sense for this package to live 
separately. In general, I much prefer to use plug-in mechanisms for 
extension when possible, as I believe the separation of concerns to lead 
to a more overall maintainable system. In this particular case, I think 
the functionality I'm providing is fairly niche. Even in the container 
space, systemd-nspawn containers aren't particularly popular.

	On a personal note, I'm not willing to maintain this code if it's 
integrated directly into Tramp, because I don't feel that I currently 
have the needed expertise to do so. I've assigned copyright to the FSF, 
however, so if someone wants to take the code and integrate it with 
Tramp, rather than maintain a separate package, they are free to do so 
and have my blessing.

-bjc

Re: [ELPA] New package: nspawn-tramp

[Michael Albinus]

Brian Cully <bjc@spork.org> writes:

Hi Brian,

> 	On a personal note, I'm not willing to maintain this code if
> 	it's integrated directly into Tramp, because I don't feel that
> 	I currently have the needed expertise to do so.

Well, this is not mandatory for you. There are Tramp citizens, which were
written by somebody else, and which are maintained by the Tramp
maintainer now. See for example tramp-adb.el.

> 	I've assigned copyright to the FSF, however, so if someone wants
> 	to take the code and integrate it with Tramp, rather than
> 	maintain a separate package, they are free to do so and have my
> 	blessing.

It's a while ago that I have played with containers. So I don't believe
I will be the guy who puts this package into Tramp proper. Personally, I
can live with both approaches, an integrated package or a GNU ELPA
package.

> -bjc

Best regards, Michael.

Re: [ELPA] New package: nspawn-tramp

[Stefan Monnier]

> It's a while ago that I have played with containers. So I don't believe
> I will be the guy who puts this package into Tramp proper. Personally, I
> can live with both approaches, an integrated package or a GNU ELPA
> package.

It should appear on GNU ELPA soonish.


        Stefan