https://pantsbuild.org/ logo
Join Slack
Powered by
Does anyone actually use the recommended git commi...
# general
b
Does anyone actually use the recommended git commit hooks for developing on the pants repo? I'm seeing a lot of shellcheck errors that aren't related to the change I'm making
e
I do. I just fixed exactly 1 shellcheck error.
There should be no more errors if you sync up to that.
Perhaps ... are you stuck on a master upstream base branch? We switched to main this weel.
b
I did do the 
main
update and I think it's correct, let me rebase and see if it still repros
Yeah I still repro. The error:
Copy code
* Checking shell scripts via shellcheck

In ./build-support/bin/create_s3_index_file.sh line 19:
for obj in $(aws s3 ls "s3://${VERSION_DIR}/" | grep "\.whl" | awk '{print $4}'); do
                                                      ^-- SC1117: Backslash is literal in "\.". Prefer explicit escaping: "\\.".


In ./build-support/bin/create_s3_index_file.sh line 26:
  echo "<br><a href=\"${URL//+/%2B}\">${obj}</a>\n";
                                                ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In ./build-support/bin/rust/calculate_engine_hash.sh line 40:
   | grep -v -E -e "/BUILD$" -e "/[^/]*\.md$" \
                                       ^-- SC1117: Backslash is literal in "\.". Prefer explicit escaping: "\\.".


In ./build-support/common.sh line 7:
CLEAR_LINE="\x1b[K"
            ^-- SC1117: Backslash is literal in "\x". Prefer explicit escaping: "\\x".


In ./build-support/common.sh line 8:
COLOR_BLUE="\x1b[34m"
            ^-- SC1117: Backslash is literal in "\x". Prefer explicit escaping: "\\x".


In ./build-support/common.sh line 9:
COLOR_RED="\x1b[31m"
           ^-- SC1117: Backslash is literal in "\x". Prefer explicit escaping: "\\x".


In ./build-support/common.sh line 10:
COLOR_GREEN="\x1b[32m"
             ^-- SC1117: Backslash is literal in "\x". Prefer explicit escaping: "\\x".


In ./build-support/common.sh line 11:
COLOR_RESET="\x1b[0m"
             ^-- SC1117: Backslash is literal in "\x". Prefer explicit escaping: "\\x".


In ./build-support/common.sh line 19:
  (($# > 0)) && log "\n${COLOR_RED}$*${COLOR_RESET}"
                     ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In ./build-support/common.sh line 24:
  (($# > 0)) && log "\n${COLOR_GREEN}$*${COLOR_RESET}"
                     ^-- SC1117: Backslash is literal in "\n". Prefer explicit escaping: "\\n".


In ./build-support/common.sh line 46:
  echo -en "travis_fold:${action}:${slug}\r${CLEAR_LINE}"
                                         ^-- SC1117: Backslash is literal in "\r". Prefer explicit escaping: "\\r".


In ./build-support/githooks/prepare-commit-msg line 38:
grep "\[ci skip-rust\]" "${COMMIT_MSG_FILEPATH}" > /dev/null
      ^-- SC1117: Backslash is literal in "\[". Prefer explicit escaping: "\\[".
                    ^-- SC1117: Backslash is literal in "\]". Prefer explicit escaping: "\\]".


In ./build-support/githooks/prepare-commit-msg line 40:
grep "\[ci skip-build-wheels\]" "${COMMIT_MSG_FILEPATH}" > /dev/null
      ^-- SC1117: Backslash is literal in "\[". Prefer explicit escaping: "\\[".
                            ^-- SC1117: Backslash is literal in "\]". Prefer explicit escaping: "\\]".

Please fix the above errors and run again.
My tree:
Copy code
$ git log
commit d42ba3751a9e835ba4b2818c09c17555d5565e86 (HEAD -> main)
Author: Patrick Lawson <patrick.a.lawson@gmail.com>
Date:   Sat Mar 20 22:47:47 2021 -0400

    WIP: JVM repl with coursier resolve
    
    # Rust tests and lints will be skipped. Delete if not intended.
    [ci skip-rust]
    
    # Building wheels and fs_util will be skipped. Delete if not intended.
    [ci skip-build-wheels]

commit 4210c6c9dece3ff6d0c00b318b880bd6050c3e96 (origin/main)
Author: John Sirois <john.sirois@gmail.com>
Date:   Sun Mar 21 13:01:10 2021 -0700

    Use append_only_caches in Pex processes. (#11760)
(My change and my staged changes definitely don't touch any of those files)
Given the way 
shellcheck
is called, it seems plausible that version drift is causing different results
👍 1
Copy code
$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.5.0
e
Yeah:
Copy code
$ shellcheck --version
ShellCheck - shell script analysis tool
version: 0.7.1
license: GNU General Public License, version 3
website: <https://www.shellcheck.net>
I'll poke at this.
👍 1
b
FWIW, that was from a plain 
apt install shellcheck
on Debian 10.8
h
Fwit, the example-plugin repo has a Pants implementation of shell check. We could hook it up to Pants to get a pinned version. I only didn't due to not wanting to duplicate across the repos and missing the infrastructure for how we want to publish plugins in a 2.x world
e
Okay, confirmed with 
docker run --rm -it debian:10.8 bash -i
then 
apt install shellcheck python3-dev git curl
in a fresh clone of pants. But grabbing latest shellcheck from 
curl -sSL <https://github.com/koalaman/shellcheck/releases/download/stable/shellcheck-stable.linux.x86_64.tar.xz> -O
and setting up that to win, errors go away.
So Patrick - short term, you can do similar to work around. I have a deep enough stack presently that I'd like to avoid fixing this at the moment if possible.
b
SGTM, it's easy enough to work around
h
Thoughts on Pants gaining official bash support, eg Shellcheck? All the code is already written - it's only whether we a) are okay with violating DRY by copying code across two repos, or b) put the cycles into figuring out what plugins will look like in 2.x, including how to indicate which Pants versions you're compatible with
e
So, I'm confused about the 2.x language. We have the same infra for publishing and consuming as v1. There are no new variables that I'm aware of.
With the new Pex tooling we can resolve plugins, then post-ask which distributons were resolved, and for any pantsbuild.pants confirm version matches us.
That lets plugin publisher just use standard Python install_requires.
h
Iiuc, plugin versioning in v1 world was the same as pantsbuild.pants, eg both were 1.30.0 Instead, I suspect we want to decouple the versioning, which would make it easier for non-pantsbuild plugins to exist without closely tracking Pants. Danny put some thoughts at https://github.com/pantsbuild/pants/issues/10671
e
My point is thats not a new problem. It snot a v2 world issue - its an old one.
Having a pure API dist is still the 1st thing to tackle IMO: https://github.com/pantsbuild/pants/issues/9736
h
That lets plugin publisher just use standard Python install_requires.
Oh, duh. That makes a lot of sense. Right now, the pants_requirement() macro does exact version requirements but that could be adjusted to allow plugin authors to express compatibility more flexibly
e
Yeah - that's fake though. The fact we pin everything in Pants makes flexible versions ~impossible today.
➕ 1
Asher has to go hunt and peck constantly.
h
My point is thats not a new problem. It snot a v2 world issue - its an old one.
Ack, I meant instead that we probably want to figure this old problem out before re-starting to publish plugins and setting some precedent. The only (known) v2 plugin is the Toolchain one
e
OK - yeah, agreed.
5 Views
Open in Slack
PreviousNext
Powered by