Add run command by coffmark · Pull Request #36 · mtj0928/nest

coffmark · 2025-05-13T20:55:25Z

Motivation

When nestfile.yaml was updated, version mismatches occurred when directly executing local binaries.

Therefore, I was using Git Hooks to run $ nest bootstrap to install the correct version of nestfile.yaml.

As an alternative to directly executing binaries, we can now use $ nest run owner/repository to execute binaries.

Describe changes

I newly implemented RunCommand.swift.

Run command accepts the following arguments and options.

$ nest help run

OVERVIEW: Run executable file on a given nestfile. If not found, it will
attempt to install.

USAGE: nest run [--verbose] [--no-install] [--nestfile-path <nestfile-path>] <arguments> ...

ARGUMENTS:
  <arguments>

OPTIONS:
  -v, --verbose
  --no-install            Will not perform installation.
  --nestfile-path <nestfile-path>
                          A nestfile written in yaml. (default: nestfile.yaml)
  -h, --help              Show help information.

I used NestInfoController for manipulating NestInfo and NestfileController for manipulating Nestfile.

Operation check

I used realm/SwiftLint as an example.

When realm/SwiftLint specified in nestfile.yaml is installed.

$ .nest/artifacts/realm_SwiftLint_github.com_https/0.55.0/SwiftLintBinary-macos/swiftlint lint Sources/NestKit
Linting Swift files at paths Sources/NestKit
// ...
Done linting! Found 44 violations, 0 serious in 34 files.

When realm/SwiftLint specified in nestfile.yaml is not installed

📦 Found an artifact bundle, SwiftLintBinary-macos.artifactbundle.zip, for SwiftLint.
🌐 Downloading the artifact bundle of SwiftLint...
✅ Success to download the artifact bundle of SwiftLint.
🪺 Success to install swiftlint version 0.55.0.
$ .nest/artifacts/realm_SwiftLint_github.com_https/0.55.0/SwiftLintBinary-macos/swiftlint lint Sources/NestKit
Linting Swift files at paths Sources/NestKit
// ...
Done linting! Found 44 violations, 0 serious in 34 files.

When realm/SwiftLint specified in nestfile.yaml is not installed and --no-install option is enabled

$ nest run --no-install realm/SwiftLint lint Sources/NestKit

Failed to find binary path, likely because it's not installed. Please try without the --no-install option or run the bootstrap command.

Issue ticket number and link

Issue: Proposal for a New run command #32

Checklist before requesting a review

I have performed a self-review of my code
I have added tests.

ref. mtj0928#32

mtj0928 · 2025-05-17T12:50:48Z

    }
+
+    /// Get the version that matches the `owner/repo`
+    public func fetchTarget(reference: String, nestfile: Nestfile) -> Nestfile.Target? {


[Naming] fetch sounds that the function get the target from a remote server.

How about this interface?

public func target(matchingTo reference: String, in nestfile: Nestfile) -> Nestfile.Target?

And please write DocC that explains the reference can accept only owner/repo format.

Thanks!
I've made the corrections here. (297dda5)

mtj0928 · 2025-05-17T12:51:51Z

+        return nestfile.targets
+            .first { target in
+                guard case let .repository(repository) = target,
+                      repository.reference == reference || GitURL.parse(string: repository.reference)?.referenceName == reference


Is not this enough?

Suggested change

repository.reference == reference || GitURL.parse(string: repository.reference)?.referenceName == reference

GitURL.parse(string: repository.reference)?.referenceName == reference

Thanks!
I've made the corrections here. (5571860)

mtj0928 · 2025-05-17T12:59:51Z

+    /// `owner/repo` format
+    public var referenceName: String? {
+        guard pathComponents.count >= 3 else { return nil }
+        let owner = pathComponents[1]
+        let repo = pathComponents[2].replacingOccurrences(of: ".\(pathExtension)", with: "")
+        return "\(owner)/\(repo)"
+    }


[Question] I'm not sure whether the logic always correct or not.
For example, is a URL like https://for.com/bar/{owner}/{repo} impossible?

is a URL like https://for.com/bar/{owner}/{repo} impossible?

In this case, bar/owner is output as a reference.
I added comments and tests (174375c)

mtj0928 · 2025-05-17T13:02:47Z

    }

+    /// Get NestInfo.Command from `owner/repository`
+    public func fetchCommand(reference: String, version: String) -> NestInfo.Command? {


Ditto (naming)

Thanks!
I've made the corrections here. (cbbe015)

mtj0928 · 2025-05-17T13:06:40Z

+                let command = $0.value
+                    .first {
+                        switch $0.manufacturer {
+                        case let .artifactBundle(sourceInfo):
+                            return sourceInfo.repository?.reference.referenceName == reference
+                        case let .localBuild(repository):
+                            return repository.reference.referenceName == reference
+                        }
+                    }
+                return command != nil


This may be straightforward.

Suggested change

let command = $0.value

.first {

switch $0.manufacturer {

case let .artifactBundle(sourceInfo):

return sourceInfo.repository?.reference.referenceName == reference

case let .localBuild(repository):

return repository.reference.referenceName == reference

}

}

return command != nil

$0.value

.contains {

switch $0.manufacturer {

case let .artifactBundle(sourceInfo):

return sourceInfo.repository?.reference.referenceName == reference

case let .localBuild(repository):

return repository.reference.referenceName == reference

}

}

Thanks!
I've made the corrections here. (3b4f465)

mtj0928 · 2025-05-17T13:09:18Z

+    @Flag(help: "Will not perform installation.")
+    var noInstall: Bool = false
+
+    @Option(help: "A nestfile written in yaml.")


That nestfile is written in yaml is obvious.

Suggested change

@Option(help: "A nestfile written in yaml.")

@Option(help: "A path to nestfile")

Thanks!
I've made the corrections here. (6f71256)

mtj0928 · 2025-05-17T13:18:40Z

+import NestCLI
+import Logging
+
+struct RunCommand: AsyncParsableCommand {


nest module should be thin as much as possible.
So could you try to move the logic to other modules and reduce the logic in this module?

Thanks!
I tried to corrected here. (1ee1a5f, 8306591)

mtj0928 · 2025-05-17T13:29:32Z

+        guard !isSkip else {
+            self = .skip
+            return
+        }
+        guard let expectedChecksum else {
+            self = .printActual { checksum in
+                logger.info("ℹ️ The checksum is \(checksum). Please add it to the nestfile to verify the downloaded file.")
+            }
+            return
+        }
+        self = .needsCheck(expected: expectedChecksum)


I think guard statement is for error case.

guard foo else { // error case } // happy path

In this function, skip and printActual cases are not error cases.
So, could you use if statements instead?

Thanks!
I've made the corrections here. (bf4e6e2)

mtj0928 · 2025-05-17T13:35:18Z

+        artifactBundleManager: ArtifactBundleManager,
+        logger: Logger
+    ) async throws -> String? {
+        guard let binaryRelativePath = nestInfoController.fetchCommand(reference: reference, version: version)?.binaryPath


Use if statement instead of guard statement.

Thanks!
I fixed it together here. (bf4e6e2)

coffmark · 2025-05-18T23:24:51Z

Thanks for your review!
I will contact you when the corrections are complete.

ref. mtj0928#36 (comment)

ref. mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

ref. mtj0928#36 (comment)

ref. mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

ref. mtj0928#36 (comment)

coffmark · 2025-05-21T22:01:53Z

+    /// Returns a relative path. If not, it will attempt to install.
+    public func resolveBinaryRelativePath(
+        noInstall: Bool,
+        reference: String,
+        version: String,
+        target: Nestfile.Target,
+        gitURL: GitURL,
+        gitVersion: GitVersion,
+        nestInfoController: NestInfoController,
+        executableBinaryPreparer: ExecutableBinaryPreparer,
+        artifactBundleManager: ArtifactBundleManager,
+        logger: Logger
+    ) async throws -> String? {


Ideally, I would write tests, but there are no tests for the dependent component (ExecutableBinaryPreparer), so I decided not to write tests for the resolveBinaryRelativePath method.

(If it is required, I will write tests together with ExecutableBinaryPreparer!)

coffmark · 2025-05-21T22:03:31Z

+        public var assetName: String? {
+            switch self {
+            case .repository(let repository): repository.assetName
+            case .zip, .deprecatedZIP: nil
+            }
+        }
+
+        public var checksum: String? {
+            switch self {
+            case .repository(let repository): repository.checksum
+            case .zip(let zipURL): zipURL.checksum
+            case .deprecatedZIP: nil
+            }
+        }


I moved properties used by bootstrap and run commands.

mtj0928

LGTM, except for the bug mentioned in a comment.
And, could you update README.md to explain the added command?

mtj0928 · 2025-05-27T14:26:45Z

+        _ = try await NestProcessExecutor(logger: logger, logLevel: .info)
+            .execute(
+                command: "\(nestDirectory.rootDirectory.relativePath)\(binaryRelativePath)",
+                executor.subcommands
+            )


I noticed a bug that a user cannot input any texts to the command even if the command requires inputs because NestProcessExecutor does't support it.

It's OK to merge this PR without fixing the issue and fix it in another PR.
Of course, it's also welcome to fix it in this PR!

I would appreciate it if you could merge this PR as the differences are getting bigger.

As for the bugs, I will fix it and submit a PR. (If you need to create an Issue, I will create it.)

@mtj0928
I tried writing a sample CLI tool to receive standard input, but could not reproduce it. (https://github.com/coffmark/InputText)

v0.0.1 -> only use readline method. (source code)
v0.0.2 -> use readline method with swift-argument-parser (source code)

Would it be possible for you to share the name of the CLI tool (if it is publicly available) or the steps to reproduce it?

Screenshot.2025-05-30.8.01.20.mov

Oh, sorry.
I might misunderstood something.
Thank you for the investigation!
For now, it's fine to keep the current code.

FYI: I've noticed that the input mechanism of xcodes's select doesn't work. #38

ref. mtj0928#36 (review)

coffmark · 2025-05-27T23:03:34Z

LGTM, except for the bug mentioned in a comment. And, could you update README.md to explain the added command?

Thanks for your review!
I have corrected it here. (ce62f3f)

mtj0928

Basically, LGTM!
Let me modify the code a bit after merging this PR, for very personal preferences.

coffmark · 2025-05-29T23:09:58Z

Thank you for merging!

coffmark added 2 commits May 14, 2025 05:53

Run command has been implemented

1734f70

ref. mtj0928#32

Add Test

aac3c1c

coffmark force-pushed the feature/run-command branch 4 times, most recently from 371f846 to 198c014 Compare May 15, 2025 21:38

refactor

849bd8b

coffmark force-pushed the feature/run-command branch from 198c014 to 849bd8b Compare May 15, 2025 21:41

coffmark changed the title ~~[WIP] Add run command~~ Add run command May 15, 2025

coffmark marked this pull request as ready for review May 15, 2025 21:43

mtj0928 reviewed May 17, 2025

View reviewed changes

coffmark changed the title ~~Add run command~~ [WIP] Add run command May 18, 2025

coffmark and others added 6 commits May 20, 2025 08:52

Renamed fetchTarget(reference:nestfile:) to target(matchingTo:in:)

297dda5

ref. mtj0928#36 (comment)

Remove unnecessary expressions

5571860

ref. mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

Add comments, add tests and renamed referenceName to reference

174375c

ref. mtj0928#36 (comment)

Renamed fetchCommand(reference:version:) to command(matchingTo:version)

cbbe015

ref. mtj0928#36 (comment)

Use contains method

3b4f465

ref. mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

Fixed help message for nestfilePath option

6f71256

mtj0928#36 (comment) Co-authored-by: matsuji <mtj0928@users.noreply.github.com>

coffmark added a commit to coffmark/nest that referenced this pull request May 20, 2025

Use if statement rather than guard statement

ff049a9

ref. mtj0928#36 (comment)

coffmark added a commit to coffmark/nest that referenced this pull request May 20, 2025

Use if statement rather than guard statement

d800cf2

ref. mtj0928#36 (comment)

coffmark force-pushed the feature/run-command branch from 4bddde4 to d800cf2 Compare May 20, 2025 21:42

Use if statement rather than guard statement

bf4e6e2

ref. mtj0928#36 (comment)

coffmark force-pushed the feature/run-command branch from d800cf2 to bf4e6e2 Compare May 20, 2025 21:44

coffmark added 3 commits May 21, 2025 06:46

Renamed referenceName to reference

46d17c4

Thin the RunCommand logic

1ee1a5f

ref. mtj0928#36 (comment)

Implemented RunCommandExecutor

8306591

ref. mtj0928#36 (comment)

coffmark commented May 21, 2025

View reviewed changes

coffmark changed the title ~~[WIP] Add run command~~ Add run command May 21, 2025

coffmark requested a review from mtj0928 May 21, 2025 22:05

mtj0928 reviewed May 27, 2025

View reviewed changes

Described the run command in README.md

ce62f3f

ref. mtj0928#36 (review)

coffmark requested a review from mtj0928 May 27, 2025 23:07

mtj0928 approved these changes May 29, 2025

View reviewed changes

mtj0928 merged commit 6c4d0f0 into mtj0928:main May 29, 2025
1 check passed

coffmark deleted the feature/run-command branch May 29, 2025 22:31

mtj0928 mentioned this pull request May 31, 2025

Run command doesn't work for xcodes #38

Closed

	repository.reference == reference \|\| GitURL.parse(string: repository.reference)?.referenceName == reference
	GitURL.parse(string: repository.reference)?.referenceName == reference

	@Option(help: "A nestfile written in yaml.")
	@Option(help: "A path to nestfile")

Conversation

coffmark commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Describe changes

Operation check

Issue ticket number and link

Checklist before requesting a review

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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

coffmark May 20, 2025 • 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!

coffmark May 20, 2025 • 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!

coffmark commented May 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mtj0928 left a comment

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!

mtj0928 May 31, 2025 • 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!

coffmark commented May 27, 2025

Uh oh!

mtj0928 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coffmark commented May 29, 2025

Uh oh!

coffmark commented May 13, 2025 •

edited

Loading

coffmark May 20, 2025 •

edited

Loading

coffmark May 20, 2025 •

edited

Loading

mtj0928 May 31, 2025 •

edited

Loading