Skip to content

Keep CVersion in the vminitd package#747

Open
dkovba wants to merge 2 commits into
apple:mainfrom
dkovba:cversion
Open

Keep CVersion in the vminitd package#747
dkovba wants to merge 2 commits into
apple:mainfrom
dkovba:cversion

Conversation

@dkovba
Copy link
Copy Markdown
Contributor

@dkovba dkovba commented May 21, 2026

Moves CVersion back to the vminitd package. This is a follow-up for #742.

@dkovba dkovba requested review from dcantah and jglogan May 21, 2026 00:11
)

static func main() async throws {
AgentCommand.versionMetadata.withLock { $0 = Self.versionMetadata() }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary? See comment for AgentCommand.swift.

Copy link
Copy Markdown
Contributor Author

@dkovba dkovba May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See a new commit as an alternative way of dropping the version metadata Mutex from AgentCommand.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting it in logging metadata is going to add version to every log entry? That seems noisy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was a bug. Fixed!

metadata["tag"] = "\(gitTag)"
}
log.info("vminitd booting", metadata: metadata)
log.info("vminitd booting", metadata: Self.versionMetadata.withLock { $0 })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
log.info("vminitd booting", metadata: Self.versionMetadata.withLock { $0 })
log.info("vminitd booting", metadata: Application.versionMetadata.withLock { $0 })

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VminitdCore can't reference Application - the executable depends on the library, not the other way around.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is AgentCommand in VminitCore? It makes more sense for all the swift-argument-parser commands to be in the same target as Application, whereas the core implements just the agent business logic.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for moving VminitdCore to the top-level package was specifically so that custom vminitd executables can reuse it. The CLI surface, including AgentCommand, InitCommand, PauseCommand, is part of what they want to reuse.

@dkovba dkovba requested a review from jglogan May 21, 2026 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants