Skip to content

[Enhancement] Automated Source Code Optimisation #8

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

Open
kevinmatthes opened this issue May 20, 2022 · 18 comments
Open

[Enhancement] Automated Source Code Optimisation #8

kevinmatthes opened this issue May 20, 2022 · 18 comments

Comments

@kevinmatthes
Copy link
Contributor

This issue originates from #6 and was created in order to mark it as an open task.

It is possible to automate the application of both cargo fmt and cargo clippy --fix to the source code. This issue is intended to discuss the options of realisation and the suggested implementation in further detail.

@kevinmatthes
Copy link
Contributor Author

The basic options are:

  • configuration of a GitHub Action
  • configuration of a pre-commit check
  • creation of a Git alias therefore

All of them have their pros and cons. The minimal solution would be a Git alias whereas the Action would solve the problems automatically at the cost of a slightly higher maintenance effort.

@wert007
Copy link
Owner

wert007 commented May 20, 2022

I actually don't really know all these options, which would you recommend and why? What are the differences between the approaches?

@kevinmatthes
Copy link
Contributor Author

With the Git alias, you define a pseudo-command for Git to execute. Just like git add and git commit are Git commands, one could define git rust to automatically invoke the Rust optimisations and persist them in a new commit with a descriptive message. I have collected some of the most important in https://github.com/kevinmatthes/routines/blob/main/git-alias.m, if you want to take a look at further examples. The pros are that

  • this option is lightweight and minimal invasive regarding the repository it is applied to,
  • no security leaks originate from this,
  • no resources of GitHub are consumed, and
  • it is portable with minimal effort.

The cons are that

  • you need to port it with a shell script or something similar,
  • you need to invoke it yourself, and
  • you need to check whether its identifier might collide with other Git commands for Rust.

With a pre-commit check, you define a shell script to check your code and save this script in the .git directory. If something is not optimised, you are not allowed to commit. I consider it rather painful to set it up, but it is an option after all, so I mentioned it. The pros are that

  • it works -- as soon as the script works --, and
  • it prevents commits for unoptimised code.

The cons are that

  • you need to maintain a further source file -- including testing, reasoning and deployment --,
  • it has to be set up in each Git clone by hand,
  • the script needs to be portable for every operating system since discriminating someone due to their OS is not an option, and
  • important and urgent changes might be lost in the hurry when they are not optimised.

The GitHub Action is basically that what I suggested in #5. You just alter the file name, behaviour, required actions and permissions and so on, and it will enhance your commits automatically. The pros are that

  • you do not need to care for optimisation because GitHub does for you, and
  • your repository is always in the state you want to have it.

The cons are that

  • you need to care for the security of your action,
  • the action consumes some of your GitHub resources,
  • the configuration might be difficult,
  • you need to install action updates manually, and
  • you need to audit the actions you use before you use them.

@wert007
Copy link
Owner

wert007 commented May 21, 2022

Regarding the GitHub Action, what do you mean by

  • you need to install action updates manually, and
  • you need to audit the actions you use before you use them.

exactly?

@kevinmatthes
Copy link
Contributor Author

You need to pass a version information of the action you are about to apply. This might happen by naming the branch's identifier (namespace/action@branch), the most recent version's tag (namespace/action@tag001) or a commit hash (namespace/action@1234567890abcdef1234567890abcdef12345678).

Only with the latter method, you can be sure that you do not receive updates without any further notice. Something bad that might happen with your workflow is that it breaks after an automatic update. So for every new version, you need to lookup its commit hash to intentionally install exactly the new version you want. ("updating manually")

With "auditing" I mean that should take a look at the definition of the action to be sure that it does nothing in addition it should rather not do. You need to make sure that the action's behaviour is what you want and expect. This also correlates with the manual updates.

@wert007
Copy link
Owner

wert007 commented May 22, 2022

I'm sorry, I still do not quite get it. I think I'll have to look at it myself some time. But I don't know when I'll have time for that.. 😅

@kevinmatthes
Copy link
Contributor Author

The official GitHub Documentation on this topic, search for "Security Hardening", is a good starting point.

I would like to suggest to leave this point open for one of the next Pull Requests.

@wert007
Copy link
Owner

wert007 commented May 22, 2022

Yeah like I said, I think I will dig into this myself. I will also set it up in the process of understanding everything about it. 😄

I leave this issue open, until I got to it.

@kevinmatthes
Copy link
Contributor Author

When trying out this feature, remember to always set the permissions of the one-time repository token accordingly.

@kevinmatthes
Copy link
Contributor Author

Another important point: when pushing to a GitHub repository with at least one GitHub Action configured, you need the workflow scope for your authentication token.

@kevinmatthes
Copy link
Contributor Author

I found another option: a new sub-command for Cargo! This would be a standalone executable named cargo-something which performs the desired optimisations and might also commit, if possible, whenever one would call cargo something.

I intended something to be a placeholder and would like to suggest optimise to be a sane name for this command. What do you think about it, @wert007?

@wert007
Copy link
Owner

wert007 commented Jun 4, 2022

Hmm, actually I think, that would have a few drawbacks, since e.g. I personally often use git commit -p or all my comment about git commit --amend would be harder to realise, if we actually expect people to not use git commit itself. And since both cargo fmt and cargo clippy are already subcommands of cargo, I do not think there needs to be one, that combines those two.

@kevinmatthes
Copy link
Contributor Author

The auto-commit behaviour was just an optional suggestion. You could opt-in this feature, if required, for instance. A new commit would also originate from an optimisation GitHub Action so basically, the tool would automate or somehow abbreviate the following common workflow with the same effects as the GitHub Action:

cargo fmt
cargo c
cargo clippy --fix --allow-dirty --allow-staged

Committing or even just adding it to the staging area would become an optional service command depending on whether an according option is set. This is an advantage in contrast to the GitHub Action since you decide whether you commit or not whereas the GitHub Action would always commit.

@kevinmatthes
Copy link
Contributor Author

Working prototype: https://github.com/kevinmatthes/cargo-optimise.

Install with cargo install --git https://github.com/kevinmatthes/cargo-optimise and run with cargo optimise. Yes, there is no hyphen necessary.

@kevinmatthes
Copy link
Contributor Author

https://github.com/kevinmatthes/cargo-optimise: now less chatty, by default.

@kevinmatthes
Copy link
Contributor Author

By the way: sysexits has a working example of GitHub Actions to at least lint and validate the format of the Rust source files. https://github.com/sorairolake/sysexits-rs/blob/develop/.github/workflows/CI.yaml

@wert007
Copy link
Owner

wert007 commented Jun 9, 2022

Yeah, I really have to look into that. But I will probably first try it out in some test repository. 😅

@kevinmatthes
Copy link
Contributor Author

If you already installed cargo-optimise, remove it with cargo uninstall cargo-optimise. The latest version does not work anymore with Cargo as intended. Hence, I renamed it to rs-optimise.

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

No branches or pull requests

2 participants