csjones / lefthook-plugin

Fast and powerful Git hooks manager for Swift packages.
MIT License
6 stars 0 forks source link

Is it possible to add an executableTarget to the project so Mint can run it? #1

Closed levibostian closed 5 months ago

levibostian commented 5 months ago

Neat project. Thanks for creating it!

Are you interested in adding an executableTarget product to this project besides just the plugin target? I ask because if there is an executableTarget added to the Package.swift of this project, then it can be executed via mint. Similar to how other popular Swift CLI tools such as soucery, swiftlint, swiftformat can be executed. I think that sounds handy to have.

Are you interested in this target being added to this project? Do you personally know how to add this? I am happy to help add it. I am not as experienced with executableTargets so I wanted to see if you had any tips to provide to kick-start the effort.

Thanks for the time.

csjones commented 5 months ago

Hi @levibostian πŸ‘‹

Thanks for the feedback and suggestion! Adding an executableTarget for mint sounds like a great idea. I'm open to it and would appreciate your help to get this implemented. While I am familiar with creating an executableTarget, the implementation challenge is finding a suitable replacement for the PackagePlugin.PluginContext since it's only available via .plugin(). Without a replacement for PackagePlugin.PluginContext, I don't know how we would invoke the lefthook binary and pass it parameters from an executableTarget.

Let me know what ideas you might have for solving this πŸ™‚

levibostian commented 5 months ago

Thanks for the reply and for interest in the feature!

As far as creating an executable target, I recall seeing some simple examples online for calling a binary and passing arguments.

How a plugin type makes this more complex, I'm not sure yet.

I'm working on this in my free time so I'll look into this when I get a chance ☺

levibostian commented 5 months ago

Well, I got it to work, but it might not have been the "correct" way to do it 🀣.

If you run this command on your computer right now, you can see it working:

mint run levibostian/lefthook-plugin@exec-target version. You should see "1.6.10" get printed. mint run levibostian/lefthook-plugin@exec-target install. You should see lefthook install git hooks and generate a config file in the current directory.

If you see the code I wrote in the branch, the executable target created runs the command swift package plugin lefthook, essentially just running the plugin target.

Originally, I tried to create an executable target that could call the lefthook binaries directly but I was not able to get that to work. If I knew more about artifact bundles, maybe I could have gotten it to work.

What do you think about this solution? If you don't mind it, I am happy to clean the code up a little bit and make a PR from my fork.

csjones commented 5 months ago

It's a good start and the kind of workaround I was hoping for! πŸ‘ I think there is one scenario to be addressed before creating a PR.

The solution works well when a developer has already added the lefthook-plugin package to their manifest file, but if they don't explicitly take that step, they'll get this error:

mint run levibostian/lefthook-plugin@exec-target version
error: Unknown subcommand or plugin name β€˜lefthook’
Usage: swift package <options> <subcommand>
  See 'package -help' for more information.

I am hoping to use your implementation for a private project because it uses mint heavily behind the scenes. Unfortunately, the project is restricted from adopting plugins (at least in the near future).

Let me know what your thoughts are 😁

Originally, I tried to create an executable target that could call the lefthook binaries directly but I was not able to get that to work. If I knew more about artifact bundles, maybe I could have gotten it to work.

No worries about the artifact bundles, this repository was my first attempt at using them and I spent a lot of time re-reading the artifact bundle proposal to wrap my head around the concept.

I originally tried bringing lefthook to Swift using a XCFramework but abandoned the approach because my solution had too much overhead to build the XCFramework (which was created from underlying GO code) on every upstream release.

levibostian commented 5 months ago

The solution works well when a developer has already added the lefthook-plugin package to their manifest file, but if they don't explicitly take that step, they'll get this error:

Ah, great catch. I was running the mint commands from my fork so that's why I was not getting the errors. When I run them from a different project, indeed, I get that error. I agree, this needs to be addressed.

I think we have 2 options.

  1. From the proposal you linked to (great resource, thank you), SPM processes the artifactbundle.zip file and provides the super handy context.tool() API to get the binary path for the platform they are running on. But, this is only available for plugins... I wonder if by browsing the API docs for Plugins, maybe there is a class or function that can be called from an executable target to get access to these great functions?

  2. I suppose, doing what SPM does manually is also an option. Manually unzipping an artifact bundle, parsing the manifest file to determine what binary target you need, and then manually running the binary target could work. It just seems like we're manually doing the work that SPM does for you.

Side note, Thinking about this solution idea, if it was implemented, it could be worth pitching the solution to Mint to add support to run binaryTargets directly πŸ€”.


I have built similar tools like this in the past where the solution I took was to download the binary directly from github releases at runtime and then execute the downloaded binary. However, because of apple code signing, you need to manually allow running the downloaded unsigned binary before it's run. It seems that if you package an artifactbundle, this is not the case. So, I like these 2 solutions more to skip this code signing hurdle.

If you have other ideas, would love to hear them!

csjones commented 5 months ago

I wonder if by browsing the API docs for Plugins, maybe there is a class or function that can be called from an executable target to get access to these great functions?

I keep coming back to this option but each time end up empty handed. Ideally, this would be the preferable approach.

I suppose, doing what SPM does manually is also an option. Manually unzipping an artifact bundle, parsing the manifest file to determine what binary target you need, and then manually running the binary target could work. It just seems like we're manually doing the work that SPM does for you.

I am leaning towards this solution because it could re-using the existing SPM code that implements artifact bundle handling. There might even be a way to utilize the artifacts folder (e.g. .build/artifacts/lefthook-plugin/lefthook/) that SPM uses to execute the binaries currently reserved for plugins.

levibostian commented 5 months ago

I read the SPM source code and duplicated the logic to create a quick solution (Code isn't cleaned up and ready for a PR, just good enough for testing it).

If I run swift run lefthook-executable version from my lefthook-plugin directory, it works as expected.

If I run mint run --verbose levibostian/lefthook-plugin@734d3ffc3785f60d40f77f450c9c6c01e78b45bd version, it doesn't work as expected. I think it's because if you look closely at the verbose mint output, you will notice towards the end that mint copies the compiled binary into your ~/.mint directory. If I browse to that directory on my machine, there is no .build/.... directory anymore next to the binary.

We need to find another way to get an artifact bundle to parse. I have a few ideas:

  1. If the executable target can have the binary target as a dependency, Swift gives our executable target access to the artifact bundle somehow? Maybe via Bundle? This is my preferred solution but not sure how to do this.
  2. The executable target downloads the artifact.zip from github release at runtime.
  3. The executable target has the artifact.zip file packaged as a local reference and committed to git. I have gotten this to work already for testing purposes, but it's not ideal that we need to commit a .zip file with each github release.
csjones commented 5 months ago

Mint says it still supports the Package.resources file (this repository uses a Package.resources for mint). This might a reasonable solution for mint even though defining resources directly in spm would be more ideal because I agree would shouldn't be committing a zip to the repository for each release.

The only other solution I can think of (without resorting to network download) is adding a new pre-build plugin target that only has one dependency (the existing binaryTarget). The pre-build plugin just makes sure spm triggers the creation of .build/artifacts/lefthook-plugin/lefthook/ so we can define a valid .process(".build/artifacts/lefthook-plugin/lefthook/") in the manifest on the executableTarget by the time mint runs swift build.

This isn't a complete test but spm didn't raise an error when I referenced a resource that did not exist yet:

        .executableTarget(
            name: "test",
            resources: [
                .process("../../.build/artifacts/lefthook-plugin/lefthook/")
            ],
            plugins: [
                .plugin(name: "pre-build-test")
            ]
        ),
levibostian commented 5 months ago

Ah, I didn't consider those ideas. Thanks for sharing.

I am not sure either of those solutions will be reliable? When I run the command: mint run --verbose levibostian/lefthook-plugin@734d3ffc3785f60d40f77f450c9c6c01e78b45bd version and view the output, it doesn't look like SPM is compiling the lefthook-plugin target. Making me think that when you run mint run... commands, then a .build/artifacts/lefthook-plugin/... directory is not going to be made. Have you found a reliable way to make sure this directory is always made?

Although I'm not yet sure that the .build/artifacts/ directory is always made, looking at the output though, binary artifacts are always downloaded:

🌱 Cloning lefthook-plugin 734d3ffc3785f60d40f77f450c9c6c01e78b45bd
Cloning into 'github.com_levibostian_lefthook-plugin'...
remote: Enumerating objects: 111, done.
remote: Counting objects: 100% (94/94), done.
remote: Compressing objects: 100% (64/64), done.
remote: Total 111 (delta 50), reused 63 (delta 25), pack-reused 17
Receiving objects: 100% (111/111), 20.43 KiB | 1.36 MiB/s, done.
Resolving deltas: 100% (53/53), done.
Note: switching to '734d3ffc3785f60d40f77f450c9c6c01e78b45bd'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 734d3ff Create target by directly calling artifact bundle binaries
🌱 Resolving package
Fetching binary artifact https://github.com/csjones/lefthook-plugin/releases/download/1.6.10/lefthook.artifactbundle.zip from cache
Fetched https://github.com/csjones/lefthook-plugin/releases/download/1.6.10/lefthook.artifactbundle.zip from cache (0.73s)
🌱 Building product lefthook-executable
Building for production...
[5/5] Linking lefthook-executable
Build complete! (5.44s)
Copying /var/folders/1w/h6spj5_s3zv661x7jtk3kzvc0000gn/T/mint/github.com_levibostian_lefthook-plugin/.build/release/lefthook-executable to /Users/levibostian/.mint/packages/github.com_levibostian_lefthook-plugin/build/734d3ffc3785f60d40f77f450c9c6c01e78b45bd/lefthook-executable
🌱 Installed lefthook-plugin 734d3ffc3785f60d40f77f450c9c6c01e78b45bd
🌱 Running lefthook-executable 734d3ffc3785f60d40f77f450c9c6c01e78b45bd...
Illegal instruction: 4

So I wondered if we could take advantage of the fact that SPM will always download the binaryTarget for us to the local file system. See if we can read that downloaded artifact bundle.

Looking at the source for this artifacts cache, it looks like ~/.swiftpm is a reliable way to find downloaded artifacts.

We could have logic at runtime for the executable that does these steps:

  1. Iterate all files in directory ~/.swiftpm/cache/artifacts/.
  2. Calculate the sha256sum of each file in this directory. When you find a checksum that matches the checksum of the binaryTarget, you found the downloaded artifact.
  3. unzip the downloaded artifact somewhere (/tmp/? ~/Application support/?).
  4. Luckily, when you look at the unzipped contents (see screenshot), it's identical to what's in .build/artifacts/lefthook-plugin/. So we can re-use the logic in this commit to find the correct binary file and execute it.

If we are not able to make sure that .build/artifacts/lefthook-plugin/ gets created when you run mint run..., the steps above seem like a nice workaround.

csjones commented 5 months ago

The ~/.swiftpm/cache/artifacts/ seems like a pretty good approach and might be the one we should use. If you think this implementation will make more sense, I'd encourage you to pursue it.

I started testing a pre-build plugin and could reliably generate the .build/artifacts/lefthook-plugin/lefthook/ directory before the executable is built using the swift build command. Unfortunately, don't have the same success using mint (seems like mint clears the artifacts directory which undoes the pre-build plugin work...) therefore this approach still needs additional solutions to things but would reduce the amount of bin handling code for running lefthook.

levibostian commented 5 months ago

Could you share how your testing with mint and your pre-build plugin was not successful? I just ran a test and it is working for me.

What I did:

CleanShot 2024-04-17 at 07 09 47

Before I help contribute code, I wanted to confirm with you that your testing was unsuccessful. If you're able to find success like I have, I am happy to help write the executable target code.

πŸ€” Now that I think about it, maybe it would be best if your PreBuildPlugin branch contained a github action that ran mint run --verbose csjsones/lefthook-plugin@<name-of-branch> version? It would be a good idea to have this CI test created for this executable anyway, maybe this would help us right now to see if the idea works?

Output of the mint run command
``` levibostian:/tmp/foo $ mint run --verbose csjones/lefthook-plugin@2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b version 🌱 Cloning lefthook-plugin 2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b Cloning into 'github.com_csjones_lefthook-plugin'... remote: Enumerating objects: 115, done. remote: Counting objects: 100% (53/53), done. remote: Compressing objects: 100% (43/43), done. remote: Total 115 (delta 46), reused 11 (delta 9), pack-reused 62 Receiving objects: 100% (115/115), 18.49 KiB | 728.00 KiB/s, done. Resolving deltas: 100% (57/57), done. Note: switching to '2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b'. You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by switching back to a branch. If you want to create a new branch to retain commits you create, you may do so (now or later) by using -c with the switch command. Example: git switch -c Or undo this operation with: git switch - Turn off this advice by setting config variable advice.detachedHead to false HEAD is now at 2a92a06 Testing PreBuildPlugin and Package resources 🌱 Resolving package Fetching binary artifact https://github.com/csjones/lefthook-plugin/releases/download/1.6.10/lefthook.artifactbundle.zip from cache Fetched https://github.com/csjones/lefthook-plugin/releases/download/1.6.10/lefthook.artifactbundle.zip from cache (0.74s) 🌱 Building product LefthookExecutable Building for production... [5/5] Linking LefthookExecutable Build complete! (3.72s) Copying /var/folders/1w/h6spj5_s3zv661x7jtk3kzvc0000gn/T/mint/github.com_csjones_lefthook-plugin/.build/release/LefthookExecutable to /Users/levibostian/.mint/packages/github.com_csjones_lefthook-plugin/build/2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b/LefthookExecutable 🌱 Copying resources for lefthook: .build ... 🌱 Installed lefthook-plugin 2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b 🌱 Running LefthookExecutable 2a92a0689e88b31519e62d12bc6bff0e5f9c2f7b... ```
csjones commented 5 months ago

Could you share how your testing with mint and your pre-build plugin was not successful? I just ran a test and it is working for me.

Oh, my mistake. I can confirm it works with mint too πŸŽ‰ When I was browsing from terminal I thought the lefthook directory under github.com_csjones_lefthook-plugin was a binary without actually checking because I was expecting to just see lefthook-plugin. Thank you for double checking! πŸ˜„

I will open a PR with the PreBuildPlugin branch and add a github action. Do you think the github action should be similar to the existing ones or maybe instead use the mint action?

levibostian commented 5 months ago

Thanks for making a PR! I'm happy to build off your branch by adding the code I wrote to execute the binary. Combine our two efforts and I think we have this thing built! It's been great working on this with you. Thanks a lot for the collaboration.

The setup-mint action you mention is great. I think we only need to use part of its features but we would still benefit from it installing the mint binary.

To test mint compatibility on ci, I was imagining something simple. A workflow containing these steps:

Check the output from the mint command to assert it contains a certain string from lefthook help. To test it actually ran a real lefthook command.

That's about it. If you have a better idea, go for it.


Typing this on my phone. Hope this makes sense.

csjones commented 5 months ago

Thank you 😊 I appreciated the your work and feedback with this feature. Your collaboration helped a lot and I think we are arriving at great solution given the constraints.

I've opened the PR and incorporated the action based on your suggestions https://github.com/csjones/lefthook-plugin/pull/3

levibostian commented 5 months ago

Thanks! Once I get this working I'll open a PR with the base set to your PreBuildPlugin branch.

levibostian commented 5 months ago

PR opened πŸŽ‰ https://github.com/csjones/lefthook-plugin/pull/4