Skip to content
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.

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

[gomod] Deprecate the '--force-gomod-tidy' CLI flag #714

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 0 additions & 4 deletions cachi2/core/package_managers/gomod.py
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,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:
if go_work_path and go_work_path.join_within_root("vendor").path.is_dir():
# NOTE: the same error will be reported even for 1.21 which doesn't support workspace
Expand All @@ -915,9 +914,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)

go_list = ["list", "-e"]
if not should_vendor:
# Make Go ignore the vendor dir even if there is one
Expand Down
5 changes: 4 additions & 1 deletion cachi2/interface/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 9 additions & 18 deletions docs/gomod.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,15 @@ by Cachi2 anyway.
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)

### Deprecated flags

* `--gomod-vendor` (deprecated in _v0.11.0_)
* `--gomod-vendor-check` (deprecated in _v0.11.0_)
* `--force-gomod-tidy` (deprecated in _v0.13.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.

### --cgo-disable

Expand All @@ -103,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/
Expand All @@ -119,15 +119,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 [metadata](metadata.md) generated for your
Expand Down
35 changes: 4 additions & 31 deletions tests/unit/package_managers/test_gomod.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,11 @@ def _parse_mocked_data(data_dir: Path, file_path: str) -> ResolvedGoModule:


@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._disable_telemetry")
Expand All @@ -153,7 +151,6 @@ def test_resolve_gomod(
mock_get_go_work: mock.Mock,
mock_disable_telemetry: mock.Mock,
cgo_disable: bool,
force_gomod_tidy: bool,
has_workspaces: bool,
tmp_path: Path,
data_dir: Path,
Expand Down Expand Up @@ -183,9 +180,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 -mod readonly -m",
Expand Down Expand Up @@ -218,8 +212,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)

Expand All @@ -243,8 +235,6 @@ def test_resolve_gomod(

if has_workspaces:
assert mock_run.call_args_list[0][0][0] == [GO_CMD_PATH, "work", "edit", "-json"]
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"

Expand Down Expand Up @@ -285,7 +275,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")
Expand All @@ -301,7 +290,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,
Expand All @@ -312,8 +300,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",
Expand Down Expand Up @@ -344,12 +330,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")
Expand Down Expand Up @@ -380,7 +360,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")
Expand All @@ -396,7 +375,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:
Expand Down Expand Up @@ -430,8 +408,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 -mod readonly -m",
Expand All @@ -456,9 +432,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
)
Expand Down
Loading