Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure Ameba Distribution #470

Open
devnote-dev opened this issue Sep 26, 2024 · 12 comments
Open

Restructure Ameba Distribution #470

devnote-dev opened this issue Sep 26, 2024 · 12 comments

Comments

@devnote-dev
Copy link

I recently read an issue involving Ameba; a contributor stated they didn't believe it should be installed and compiled as a development dependency, but rather distributed and installed like any other external tool. I (also recently, coincidentally) encountered an odd issue when running Ameba in one of my projects: I have several folders that link to PATH so running ameba can call any of them which can be any version and return varying results. And that's exactly what happened 😄

Now, to remove these binaries wouldn't necessarily solve the issue as different projects use different versions of Ameba, some of which require refactoring to support newer versions. I also can't have multiple versions of Ameba available in PATH because of the aforementioned issue. But what if I could have one version of Ameba with multiple versions?

I propose that all rules be bound to a version requirement and introduce a --version flag (or command, whatever works best) that accepts a valid Ameba version. Running ameba --version 1.5.0 would only enable rules matching the < 1.5.0 requirement. Running ameba would default to the latest available version, that is, the version of that binary. This process is similar to Rust editions (if I've understood that correctly) and would remove the need to depend on multiple local versions.

Alongside this, I propose that Ameba is released in official distribution channels and discourage the current practice of local development installations with Shards (perhaps a compile time warning, similar to how Crystal's i_know_what_im_doing flag works).

@straight-shoota
Copy link
Contributor

I completely agree. But the ameba version should generally be recorded in .ameba.yml. There's already a discussion about keeping the ruleset consistent in #448.

You're totally right that in order to remove ameba from an implicit postinstall, we need to improve and expand its distribution channels. It needs to be easy to get a recent version installed.

There are currently couple of packages available in the distribution package repos of Alpine Linux, AUR and nixpkgs (https://repology.org/project/ameba/versions).

@devnote-dev
Copy link
Author

But the ameba version should generally be recorded in .ameba.yml.

I hadn't even thought about that but it's perfect! 👍

As for distribution, I believe that providing compiled binaries as part of each release would greatly support this. I'm not sure how practical it would be to provide a couple previous versions' releases, but it's definitely viable going forward.

@straight-shoota
Copy link
Contributor

I'm not sure how practical it would be to provide a couple previous versions' releases, but it's definitely viable going forward.

If binaries are attached to GitHub releases for example, earlier versions would automatically be available.
However I don't think there should be much need for installing older versions if newer ones are able to emulate the configuration of previous versions.

@devnote-dev
Copy link
Author

I'll be starting work on this shortly, is there a preferred implementation for this? I've been familiarising myself with the codebase and there seems to be a lot of heavy macro processing. (cc: @Sija)

@Sija
Copy link
Member

Sija commented Oct 7, 2024

@devnote-dev I've created a PR which implements rule versioning as the first step towards that. Feel free to comment on that and propose further action points.

@devnote-dev
Copy link
Author

This may be unrelated but when building that branch and running Ameba, I got this error:

image

@Sija
Copy link
Member

Sija commented Oct 7, 2024

@devnote-dev yup, looks like unrelated Crystal Windows-related bug, or a variant of #473.

@nobodywasishere
Copy link
Contributor

The downside to this (even though I'm for it due to not wanting to have a separate version of ameba in each project), is that there's no way to create custom per-project plugins like there are with the current system.

https://crystal-ameba.github.io/2019/07/22/how-to-write-extension/

@straight-shoota
Copy link
Contributor

that there's no way to create custom per-project plugins like there are with the current system.

You can still have custom extensions in your codebase or a dependency and use it to build a custom version of ameba. I don't think this would really change much at all since you already have to explicitly build ameba in this case anyway.

@devnote-dev
Copy link
Author

With rule versioning now implemented, we can start looking at distribution (thank you Sija ❤️).

For Windows this should be pretty straightforward as Scoop is a generally widely accessible package manager. We could also support WinGet but that requires a separate packaging process (Scoop just reads from a package file) and I haven't really seen anyone using it for Crystal related work.

For Linux, adding the common tools (APT, Pacman, etc.) should be fine. I'm not aware of other managers that need to be supported but I can do some research into that.

Homebrew should be fine as is, the only thing we'd need to change is the compile time flag to disable the warning — any strong naming towards this or should we go with the one in OP?

@straight-shoota
Copy link
Contributor

straight-shoota commented Nov 26, 2024

I'd recommend as the very first step to provide generic builds on the GitHub release.
That would provide a good base. And I imagine it could already satisfy quite many use cases.

On many platforms installing ameba could be as simple as downloading a statically linked build and making it executable.

curl -L -o ~/bin/ameba https://github.com/crystal-ameba/ameba/releases/download/1.7.0/ameba-1.7.0-linux-x86_64 && chmod +x ~/bin/ameba

We might not even need to bother about providing packages. That's quite a rabbit hole.
Most use cases should be perfectly fine with a simple mechanism to install a statically linked executable.
So it should be fine to leave packaging to downstream package maintainers.

@devnote-dev
Copy link
Author

devnote-dev commented Nov 27, 2024

FTR Scoop and Linux would all depend on a GitHub release being available, so simply downloading a static build and making it executable would be possible in all cases.

On the package manager side of things, there does need to be a configuration. Scoop is literally just a JSON file, whereas APT is a bit more work (and I have no idea about Pacman/AUR).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants