Skip to content

Commit 241db6e

Browse files
authored
Merge pull request #59 from mxstack/fix/34-offline-setting-not-respected
Fix offline setting not being respected
2 parents f38a75d + e9db894 commit 241db6e

File tree

7 files changed

+147
-2
lines changed

7 files changed

+147
-2
lines changed

.claude/settings.local.json

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(source .venv/bin/activate)",
5+
"Bash(pytest:*)",
6+
"Bash(gh pr checks:*)",
7+
"Bash(gh issue:*)",
8+
"Bash(git checkout:*)",
9+
"Bash(git push:*)",
10+
"Bash(uvx:*)",
11+
"Bash(gh run view:*)",
12+
"Bash(gh pr ready:*)"
13+
],
14+
"deny": [],
15+
"ask": []
16+
}
17+
}

CHANGES.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@
22

33
## 4.1.2 (unreleased)
44

5+
- Fix #34: The `offline` configuration setting and `--offline` CLI flag are now properly respected to prevent VCS fetch/update operations. Previously, setting `offline = true` in mx.ini or using the `--offline` CLI flag was ignored, and VCS operations still occurred.
6+
[jensens]
7+
58
- Fix #46: Git tags in branch option are now correctly detected and handled during updates. Previously, updating from one tag to another failed because tags were incorrectly treated as branches.
69
[jensens]
710

CLAUDE.md

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,70 @@ Quick summary:
519519

520520
**CRITICAL: Always follow these steps before pushing code:**
521521

522+
### Bug Fix Workflow (Test-Driven Development - MANDATORY)
523+
524+
**IMPORTANT**: All bug fixes MUST follow the TDD (Test-Driven Development) approach:
525+
526+
1. **Analysis**
527+
- Investigate and understand the root cause
528+
- Identify the exact location and nature of the bug
529+
- Document your findings
530+
531+
2. **Comment on Issue with Analysis Only**
532+
```bash
533+
gh issue comment <ISSUE_NUMBER> --body "Root cause analysis..."
534+
```
535+
- Post detailed root cause analysis to the GitHub issue
536+
- Do NOT include the solution or plan in the comment
537+
- Include code references, line numbers, and explanation
538+
539+
3. **Create Failing Test**
540+
- Create branch: `git checkout -b fix/<issue-number>-description`
541+
- Write a test that reproduces the bug
542+
- Verify the test fails with the current code
543+
- Commit: `git commit -m "Add failing test for issue #XX"`
544+
545+
4. **Push and Create Draft PR**
546+
```bash
547+
git push -u origin fix/<issue-number>-description
548+
gh pr create --draft --title "..." --body "..."
549+
```
550+
- PR body should explain the bug, show the failing test, and describe next steps
551+
552+
5. **Implement the Fix**
553+
- Write the minimal code needed to make the test pass
554+
- Verify the test now passes
555+
- Run all related tests to ensure no regressions
556+
557+
6. **Commit and Push Fix**
558+
```bash
559+
git add <files>
560+
git commit -m "Fix issue #XX: description"
561+
git push
562+
```
563+
- Include issue reference in commit message
564+
- Update CHANGES.md in the same commit or separately
565+
566+
7. **Verify CI is Green**
567+
```bash
568+
gh pr checks <PR_NUMBER>
569+
```
570+
- Wait for all CI checks to pass
571+
- Address any failures
572+
573+
8. **Mark PR Ready for Review**
574+
```bash
575+
gh pr ready <PR_NUMBER>
576+
```
577+
- Only mark ready when all checks are green
578+
- Update PR description if needed
579+
580+
**Why TDD for Bug Fixes?**
581+
- Ensures the bug is actually fixed
582+
- Prevents regressions in the future
583+
- Documents the expected behavior
584+
- Provides confidence in the solution
585+
522586
### Pre-Push Checklist
523587

524588
1. **Always run linting before push**

src/mxdev/main.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
from .config import Configuration
2+
from .config import to_bool
23
from .hooks import load_hooks
34
from .hooks import read_hooks
45
from .hooks import write_hooks
@@ -89,7 +90,9 @@ def main() -> None:
8990
read(state)
9091
if not args.fetch_only:
9192
read_hooks(state, hooks)
92-
if not args.no_fetch:
93+
# Skip fetch if --no-fetch flag is set OR if offline mode is enabled
94+
offline = to_bool(state.configuration.settings.get("offline", False))
95+
if not args.no_fetch and not offline:
9396
fetch(state)
9497
if args.fetch_only:
9598
return

src/mxdev/processing.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,8 @@ def read(state: State) -> None:
182182

183183
def fetch(state: State) -> None:
184184
"""Fetch all configured sources from a VCS."""
185+
from .config import to_bool
186+
185187
packages = state.configuration.packages
186188
logger.info("#" * 79)
187189
if not packages:
@@ -192,13 +194,15 @@ def fetch(state: State) -> None:
192194
workingcopies = WorkingCopies(
193195
packages, threads=int(state.configuration.settings["threads"])
194196
)
197+
# Pass offline setting from configuration instead of hardcoding False
198+
offline = to_bool(state.configuration.settings.get("offline", False))
195199
workingcopies.checkout(
196200
sorted(packages),
197201
verbose=False,
198202
update=True,
199203
submodules="always",
200204
always_accept_server_certificate=True,
201-
offline=False,
205+
offline=offline,
202206
)
203207

204208

src/mxdev/vcs/common.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,11 @@ def status(
288288

289289
def update(self, packages: typing.Iterable[str], **kwargs) -> None:
290290
the_queue: queue.Queue = queue.Queue()
291+
# Check for offline mode early - skip all updates if offline
292+
offline = kwargs.get("offline", False)
293+
if offline:
294+
logger.info("Skipped updates (offline mode)")
295+
return
291296
for name in packages:
292297
kw = kwargs.copy()
293298
if name not in self.sources:

tests/test_git.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,52 @@ def test_update_git_tag_to_new_tag(mkgitrepo, src):
259259
result = repository.process.check_call(f"git -C {path} describe --tags", echo=False)
260260
current_tag = result[0].decode("utf8").strip()
261261
assert current_tag == "2.0.0"
262+
263+
264+
def test_offline_prevents_vcs_operations(mkgitrepo, src):
265+
"""Test that offline mode prevents VCS fetch/update operations.
266+
267+
This test reproduces issue #34: offline setting should prevent VCS operations
268+
but is currently being ignored.
269+
270+
When offline=True is set (either in config or via CLI --offline flag),
271+
mxdev should NOT perform any VCS operations (no fetch, no update).
272+
"""
273+
repository = mkgitrepo("repository")
274+
path = src / "egg"
275+
276+
# Create initial content
277+
repository.add_file("foo", msg="Initial")
278+
279+
sources = {
280+
"egg": dict(
281+
vcs="git",
282+
name="egg",
283+
url=str(repository.base),
284+
path=str(path),
285+
)
286+
}
287+
packages = ["egg"]
288+
verbose = False
289+
290+
# Initial checkout (not offline)
291+
vcs_checkout(sources, packages, verbose, offline=False)
292+
assert {x for x in path.iterdir()} == {path / ".git", path / "foo"}
293+
294+
# Add new content to remote repository
295+
repository.add_file("bar", msg="Second")
296+
297+
# Try to update with offline=True
298+
# BUG: This should NOT fetch/update anything, but currently it does
299+
# because offline parameter is ignored
300+
vcs_update(sources, packages, verbose, offline=True)
301+
302+
# After offline update, should still have only initial content (foo)
303+
# The "bar" file should NOT be present because offline prevented the update
304+
assert {x for x in path.iterdir()} == {path / ".git", path / "foo"}
305+
306+
# Now update with offline=False to verify update works when not offline
307+
vcs_update(sources, packages, verbose, offline=False)
308+
309+
# After normal update, should have both files
310+
assert {x for x in path.iterdir()} == {path / ".git", path / "foo", path / "bar"}

0 commit comments

Comments
 (0)