diff --git a/CHANGES.md b/CHANGES.md index 080d459..cefa88b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,9 @@ ## 4.1.2 (unreleased) +- Fix #35: Add `smart-threading` configuration option to prevent overlapping credential prompts when using HTTPS URLs. When enabled (default), HTTPS packages are processed serially first to ensure clean credential prompts, then other packages are processed in parallel for speed. Can be disabled with `smart-threading = false` if you have credential helpers configured. + [jensens] + - 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. [jensens] diff --git a/CLAUDE.md b/CLAUDE.md index 96a5347..357fac6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -599,13 +599,22 @@ Quick summary: - Create unreleased section if it doesn't exist - Include issue number when applicable -3. **Run relevant tests locally** +3. **Always check and update documentation** + - **README.md**: Update configuration tables, usage examples, or feature descriptions + - **EXTENDING.md**: Update if hooks API changed + - **RELEASE.md**: Update if release process changed + - Check if new configuration options need documentation + - Check if new features need usage examples + - Update any affected sections (don't just append) + - **MANDATORY**: After any code change that adds/modifies features or configuration, verify documentation is updated + +4. **Run relevant tests locally** ```bash source .venv/bin/activate pytest tests/test_*.py -v ``` -4. **Check CI status before marking PR ready** +5. **Check CI status before marking PR ready** ```bash gh pr checks ``` diff --git a/README.md b/README.md index 909c7b9..9caaa2c 100644 --- a/README.md +++ b/README.md @@ -84,11 +84,23 @@ The **main section** must be called `[settings]`, even if kept empty. |--------|-------------|---------| | `default-target` | Target directory for VCS checkouts | `./sources` | | `threads` | Number of parallel threads for fetching sources | `4` | +| `smart-threading` | Process HTTPS packages serially to avoid overlapping credential prompts (see below) | `True` | | `offline` | Skip all VCS fetch operations (handy for offline work) | `False` | | `default-install-mode` | Default `install-mode` for packages: `direct` or `skip` | `direct` | | `default-update` | Default update behavior: `yes` or `no` | `yes` | | `default-use` | Default use behavior (when false, sources not checked out) | `True` | +##### Smart Threading + +When `smart-threading` is enabled (default), mxdev uses a two-phase approach to prevent credential prompts from overlapping: + +1. **Phase 1**: HTTPS packages are processed serially (one at a time) to ensure clean, visible credential prompts +2. **Phase 2**: Remaining packages (SSH, local) are processed in parallel for speed + +This solves the problem where parallel git operations would cause multiple credential prompts to overlap, making it confusing which package needs credentials. + +**When to disable**: Set `smart-threading = false` if you have git credential helpers configured (e.g., credential cache, credential store) and never see prompts. + #### Package Overrides ##### `version-overrides` diff --git a/src/mxdev/config.py b/src/mxdev/config.py index e60a7f3..a719cc4 100644 --- a/src/mxdev/config.py +++ b/src/mxdev/config.py @@ -46,6 +46,10 @@ def __init__( else: settings["threads"] = "4" + # Set default for smart-threading (process HTTPS packages serially to avoid + # overlapping credential prompts) + settings.setdefault("smart-threading", "true") + mode = settings.get("default-install-mode", "direct") if mode not in ["direct", "skip"]: raise ValueError("default-install-mode must be one of 'direct' or 'skip'") diff --git a/src/mxdev/processing.py b/src/mxdev/processing.py index e01ac38..b11e0bb 100644 --- a/src/mxdev/processing.py +++ b/src/mxdev/processing.py @@ -191,8 +191,11 @@ def fetch(state: State) -> None: return logger.info("# Fetch sources from VCS") + smart_threading = to_bool(state.configuration.settings.get("smart-threading", True)) workingcopies = WorkingCopies( - packages, threads=int(state.configuration.settings["threads"]) + packages, + threads=int(state.configuration.settings["threads"]), + smart_threading=smart_threading, ) # Pass offline setting from configuration instead of hardcoding False offline = to_bool(state.configuration.settings.get("offline", False)) diff --git a/src/mxdev/vcs/common.py b/src/mxdev/vcs/common.py index 7f9bebd..619ec68 100644 --- a/src/mxdev/vcs/common.py +++ b/src/mxdev/vcs/common.py @@ -163,12 +163,41 @@ def get_workingcopytypes() -> typing.Dict[str, typing.Type[BaseWorkingCopy]]: class WorkingCopies: - def __init__(self, sources: typing.Dict[str, typing.Dict], threads=5): + def __init__( + self, + sources: typing.Dict[str, typing.Dict], + threads=5, + smart_threading=True, + ): self.sources = sources self.threads = threads + self.smart_threading = smart_threading self.errors = False self.workingcopytypes = get_workingcopytypes() + def _separate_https_packages( + self, packages: typing.List[str] + ) -> typing.Tuple[typing.List[str], typing.List[str]]: + """Separate HTTPS packages from others for smart threading. + + Returns (https_packages, other_packages) + """ + https_packages = [] + other_packages = [] + + for name in packages: + if name not in self.sources: + other_packages.append(name) + continue + source = self.sources[name] + url = source.get("url", "") + if url.startswith("https://"): + https_packages.append(name) + else: + other_packages.append(name) + + return https_packages, other_packages + def process(self, the_queue: queue.Queue) -> None: if self.threads < 2: worker(self, the_queue) @@ -187,6 +216,43 @@ def process(self, the_queue: queue.Queue) -> None: sys.exit(1) def checkout(self, packages: typing.Iterable[str], **kwargs) -> None: + # Smart threading: process HTTPS packages serially to avoid overlapping prompts + packages_list = list(packages) + if self.smart_threading and self.threads > 1: + https_pkgs, other_pkgs = self._separate_https_packages(packages_list) + if https_pkgs and other_pkgs: + logger.info( + "Smart threading: processing %d HTTPS package(s) serially...", + len(https_pkgs), + ) + # Save original thread count and process HTTPS packages serially + original_threads = self.threads + self.threads = 1 + self._checkout_impl(https_pkgs, **kwargs) + self.threads = original_threads + # Process remaining packages in parallel + logger.info( + "Smart threading: processing %d other package(s) in parallel...", + len(other_pkgs), + ) + self._checkout_impl(other_pkgs, **kwargs) + return + elif https_pkgs: + logger.info( + "Smart threading: processing %d HTTPS package(s) serially...", + len(https_pkgs), + ) + original_threads = self.threads + self.threads = 1 + self._checkout_impl(packages_list, **kwargs) + self.threads = original_threads + return + + # Normal processing (smart_threading disabled or threads=1) + self._checkout_impl(packages_list, **kwargs) + + def _checkout_impl(self, packages: typing.List[str], **kwargs) -> None: + """Internal implementation of checkout logic.""" the_queue: queue.Queue = queue.Queue() if "update" in kwargs and not isinstance(kwargs["update"], bool): if kwargs["update"].lower() in ("true", "yes", "on", "force"): @@ -287,12 +353,50 @@ def status( sys.exit(1) def update(self, packages: typing.Iterable[str], **kwargs) -> None: - the_queue: queue.Queue = queue.Queue() # Check for offline mode early - skip all updates if offline offline = kwargs.get("offline", False) if offline: logger.info("Skipped updates (offline mode)") return + + # Smart threading: process HTTPS packages serially to avoid overlapping prompts + packages_list = list(packages) + if self.smart_threading and self.threads > 1: + https_pkgs, other_pkgs = self._separate_https_packages(packages_list) + if https_pkgs and other_pkgs: + logger.info( + "Smart threading: updating %d HTTPS package(s) serially...", + len(https_pkgs), + ) + # Save original thread count and process HTTPS packages serially + original_threads = self.threads + self.threads = 1 + self._update_impl(https_pkgs, **kwargs) + self.threads = original_threads + # Process remaining packages in parallel + logger.info( + "Smart threading: updating %d other package(s) in parallel...", + len(other_pkgs), + ) + self._update_impl(other_pkgs, **kwargs) + return + elif https_pkgs: + logger.info( + "Smart threading: updating %d HTTPS package(s) serially...", + len(https_pkgs), + ) + original_threads = self.threads + self.threads = 1 + self._update_impl(packages_list, **kwargs) + self.threads = original_threads + return + + # Normal processing (smart_threading disabled or threads=1) + self._update_impl(packages_list, **kwargs) + + def _update_impl(self, packages: typing.List[str], **kwargs) -> None: + """Internal implementation of update logic.""" + the_queue: queue.Queue = queue.Queue() for name in packages: kw = kwargs.copy() if name not in self.sources: