Rewrite in Go #318
Rewrite in Go #318
Conversation
|
So first: Let's try to get a vote among contributors here - do you really really want Go? I think the code in #319 is pretty good, Rust's "structopt" is a lot nicer than anything Go can do, etc. But, if you don't know Rust (and don't have the time to learn), that's a totally valid argument against. Go is very easy to learn. It's also valid that podman is in Go. But I'd hope people at least take a careful look at the toolbox-rs code and think about it! But I write Go a lot, and I just really dislike the language. The "Go strength" in web services doesn't apply here at all. On the other hand, hopefully this project doesn't exceed 2000 lines, so it doesn't matter too much. |
I really want to learn more rust and I like the concept of the language.
I think these points are the biggest one in favor of Go and why we have centered on go in previous discussions.
+1 - I looked at it earlier today. I don't quite understand it, because I don't know rust, but it at least looks nice.
Understood.
Agree, Either one should be fine implmentations for this. We should hoist advanced features back into podman. |
|
Switching to either opens opportunities for me to contribute to the code (I can't write bash). However, toolbox not in a vacuum and entire container domain is almost Go-exclusive, which is not exactly good, but means moving toolbox to Go would attract more contributions from people that already work on container projects and also new people (Go is really easy to get started with, after all). Podman being in Go is really important point. Still, it seems to me an even better option is a scripting/interpreted language. As a wrapper around a complete container solution (podman), I'm not sure what benefit there will be from features of a compiled language like Rust or Go (performance, advanced memory management, complex data representations, direct system calls, etc.). I would make many points in favor of Python, but as I understood because of CoreOS it's a no-go. Is it the case? |
HarryMichal
added
5. Help Wanted
It is true that the CoreOS team has a goal of keeping Python out of the base. See https://github.com/coreos/fedora-coreos-tracker/blob/master/Design.md#approach-towards-shipping-python |
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
e667f44
to
1501b09
Compare
I see, thanks for the reference. Python is not an option then (unless you feel like freezing binaries) :( So to me it seems Go has more advantages if starting from scratch. At the same time there is cgwalters/coretoolbox which is an already functional implementation. Between the two, I'm not sure what I would choose. Very curious to see how this decision goes. |
|
If you pick Go, I'd also suggest to use podman directly from the CLI, or alternatively through varlink. The |
Yes. Agree. This is something we had previously discussed as well and we agreed to not vendor libpod. |
HarryMichal
force-pushed
the
rewrite-in-go
branch
5 times, most recently
from
9542708
to
58e3dac
Compare
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
58e3dac
to
e00462d
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
004df35
to
1d53152
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
1d53152
to
43230a4
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
43230a4
to
462abda
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
HarryMichal
force-pushed
the
rewrite-in-go
branch
from
462abda
to
1599d7d
Compare
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
1 similar comment
|
Merge Failed. This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset. |
For some reason, the State:Pid value in the JSON returned by 'podman inspect --format json --type container' is a float64 [1], even though internally Podman represents it as an int. [1] containers/podman#6105 containers#318
The Go version supports specifying the name of the toolbox container as 'toolbox enter NAME', on top of the 'toolbox enter --container NAME' form. containers#318
The Go version supports specifying the name of the new toolbox container as 'toolbox create NAME', in addition to the existing 'toolbox create --container NAME' form. containers#318
For some reason, the State:Pid value in the JSON returned by 'podman inspect --format json --type container' is a float64 [1], even though internally Podman represents it as an int. [1] containers/podman#6105 containers#318
The Go version supports specifying the name of the toolbox container as 'toolbox enter NAME', on top of the 'toolbox enter --container NAME' form. containers#318
debarshiray
force-pushed
the
rewrite-in-go
branch
from
a60e4a3
to
ebd92b9
Compare
|
No way! :D |
The concept of a 'base' image for creating Toolbx containers hasn't existed since Toolbx 0.0.10, when the Buildah dependency and the user-specific customized image were dropped [1]. That was a long time ago. Toolbx containers created before that were never supported by the Go implementation [2]. Therefore, it's time to update the terminology. Only the images for currently maintained Fedoras (ie., 38 and 39) were updated. [1] Commit 8b84b5e containers@8b84b5e4604921fa containers#160 [2] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1456
The concept of a 'base' image for creating Toolbx containers hasn't existed since Toolbx 0.0.10, when the Buildah dependency and the user-specific customized image were dropped [1]. That was a long time ago. Toolbx containers created before that were never supported by the Go implementation [2]. Therefore, it's time to update the terminology. [1] Commit 8b84b5e containers@8b84b5e4604921fa containers#160 [2] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1456
The concept of a 'base' image for creating Toolbx containers hasn't existed since Toolbx 0.0.10, when the Buildah dependency and the user-specific customized image were dropped [1]. That was a long time ago. Toolbx containers created before that were never supported by the Go implementation [2]. Therefore, it's time to update the terminology. [1] Commit 8b84b5e containers@8b84b5e4604921fa containers#160 [2] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1456
Unmarshal the JSON from 'podman inspect --format json --type container' directly inside podman.InspectContainer() to confine the details within the podman package. The JSON samples for the unit tests were taken using the default Toolbx container on versions of Fedora that shipped a specific Podman and Toolbx version. This accounts for differences in the JSON output caused by different major versions of Podman and the way different Toolbx versions set up the containers over time. The minimum required Podman version is 1.6.4 [1], and the Go implementation has been encouraging users to create containers with Toolbx version 0.0.17 or newer [2]. The versions used to collect the JSON samples for the unit tests were chosen accordingly. They don't exhaustively cover all supported version combinations, but hopefully cover enough to be useful. [1] Commit 8e80dd5 containers@8e80dd5db1e6f40b containers#1253 [2] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1490
Unmarshal the JSON from 'podman inspect --format json --type container' directly inside podman.InspectContainer() to confine the details within the podman package. The JSON samples for the unit tests were taken using the default Toolbx container on versions of Fedora that shipped a specific Podman and Toolbx version. This accounts for differences in the JSON caused by different major versions of Podman and the way different Toolbx versions set up the containers. One exception was Fedora 28, which had Podman 1.1.2 and Toolbx 0.0.9, which is the last Toolbx version before 'toolbox init-container' became the entry point for all Toolbx containers [1]. However, the default Toolbx image is no longer available from registry.fedoraproject.org. Hence, the image for Fedora 29 was used. The minimum required Podman version is 1.6.4 [2], and the Go implementation has been encouraging users to create containers with Toolbx version 0.0.17 or newer [3]. The versions used to collect the JSON samples for the unit tests were chosen accordingly. They don't exhaustively cover all possible supported and unsupported version combinations, but hopefully enough to be useful. [1] Commit 8b84b5e containers@8b84b5e4604921fa https://github.com/debarshiray/toolbox/pull/160 [2] Commit 8e80dd5 containers@8e80dd5db1e6f40b containers#1253 [3] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1490
Unmarshal the JSON from 'podman inspect --format json --type container' directly inside podman.InspectContainer() to confine the details within the podman package. The JSON samples for the unit tests were taken using the default Toolbx container on versions of Fedora that shipped a specific Podman and Toolbx version. This accounts for differences in the JSON caused by different major versions of Podman and the way different Toolbx versions set up the containers. One exception was Fedora 28, which had Podman 1.1.2 and Toolbx 0.0.9, which is the last Toolbx version before 'toolbox init-container' became the entry point for all Toolbx containers [1]. However, the default Toolbx image is no longer available from registry.fedoraproject.org. Hence, the image for Fedora 29 was used. The minimum required Podman version is 1.6.4 [2], and the Go implementation has been encouraging users to create containers with Toolbx version 0.0.17 or newer [3]. The versions used to collect the JSON samples for the unit tests were chosen accordingly. They don't exhaustively cover all possible supported and unsupported version combinations, but hopefully enough to be useful. [1] Commit 8b84b5e containers@8b84b5e4604921fa https://github.com/debarshiray/toolbox/pull/160 [2] Commit 8e80dd5 containers@8e80dd5db1e6f40b containers#1253 [3] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1490
Unmarshal the JSON from 'podman inspect --format json --type container' directly inside podman.InspectContainer() to confine the details within the podman package. The JSON samples for the unit tests were taken using the default Toolbx container on versions of Fedora that shipped a specific Podman and Toolbx version. This accounts for differences in the JSON caused by different major versions of Podman and the way different Toolbx versions set up the containers. One exception was Fedora 28, which had Podman 1.1.2 and Toolbx 0.0.9, which is the last Toolbx version before 'toolbox init-container' became the entry point for all Toolbx containers [1]. However, the default Toolbx image is no longer available from registry.fedoraproject.org. Hence, the image for Fedora 29 was used. The minimum required Podman version is 1.6.4 [2], and the Go implementation has been encouraging users to create containers with Toolbx version 0.0.17 or newer [3]. The versions used to collect the JSON samples for the unit tests were chosen accordingly. They don't exhaustively cover all possible supported and unsupported version combinations, but hopefully enough to be useful. [1] Commit 8b84b5e containers@8b84b5e4604921fa https://github.com/debarshiray/toolbox/pull/160 [2] Commit 8e80dd5 containers@8e80dd5db1e6f40b containers#1253 [3] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1490
Unmarshal the JSON from 'podman inspect --format json --type container' directly inside podman.InspectContainer() to confine the details within the podman package. The JSON samples for the unit tests were taken using the default Toolbx container on versions of Fedora that shipped a specific Podman and Toolbx version. This accounts for differences in the JSON caused by different major versions of Podman and the way different Toolbx versions set up the containers. One exception was Fedora 28, which had Podman 1.1.2 and Toolbx 0.0.9, which was the last Toolbx version before 'toolbox init-container' became the entry point for all Toolbx containers [1]. However, the default Toolbx image is no longer available from registry.fedoraproject.org. Hence, the image for Fedora 29 was used. The minimum required Podman version is 1.6.4 [2], and the Go implementation has been encouraging users to create containers with Toolbx version 0.0.17 or newer [3]. The versions used to collect the JSON samples for the unit tests were chosen accordingly. They don't exhaustively cover all possible supported and unsupported version combinations, but hopefully enough to be useful. [1] Commit 8b84b5e containers@8b84b5e4604921fa https://github.com/debarshiray/toolbox/pull/160 [2] Commit 8e80dd5 containers@8e80dd5db1e6f40b containers#1253 [3] Commit 238f245 containers@238f2451e7d7d54a containers#318 containers#1490
This is where the Go rewrite is happening. I welcome any kind of comment/contribution.
The shell script has been renamed to toolbox.sh so that toolbox.go can
be compiled.
The new codebase is based on Cobra which takes care of the CLI heavy lifting.