X Tutup
Skip to content

Add Rust mirrors#34

Merged
chriskuehl merged 2 commits intopre-commit:masterfrom
chriskuehl:rust
May 25, 2018
Merged

Add Rust mirrors#34
chriskuehl merged 2 commits intopre-commit:masterfrom
chriskuehl:rust

Conversation

@chriskuehl
Copy link
Member

This goes along with pre-commit/pre-commit#751 (and probably shouldn't be merged first).

Testing Done™

$ git init shellharden-mirror
Initialized empty Git repository in /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror/.git/

$ python -m pre_commit_mirror_maker.main --language rust --package-name shellharden --types shell --entry shellharden --args='--replace'  shellharden-mirror[master (root-commit) ec6a78a] Mirror: 3.1.0
 4 files changed, 16 insertions(+)
 create mode 100644 .pre-commit-hooks.yaml
 create mode 100644 .version
 create mode 100644 Cargo.toml
 create mode 100644 main.rs
$ pre-commit try-repo -v /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror shellharden
===============================================================================
Using config:
===============================================================================
repos:
-   repo: /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror
    rev: ec6a78a94c75512411918da0b2cf7845150f80f8
    hooks:
    -   id: shellharden
===============================================================================
[INFO] Initializing environment for /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror.
[INFO] Initializing environment for /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror:cli:shellharden:3.1.0.
[INFO] Installing environment for /home/ckuehl/proj/pre-commit-mirror-maker/shellharden-mirror.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
[shellharden] shellharden................................................Failed
hookid: shellharden

Files were modified by this hook.

(venv) [1] ckuehl@raziel:~/proj/identify$ git diff | head -n20
diff --git a/zgrep b/zgrep
index f4ff2f5..d550c72 100755
--- a/zgrep
+++ b/zgrep
@@ -23,10 +23,10 @@
 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.

 bindir='/bin'
-case $1 in
---__bindir) bindir=${2?}; shift; shift;;
+case "$1" in
+--__bindir) bindir="${2?}"; shift; shift;;
 esac
-PATH=$bindir:$PATH
+PATH="$bindir:$PATH"

 grep='${GREP-'\''/bin/grep'\''}'

@@ -60,53 +60,53 @@ no_filename=0
 with_filename=0

@chriskuehl chriskuehl requested a review from asottile May 24, 2018 04:58
language: {language}
'{match_key}': {match_val}
args: {args}
additional_dependencies: {additional_dependencies}
Copy link
Member Author

Choose a reason for hiding this comment

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

How important is the minimum_pre_commit_version key? Is it worth doing some hackery to get it in for projects which have additional_dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

probably not all that important, it's not exactly a new feature (and will fail for other reasons) 😆


[[bin]]
name = "__fake_cmd"
path = "main.rs"
Copy link
Member Author

Choose a reason for hiding this comment

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

It turns out you can't really make a noop Cargo.toml file, and since we're installing with --bins, you can't really even make a library-only crate (so there would be no binary); cargo is "smart" enough to complain about that.

So this builds a bogus executable which does nothing. Maybe okay?

An alternative could be to make pre-commit's Rust support allow repos with no Cargo.toml (and only use additional_dependencies in that case), but I'm worried about it masking bugs.

Copy link
Member

Choose a reason for hiding this comment

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

this is probably fine, we have to do the same dance for go as well

'.',
version='0.6.2', language='python', name='yapf', entry='yapf',
match_key='files', match_val=r'\.py$', args='["-i"]',
additional_dependencies=['scikit-learn'],
Copy link
Member

Choose a reason for hiding this comment

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

lol you troll

Whoops, it was ordering them wrong.
@chriskuehl chriskuehl merged commit f5ae7b8 into pre-commit:master May 25, 2018
@chriskuehl chriskuehl deleted the rust branch May 25, 2018 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup