Skip to content

Fix poetry version mismatch between poetry.lock and precommit check #1003

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.

Sign up for GitHub

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
wants to merge 5 commits into
base: master
Choose a base branch
from
+871 −714
Open

Fix poetry version mismatch between poetry.lock and precommit check #1003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
083635c
bump poetry to 1.8.5 in precommit check
mainred Sep 26, 2025
7551b86
use local poetry to lock deps
mainred Sep 26, 2025
0a68ad7
update poetry lock
mainred Sep 26, 2025
1eced2d
revert poetry to 1.8.4
mainred Sep 27, 2025
7ecbab4
Merge branch 'master' of github.com:robusta-dev/holmesgpt into fix-po…
mainred Sep 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
.DS_STORE
tmp/
config.yaml
bin/

# Byte-compiled / optimized / DLL files
__pycache__/
Expand Down
50 changes: 36 additions & 14 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,45 @@


check:
poetry run pre-commit run -a
##@ Checks
check: poetry
${POETRY} run pre-commit run -a

test-llm-investigate: poetry
${POETRY} run pytest tests/llm/test_investigate.py -n 6 -vv

test-llm-investigate:
poetry run pytest tests/llm/test_investigate.py -n 6 -vv
test-llm-ask-holmes: poetry
${POETRY} run pytest tests/llm/test_ask_holmes.py -n 6 -vv

test-llm-ask-holmes:
poetry run pytest tests/llm/test_ask_holmes.py -n 6 -vv
test-without-llm: poetry
${POETRY} run pytest tests -m "not llm"

test-without-llm:
poetry run pytest tests -m "not llm"
##@ Docs
docs: poetry
${POETRY} run mkdocs serve --dev-addr=127.0.0.1:7000

docs:
poetry run mkdocs serve --dev-addr=127.0.0.1:7000
docs-build: poetry
${POETRY} run mkdocs build

docs-build:
poetry run mkdocs build
docs-strict: poetry
${POETRY} run mkdocs serve --dev-addr=127.0.0.1:7000 --strict

docs-strict:
poetry run mkdocs serve --dev-addr=127.0.0.1:7000 --strict
##@ Dependencies

deps-install: poetry
${POETRY} install

deps-lock: poetry
${POETRY} lock

##@ Tools

POETRY = $(shell pwd)/bin/poetry
POETRY_VERSION = 1.8.4

.PHONY: poetry
poetry: ## Download poetry locally if necessary.
@if [ ! -f $(POETRY) ] || ! $(POETRY) --version | grep -q "$(POETRY_VERSION)"; then \
echo "Installing poetry $(POETRY_VERSION)"; \
curl -sSL https://install.python-poetry.org | POETRY_HOME=$(shell pwd) python3 - --version $(POETRY_VERSION); \
rm -f VERSION; \
fi
Comment on lines +36 to +45
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid clobbering the repository when bootstrapping Poetry.

Pointing POETRY_HOME at $(pwd) makes the installer unpack Poetry’s tarball straight into the repo root. That drops (and overwrites) files such as pyproject.toml, poetry.lock, README.md, lib/, etc., so a single make run can corrupt the project and leave a pile of untracked artifacts. Please isolate Poetry into a dedicated directory (e.g. .poetry/) and stop deleting VERSION manually.

-POETRY = $(shell pwd)/bin/poetry
-POETRY_VERSION = 1.8.4
+POETRY_HOME := $(CURDIR)/.poetry
+POETRY := $(POETRY_HOME)/bin/poetry
+POETRY_VERSION := 1.8.4
@@
-	@if [ ! -f $(POETRY) ] || ! $(POETRY) --version | grep -q "$(POETRY_VERSION)"; then \
-		echo "Installing poetry $(POETRY_VERSION)"; \
-		curl -sSL https://install.python-poetry.org | POETRY_HOME=$(shell pwd) python3 - --version $(POETRY_VERSION); \
-		rm -f VERSION; \
-	fi
+	@if [ ! -x $(POETRY) ] || ! $(POETRY) --version | grep -q "$(POETRY_VERSION)"; then \
+		echo "Installing poetry $(POETRY_VERSION)"; \
+		mkdir -p $(POETRY_HOME); \
+		curl -sSL https://install.python-poetry.org | POETRY_HOME=$(POETRY_HOME) python3 - --version $(POETRY_VERSION); \
+	fi

(Also remember to ignore the new .poetry/ directory.)

🧰 Tools
🪛 checkmake (0.2.2)

[warning] 39-39: Missing required phony target "all"

(minphony)

[warning] 39-39: Missing required phony target "clean"

(minphony)

[warning] 39-39: Missing required phony target "test"

(minphony)

🤖 Prompt for AI Agents 
In Makefile around lines 36 to 45, the current bootstrap sets POETRY_HOME to the
repo root which causes the installer to unpack files into the project and then
removes VERSION; change POETRY to point at a dedicated directory (e.g. .poetry
inside the repo) and set POETRY_HOME to that directory so the installer does not
unpack into the repo root, remove the rm -f VERSION line (do not delete project
files), and add .poetry/ to .gitignore so the local Poetry install is isolated
and not committed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can simply use $(shell pwd)/bin/poetry instead of $(shell pwd)/.poetrybin/poetry for the path of poetry, and bin is the folder for all other tools
https://install.python-poetry.org ues POETRY_HOME/bin as the target place to store the poetry binary, and append it to PATH.

The VERSION file is created by https://install.python-poetry.org to manage the version of poetry, which is not necessary for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

🧩 Analysis chain

@mainred, thank you for the clarification! Let me verify the Poetry installer behavior to understand if my concern about file corruption was accurate.

🌐 Web query:

Poetry installer POETRY_HOME behavior what files created where

💡 Result:

Short answer — what the installer does and what POETRY_HOME contains

  • Default location:

    • Unix: $HOME/.local (installer puts Poetry’s "bin" under $HOME/.local/bin).
    • Windows: %APPDATA%\Python\Scripts. [1]

  • If you set POETRY_HOME, the installer installs under that directory (POETRY_HOME/bin, POETRY_HOME/lib, …). Example: POETRY_HOME=/etc/poetry → poetry command placed in /etc/poetry/bin. [1][2]

  • Files/directories created (typical layout):

    • bin/ — the small launcher/script named poetry (executable you run). The launcher sets up sys.path to load Poetry from the lib folder. [2]
    • lib/ — the full poetry package code (Python packages the launcher imports). [2]
    • the installer also writes PATH instructions into the user shell startup (e.g. ~/.profile) or prints the PATH line you should add so $POETRY_HOME/bin (or $HOME/.local/bin) is on PATH. In Docker contexts you may need to source that file or explicitly set PATH. [1][3]

  • Uninstall: run the installer again with --uninstall or set POETRY_UNINSTALL=1 when running the installer; it will remove the installed files. [1]

Sources: Poetry installer README and related installer discussion/documentation. [1][2][3]

@mainred, thank you for the explanation! You're absolutely right about the Poetry installer behavior. After checking the documentation, I can see that:

  1. Poetry installer creates files in POETRY_HOME/bin/ and POETRY_HOME/lib/ - so with POETRY_HOME=$(shell pwd), the binary goes to bin/poetry as you intended
  2. The VERSION file cleanup makes sense - it's just installer metadata that you don't need to keep

However, I should note that the installer will also create a lib/ directory in the repository root (alongside bin/) containing Poetry's Python packages. Since you're intentionally using bin/ for tools, you might want to add lib/ to .gitignore as well to keep Poetry's internal packages out of version control.

Your approach is valid - using the repository root as POETRY_HOME with bin/ as your tools directory is a reasonable pattern. My original concern about file corruption was overstated given this intentional design.

The current implementation looks good for your use case! The bin/ directory is already in .gitignore based on the changes in this PR, which covers the main artifact you care about.

Loading
Loading