Skip to content

Conversation

@WVerlaek
Copy link
Member

Reopening #153 to fix script dependencies not being cached

Description

Related Issue(s)

Fixes #

How to test

Documentation

/hold

pkg/build.Build may be called with a BuildOptions that contains a
pre-generated BuildContext; in this case the BuildOptions value is
largely ignored during context setup and may not be complete. The
buildContext struct will be authoritative in this context and must be
be used instead of buildOptions.

One way that this issue manifests is when running a leeway script with
dependencies as `leeway run` generates a buildContext and passes that
into leeway.Build; the rest of the buildOptions is empty. When using
buildOptions the RemoteCache field will always be empty, even if the
buildContext value has a RemoteCache defined. By only using buildContext
instead of buildOptions we resolve this confusion on what data is authoritative.
@WVerlaek WVerlaek requested a review from a team August 30, 2023 13:18
@codecov
Copy link

codecov bot commented Aug 30, 2023

Codecov Report

Merging #162 (e4d8ef9) into main (8e1719d) will not change coverage.
The diff coverage is 0.00%.

@@          Coverage Diff          @@
##            main    #162   +/-   ##
=====================================
  Coverage   8.36%   8.36%           
=====================================
  Files         20      20           
  Lines       3406    3406           
=====================================
  Hits         285     285           
  Misses      3069    3069           
  Partials      52      52           

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

I don't have as much context, but change looks sensible. We can revert if this goes wrong.

Copy link
Collaborator

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

LGTM

@aledbf aledbf merged commit e591a13 into main Aug 30, 2023
@WVerlaek WVerlaek deleted the alt/leeway-run-cache branch August 30, 2023 17:49
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.

5 participants