feat: make kopium usable as a library #347

the-wondersmith · 2025-08-05T14:23:58Z

PR makes kopium usable as a library in addition to its current standalone tool status.

Addresses #335

clux

hey, thanks for the showcase for this. it's a decent start.

i'm not keen on just exposing most of main as a library interface though. arg parsing / stdin reading / auto complete / crd detection etc is not really a lib concern.

i am open to refactors that lifts stuff from main into the output module (or some similar formatter moduler) because then the already published library (e.g. the one with docs here) would be more useful.

also, file renames and large shifting around like this is also not great because this means i have a very hard github diff to review. kind of github problem and not yours, but i work with what's here 😢

the-wondersmith · 2025-08-07T02:15:05Z

@clux Would it be uhh.. keener to extract everything that isn't parsing / stdin reading / auto complete / crd detection to the lib level then? Happy to take a second pass at it.

Signed-off-by: Mark S <[email protected]>

the-wondersmith · 2025-08-07T17:01:56Z

@clux took that second pass and separated all the "clearly CLI" from "probably library" code. Hopefully it's closer to spec?

Signed-off-by: Mark S <[email protected]>

Also used kopium to avoid manual adjustments to get rust types from xrd's. Atm has a compile-time dependency on the kopium-cli, but maybe we won't need this long time. (see kube-rs/kopium#347) Open Bugs: - metadata.generation seems to be passed as float and we won't accept it as it should be formally i64

clux · 2025-08-18T07:00:21Z

hey, sorry slow response. not had a time to have a proper think about this, and the large diff makes it hard to sit down in small periods of time. but i've had your idea of the big typed builder emulating the cli bouncing around in my head for a while, and i wasn't sure about it last week, but think i am coming around to it. 😄

are you able to run git mv src/bin/kopium.rs src/main.rs to preserve parts of the diff in the main bin?

# Conflicts: # Cargo.toml

the-wondersmith · 2025-08-18T20:02:43Z

hey, sorry slow response. { ... }

Absolutely no worries, I know how it is.

{ ... } the big typed builder emulating the cli bouncing around in my head for a while, and i wasn't sure about it last week, but think i am coming around to it. 😄

Yeah, it's kinda nice. I actually put in a PR to the rust-typed-builder repo to fix an issue I found while working that this, and with that fix in place the generated typed-builders can now be made even nicer. (Got a separate PR for that once we work our way through this one 😅)

are you able to run git mv src/bin/kopium.rs src/main.rs to preserve parts of the diff in the main bin?

Done, and merge conflicts resolved 🙂

clux

have had one go over, lmk what you think 😄
need to look more later, haven't dug in detail into the main replacement (generate_rust_types_for) yet

clux · 2025-08-20T10:08:44Z

src/lib.rs

+                        self.derive_traits.push(json)
+                    }
+                }
+                _ => {}


we can probably have an enum exposed for this for library case here to avoid string typing.
we can reference the kube-derive upstream derive modes it is based on

clux · 2025-08-20T10:10:39Z

src/lib.rs

+        /// Enable all automation features
+        ///
+        /// This is functionally the same as supplying `--auto` to the `kopium` command
+        pub fn auto(&mut self, value: bool) {
+            self.emit_docs = value;
+
+            if value {
+                self.schema_mode = "derived".into();
+            } else {
+                self.schema_mode = "disabled".into();
+            }
+        }


i'm a little unsure of whether this "fuzzy helper alias" should be part of the library api, but it is at least consistent with the cli 🤔

clux · 2025-08-20T10:18:02Z

src/lib.rs

+        /// Add the supplied [`Derive`] directive to the list of traits to be derived on
+        /// generated types
+        pub fn derive(&mut self, value: Derive) {
+            self.derive_traits.push(value);
+        }
+
+        /// Attempt to parse the supplied value as a [`Derive`] directive and add it to
+        /// the list of traits to be derived on generated types
+        pub fn try_derive(&mut self, value: impl AsRef<str>) {
+            let derive = value.as_ref().parse::<Derive>().expect("unparsable value");
+            self.derive_traits.push(derive);
+        }
+
+        /// Add all the supplied [`Derive`] directives to the list of traits to be derived
+        /// on generated types
+        pub fn derive_all(&mut self, values: impl IntoIterator<Item = Derive>) {
+            self.derive_traits.extend(values);
+        }
+
+        /// Attempt to parse each of the supplied values as a [`Derive`] directive and add
+        /// them to the list of traits to be derived on generated types
+        pub fn try_derive_all(&mut self, values: impl IntoIterator<Item = impl AsRef<str>>) {
+            let values = values.into_iter().map(|value| {
+                value.as_ref().parse::<Derive>().expect("unparsable value")
+            });
+
+            self.derive_traits.extend(values);
+        }


i am not a fan of relying on the FromStr impl for Derive which takes the same fuzzy input. Maybe we can make Target and Derive public and let people implement them for precise library input?

clux · 2025-08-20T10:24:53Z

src/output.rs

+impl<T: AsRef<str>> From<T> for MapType {
+    fn from(value: T) -> Self {
+        let value = value.as_ref();
+
+        if value.eq_ignore_ascii_case("HashMap") {
+            Self::HashMap
+        } else {
+            if !value.eq_ignore_ascii_case("BTreeMap") {
+                log::warn!("unknown map type '{}', defaulting to BTreeMap", value);
+            }
+
+            Self::BTreeMap
+        }
+    }
+}


i see this is to get around clap arg parse, and we have kept it fuzzy. in this case, maybe it's better to do something like what prost_build is doing https://docs.rs/prost-build/latest/prost_build/struct.Config.html#method.btree_map
(with a .hash_map setter instead)

src/lib.rs

clux · 2025-08-20T10:38:21Z

src/lib.rs

+    #[builder(
+        default_code = r#"String::from("disabled")"#,
+        via_mutators(init = String::from("disabled")),
+    )]
+    schema_mode: String,


is the builder code like this flexible? the suggestion i made may be not super compatible with an inflexible builder setup?

In my fiddling so far, it has been, yeah. It's really only there to try and maintain parity with the CLI behavior though, which may or may not be sensible?

i do have a stronger sense of guarantees for library behavior baked into my head personally, because you can do a lot more of the common rust mantra (make invalid states unrepresentable) with the code as opposed to various cli flags that ultimately takes strings.

we don't have to go too far with it, in an initial pr like this, but ideally we can make the library interface follow standard library conventions a little more. as in, i don't think we need to maintain parity with the ad-hoc way i set up most of the kopium cli flags.

Well, when you put it like that, I think it makes a lot of sense. Seems like we could go the other way with it actually, and start (in a follow-up) by bringing the library interface up to snuff with standard rust library conventions and then bringing the CLI into line so to speak.

(Side note, please take all of ^ these type comments from me as a tacit offer to do the work in question, as opposed to "wouldn't it be great if someone..." 😅)

the-wondersmith · 2025-09-16T15:57:06Z

@clux Do you think it'd be possible to schedule a working session to get this into shape for merging? I'm sure your schedule is probably as unhelpful as mine, but I'd love to carve out some time to get it over the line if we can 😅

clux · 2025-09-17T09:14:52Z

@clux Do you think it'd be possible to schedule a working session to get this into shape for merging? I'm sure your schedule is probably as unhelpful as mine, but I'd love to carve out some time to get it over the line if we can 😅

Yeah, that's probably a good idea. Do you mean like a video call or something? Can probably set something up or we can piggy-back on the kube sync. I'm UK, and if you're east coast, can do something in US morning a day you have time. Monday/Thu/Fri are good for me.

the-wondersmith · 2025-09-18T13:32:25Z

@clux Do you think it'd be possible to schedule a working session ...

... Do you mean like a video call or something?

Yes.

Can probably set something up or we can piggy-back on the kube sync. I'm UK, and if you're east coast, can do something in US morning a day you have time. Monday/Thu/Fri are good for me.

Awesome. If you're on the CNCF Slack, wanna coordinate there? (I sent you a DM there already, just in case)

the-wondersmith · 2025-09-25T14:26:32Z

Per our conversation today, the recent changes are a bit much to be able to rebase / resolve the merge conflicts with any degree of certainty, so I'm I'm gonna close this PR in favor of simply redoing the work against the updated main and opening a fresh PR.

the-wondersmith marked this pull request as ready for review August 5, 2025 14:24

the-wondersmith mentioned this pull request Aug 5, 2025

publish a library that is intended for use in build.rs scripts. #335

Open

the-wondersmith force-pushed the feat/kopium-as-library branch from 9ddea53 to 24a5319 Compare August 5, 2025 14:27

clux requested changes Aug 5, 2025

View reviewed changes

feat: make kopium usable as a library

e936b56

Signed-off-by: Mark S <[email protected]>

the-wondersmith force-pushed the feat/kopium-as-library branch from c35e0d3 to cfac1bd Compare August 7, 2025 17:00

refactor: reorganize to separate library from cli code

6de78b3

Signed-off-by: Mark S <[email protected]>

the-wondersmith force-pushed the feat/kopium-as-library branch from cfac1bd to 6de78b3 Compare August 8, 2025 19:06

the-wondersmith requested a review from clux August 12, 2025 12:57

Merge branch 'main' into feat/kopium-as-library

148b713

the-wondersmith added 2 commits August 18, 2025 15:52

refactor: address requested PR changes

3d0e625

Merge remote-tracking branch 'upstream/main' into feat/kopium-as-library

6723ea6

# Conflicts: # Cargo.toml

clux reviewed Aug 20, 2025

View reviewed changes

the-wondersmith closed this Sep 25, 2025

Uh oh!

feat: make kopium usable as a library #347

feat: make kopium usable as a library #347

Uh oh!

Conversation

the-wondersmith commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clux left a comment

Choose a reason for hiding this comment

Uh oh!

the-wondersmith commented Aug 7, 2025

Uh oh!

the-wondersmith commented Aug 7, 2025

Uh oh!

clux commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-wondersmith commented Aug 18, 2025

Uh oh!

clux left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the-wondersmith commented Sep 16, 2025

Uh oh!

clux commented Sep 17, 2025

Uh oh!

the-wondersmith commented Sep 18, 2025

Uh oh!

the-wondersmith commented Sep 25, 2025

Uh oh!

Uh oh!

the-wondersmith commented Aug 5, 2025 •

edited

Loading

clux commented Aug 18, 2025 •

edited

Loading

clux left a comment •

edited

Loading