From 76484265cd3df48bf98d7866018b69b5e872423b Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 25 Oct 2024 16:51:53 +0200 Subject: [PATCH 1/5] docs: gomod: Drop vendoring flags from the list of CLI flags Fixes: bbd2428543ed72da6d106c7e25b9b627402555fc Signed-off-by: Erik Skultety --- docs/gomod.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/docs/gomod.md b/docs/gomod.md index f26bb9293..1746d20c4 100644 --- a/docs/gomod.md +++ b/docs/gomod.md @@ -91,8 +91,6 @@ The `cachi2 fetch-deps` command accepts the following gomod-related flags: * [--cgo-disable](#--cgo-disable) * [--force-gomod-tidy](#--force-gomod-tidy) -* --gomod-vendor - see [vendoring](#vendoring) -* --gomod-vendor-check - see [vendoring](#vendoring) ### --cgo-disable From bbe4491bd4ebe9dcf29e24dbba314b3fd538b4af Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 25 Oct 2024 16:49:00 +0200 Subject: [PATCH 2/5] docs: gomod: Move CLI flag deprecation section Originally, the section in question was hosted under 'Vendoring' where it should have been under 'gomod flags'. This minor change is going to be even more emphasized with further deprecated gomod flags, like '--force-gomod-tidy' which future patches will deprecate. Signed-off-by: Erik Skultety --- docs/gomod.md | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/docs/gomod.md b/docs/gomod.md index 1746d20c4..30d852792 100644 --- a/docs/gomod.md +++ b/docs/gomod.md @@ -92,6 +92,13 @@ The `cachi2 fetch-deps` command accepts the following gomod-related flags: * [--cgo-disable](#--cgo-disable) * [--force-gomod-tidy](#--force-gomod-tidy) +### Deprecated flags + +* `--gomod-vendor` +* `--gomod-vendor-check` + +Both are deprecated and will have no effect when set. They are only kept for backwards compatibility reasons. + ### --cgo-disable Makes Cachi2 internally disable [cgo](https://pkg.go.dev/cmd/cgo) while processing your Go modules. Typically, you would @@ -117,15 +124,6 @@ We generally discourage vendoring, but Cachi2 does support processing repositori this case, instead of a regular prefetching of dependencies, Cachi2 will only validate if the contents of the vendor directory are consistent with what `go mod vendor` would produce. -### Deprecated flags - -Cachi2's behavior towards vendoring used to be governed by two flags: - -* `--gomod-vendor` -* `--gomod-vendor-check` - -Both are deprecated and will have no effect when set. They are only kept for backwards compatibility reasons. - ## Understanding reported dependencies Cachi2 reports two (arguably three) different types of dependencies in the generated SBOM for your Go modules: From 8a3df6d2d1a7804743d79484843a81175b400de2 Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 25 Oct 2024 16:50:35 +0200 Subject: [PATCH 3/5] docs: gomod: Reword CLI flag deprecation Specifically mention that they're bound to be removed in future releases and not to be kept around forever. Signed-off-by: Erik Skultety --- docs/gomod.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/gomod.md b/docs/gomod.md index 30d852792..3676a1681 100644 --- a/docs/gomod.md +++ b/docs/gomod.md @@ -94,10 +94,11 @@ The `cachi2 fetch-deps` command accepts the following gomod-related flags: ### Deprecated flags -* `--gomod-vendor` -* `--gomod-vendor-check` +* `--gomod-vendor` (deprecated in _v0.11.0_) +* `--gomod-vendor-check` (deprecated in _v0.11.0_) -Both are deprecated and will have no effect when set. They are only kept for backwards compatibility reasons. +All of them are deprecated and will have no effect when set. They are only kept for backwards +compatibility reasons and will be removed in future releases. ### --cgo-disable From fb522d91811e4411fee24734fcbf5da897349aa7 Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 25 Oct 2024 16:52:23 +0200 Subject: [PATCH 4/5] docs: gomod: Deprecate --force-gomod-tidy This flag has always been documented as "questionable" and a candidate for a removal (documented by commit 9e9615c5 ) ever since its introduction. This patch officially deprecates the flag. Signed-off-by: Erik Skultety --- cachi2/core/package_managers/gomod.py | 4 --- cachi2/interface/cli.py | 5 +++- tests/unit/package_managers/test_gomod.py | 36 +++-------------------- 3 files changed, 8 insertions(+), 37 deletions(-) diff --git a/cachi2/core/package_managers/gomod.py b/cachi2/core/package_managers/gomod.py index 17a0c4e00..e095b5e0d 100644 --- a/cachi2/core/package_managers/gomod.py +++ b/cachi2/core/package_managers/gomod.py @@ -1025,7 +1025,6 @@ def _resolve_gomod( modules_in_go_sum = _parse_go_sum(app_dir.join_within_root("go.sum")) # Vendor dependencies if the gomod-vendor flag is set - flags = request.flags if should_vendor: downloaded_modules = _vendor_deps(go, app_dir, bool(go_work), run_params) else: @@ -1035,9 +1034,6 @@ def _resolve_gomod( for obj in load_json_stream(go(["mod", "download", "-json"], run_params, retry=True)) ) - if "force-gomod-tidy" in flags: - go(["mod", "tidy"], run_params) - main_module, workspace_modules = _parse_local_modules( go_work, go, run_params, app_dir, version_resolver ) diff --git a/cachi2/interface/cli.py b/cachi2/interface/cli.py index 4a84dca6d..5e7028087 100644 --- a/cachi2/interface/cli.py +++ b/cachi2/interface/cli.py @@ -160,7 +160,10 @@ def fetch_deps( force_gomod_tidy: bool = typer.Option( False, "--force-gomod-tidy", - help="Run 'go mod tidy' after downloading go dependencies.", + help=( + "DEPRECATED (no longer has any effect when set). " + "Run 'go mod tidy' after downloading go dependencies." + ), ), gomod_vendor: bool = typer.Option( False, diff --git a/tests/unit/package_managers/test_gomod.py b/tests/unit/package_managers/test_gomod.py index 379b42469..6ec20c7c5 100644 --- a/tests/unit/package_managers/test_gomod.py +++ b/tests/unit/package_managers/test_gomod.py @@ -141,13 +141,11 @@ def _parse_go_list_deps_data(data_dir: Path, file_path: str) -> list[ParsedPacka @pytest.mark.parametrize( - "cgo_disable, force_gomod_tidy, has_workspaces", + "cgo_disable, has_workspaces", ( - pytest.param(False, False, False, id="cgo_disabled__dont_force_tidy"), - pytest.param(True, False, False, id="cgo_enabled__dont_force_tidy"), - pytest.param(False, True, False, id="cgo_disabled__force_tidy"), - pytest.param(True, True, False, id="cgo_enabled__force_tidy"), - pytest.param(False, False, True, id="has_workspaces"), + pytest.param(False, False, id="cgo_disabled"), + pytest.param(True, False, id="cgo_enabled"), + pytest.param(False, True, id="has_workspaces"), ), ) @mock.patch("cachi2.core.package_managers.gomod._go_list_deps") @@ -172,7 +170,6 @@ def test_resolve_gomod( mock_parse_packages: mock.Mock, mock_go_list_deps: mock.Mock, cgo_disable: bool, - force_gomod_tidy: bool, has_workspaces: bool, tmp_path: Path, data_dir: Path, @@ -198,9 +195,6 @@ def test_resolve_gomod( ) ) - if force_gomod_tidy: - run_side_effects.append(proc_mock("go mod tidy", returncode=0, stdout=None)) - run_side_effects.append( proc_mock( "go list -e -m", @@ -252,8 +246,6 @@ def test_resolve_gomod( flags: list[Flag] = [] if cgo_disable: flags.append("cgo-disable") - if force_gomod_tidy: - flags.append("force-gomod-tidy") gomod_request.flags = frozenset(flags) @@ -266,9 +258,6 @@ def test_resolve_gomod( module_dir, gomod_request, tmp_path, mock_version_resolver, go_work ) - if force_gomod_tidy: - assert mock_run.call_args_list[1][0][0] == [GO_CMD_PATH, "mod", "tidy"] - assert mock_run.call_args_list[0][1]["env"]["GOMODCACHE"] == f"{tmp_path}/pkg/mod" # Assert that _parse_packages was called exactly once. @@ -303,7 +292,6 @@ def test_resolve_gomod( ) -@pytest.mark.parametrize("force_gomod_tidy", [False, True]) @mock.patch("cachi2.core.package_managers.gomod._disable_telemetry") @mock.patch("cachi2.core.package_managers.gomod.Go.release", new_callable=mock.PropertyMock) @mock.patch("cachi2.core.package_managers.gomod._get_gomod_version") @@ -319,7 +307,6 @@ def test_resolve_gomod_vendor_dependencies( mock_get_gomod_version: mock.Mock, mock_go_release: mock.PropertyMock, mock_disable_telemetry: mock.Mock, - force_gomod_tidy: bool, tmp_path: Path, data_dir: Path, gomod_request: Request, @@ -333,8 +320,6 @@ def test_resolve_gomod_vendor_dependencies( # Mock the "subprocess.run" calls run_side_effects = [] run_side_effects.append(proc_mock("go mod vendor", returncode=0, stdout=None)) - if force_gomod_tidy: - run_side_effects.append(proc_mock("go mod tidy", returncode=0, stdout=None)) run_side_effects.append( proc_mock( "go list -e -m -json", @@ -365,12 +350,6 @@ def test_resolve_gomod_vendor_dependencies( mock_get_gomod_version.return_value = ("0.1.1", "0.1.2") mock_vendor_changed.return_value = False - flags: list[Flag] = [] - if force_gomod_tidy: - flags.append("force-gomod-tidy") - - gomod_request.flags = frozenset(flags) - module_dir.join_within_root("vendor").path.mkdir(parents=True) module_dir.join_within_root("vendor/modules.txt").path.write_text( get_mocked_data(data_dir, "vendored/modules.txt") @@ -402,7 +381,6 @@ def test_resolve_gomod_vendor_dependencies( assert resolve_result.modules_in_go_sum == expect_result.modules_in_go_sum -@pytest.mark.parametrize("force_gomod_tidy", [False, True]) @mock.patch("cachi2.core.package_managers.gomod._disable_telemetry") @mock.patch("cachi2.core.package_managers.gomod.Go.release", new_callable=mock.PropertyMock) @mock.patch("cachi2.core.package_managers.gomod.Go._install") @@ -418,7 +396,6 @@ def test_resolve_gomod_no_deps( mock_go_install: mock.Mock, mock_go_release: mock.PropertyMock, mock_disable_telemetry: mock.Mock, - force_gomod_tidy: bool, tmp_path: Path, gomod_request: Request, ) -> None: @@ -455,8 +432,6 @@ def test_resolve_gomod_no_deps( # Mock the "subprocess.run" calls run_side_effects = [] run_side_effects.append(proc_mock("go mod download -json", returncode=0, stdout="")) - if force_gomod_tidy: - run_side_effects.append(proc_mock("go mod tidy", returncode=0, stdout=None)) run_side_effects.append( proc_mock( "go list -e -m", @@ -477,9 +452,6 @@ def test_resolve_gomod_no_deps( mock_go_install.return_value = "/usr/bin/go" mock_get_gomod_version.return_value = ("1.21.4", None) - if force_gomod_tidy: - gomod_request.flags = frozenset({"force-gomod-tidy"}) - main_module, modules, packages, _ = _resolve_gomod( module_path, gomod_request, tmp_path, mock_version_resolver, mocked_go_work ) From fc087e314b2c054ca9cc495b8876a2aac049cc9c Mon Sep 17 00:00:00 2001 From: Erik Skultety Date: Fri, 25 Oct 2024 17:01:40 +0200 Subject: [PATCH 5/5] docs: gomod: Document --force-gomod-tidy deprecation Resolves: https://github.com/containerbuildsystem/cachi2/issues/713 Signed-off-by: Erik Skultety --- docs/gomod.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/docs/gomod.md b/docs/gomod.md index 3676a1681..402c40fb9 100644 --- a/docs/gomod.md +++ b/docs/gomod.md @@ -90,12 +90,12 @@ by Cachi2 anyway. The `cachi2 fetch-deps` command accepts the following gomod-related flags: * [--cgo-disable](#--cgo-disable) -* [--force-gomod-tidy](#--force-gomod-tidy) ### Deprecated flags * `--gomod-vendor` (deprecated in _v0.11.0_) * `--gomod-vendor-check` (deprecated in _v0.11.0_) +* `--force-gomod-tidy` (deprecated in _v0.18.0_) All of them are deprecated and will have no effect when set. They are only kept for backwards compatibility reasons and will be removed in future releases. @@ -109,12 +109,6 @@ disable cgo in your build (nor should you disable it yourself if you rely on C). Disabling cgo should not prevent Cachi2 from fetching your Go dependencies as usual. Note that Cachi2 will not make any attempts to fetch missing C libraries. If required, you would need to get them through other means. -### --force-gomod-tidy - -Makes Cachi2 run `go mod tidy` after downloading dependencies. - -⚠ This flag is questionable and may be removed in the future. - ## Vendoring Go supports [vendoring](https://go.dev/ref/mod#vendoring) to store the source code of all dependencies in the vendor/