Skip to content

Conversation

@willie-yao
Copy link
Contributor

What this PR does / why we need it:
Continuation of #847

Which issue(s) this PR fixes (optional, using fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when the PR gets merged):
Fixes #

Special notes for your reviewer:

@willie-yao willie-yao force-pushed the go-replace branch 4 times, most recently from 8f760ff to 7b08746 Compare December 4, 2025 21:31
// This must happen after build deps are installed so Go is available
if err := spec.Preprocess(client, sOpt, worker, opts...); err != nil {
return llb.Scratch()
return dalec.ErrorState(worker, err)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change exposes the error of the state to the user as this is in an error check

@willie-yao willie-yao marked this pull request as ready for review December 8, 2025 18:55
Copilot AI review requested due to automatic review settings December 8, 2025 18:55
@willie-yao willie-yao marked this pull request as draft December 8, 2025 18:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds support for go mod require and replace directives in the gomod generator, allowing users to programmatically modify go.mod files during the build process. This is a continuation of #847 and enables important use cases like replacing dependencies with forks, working with multi-module repositories, and pinning specific dependency versions.

Key Changes:

  • New GomodEdits, GomodReplace, and GomodRequire types for declarative go.mod manipulation
  • Preprocessing logic that generates patches from gomod edits before building
  • Internal SourceLLB type for programmatically generated patch sources
  • Validation and extensive test coverage for the new functionality

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
website/docs/sources.md Documents the new gomod replace/require directives (contains format errors that need correction)
spec.go Defines GomodEdits, GomodReplace, and GomodRequire types with marshaling/unmarshaling logic
preprocess.go Implements preprocessing logic to generate patches from gomod edits
source_llb.go Introduces internal-only SourceLLB type for programmatically generated sources
source_generated.go Generated code for SourceLLB integration
source.go Integrates LLB field into Source type
load.go Adds validation for gomod edits
generator_gomod_test.go Unit tests for unmarshaling and validation of gomod directives
spec_test.go Validation tests for SourceGenerator gomod edits
test/source_test.go Unit tests for gomod require/replace with debug/patched-sources target
test/linux_target_test.go Integration tests for gomod edits across different scenarios
test/fixtures/simple-go-require.yml Example fixture demonstrating correct usage
targets/windows/handle_zip.go Adds Preprocess call before building
targets/linux/rpm/distro/pkg.go Adds Preprocess call before building
targets/linux/deb/distro/pkg.go Adds Preprocess call before building
targets/linux/distro_handler.go Standard import reordering
frontend/gateway.go Standard import reordering
frontend/debug/handle_sources.go Adds Preprocess call before getting sources
docs/spec.schema.json Updates JSON schema with new gomod edit types
docker-bake.hcl Removes trailing blank line

@willie-yao willie-yao force-pushed the go-replace branch 2 times, most recently from a7c7424 to c561d30 Compare December 10, 2025 17:55
spec.go Outdated
Replace []GomodReplace `yaml:"replace,omitempty" json:"replace,omitempty"`
// Require applies go.mod require directives before downloading module dependencies.
// Each entry can be either a string "module:version" or a struct with Module and Version fields.
Require []GomodRequire `yaml:"require,omitempty" json:"require,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this injects totally new requires?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so. Is that a concern or should we be doing this a different way? @DannyBrito can maybe also clarify since he wrote the original struct.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess the main thing is, do we have a use-case for this right now?

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry I missed this but tbh I don't really understand the question here; I reused initial implementation of the GomodEdits https://github.com/project-dalec/dalec/pull/769/changes#diff-f23a639898f3d395e15d7331eeab9d8d6217fe161c2851610171da61d9f92dd5R255.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Basically, do we need to support adding new requires in this feature.

Copy link
Member

Choose a reason for hiding this comment

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

we don't need to inject new requires since they'll get cleaned up by go mod tidy

we should only use require to update existing requires. require: old:new, not require: new

@willie-yao willie-yao changed the title wip: add go mod require & replace support add go mod require & replace support Jan 5, 2026
@willie-yao willie-yao marked this pull request as ready for review January 5, 2026 19:12
@cpuguy83
Copy link
Collaborator

cpuguy83 commented Jan 5, 2026

@willie-yao This one needs a rebase to get CI fixes and I think one merge conflict?

Signed-off-by: Danny Brito <54994380+DannyBrito@users.noreply.github.com>
Signed-off-by: Danny Brito <54994380+DannyBrito@users.noreply.github.com>
Signed-off-by: Danny Brito <54994380+DannyBrito@users.noreply.github.com>
Signed-off-by: William Yao <william2000yao@gmail.com>

use go 1.18 in go.mod

Signed-off-by: William Yao <william2000yao@gmail.com>

Fix Bionic go path

Signed-off-by: William Yao <william2000yao@gmail.com>

Reviews

Signed-off-by: William Yao <william2000yao@gmail.com>

set source maps

Signed-off-by: William Yao <william2000yao@gmail.com>
@willie-yao
Copy link
Contributor Author

@cpuguy83 The test failures seem to be still related to runners not having enough memory. Otherwise CI looks good?

Signed-off-by: William Yao <williamyao@microsoft.com>
Copy link
Collaborator

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

For the string format, I think we should either drop it or do the exact format that go.mod uses ( old/mod/path => new/mod/path version)

Signed-off-by: William Yao <williamyao@microsoft.com>
@willie-yao willie-yao changed the title add go mod require & replace support add go mod require support Jan 7, 2026
@cpuguy83 cpuguy83 merged commit 71d79bf into project-dalec:main Jan 7, 2026
44 of 45 checks passed
@willie-yao willie-yao changed the title add go mod require support add go mod replace support Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants