Skip to content

Comments

Embed mill wrapper script#752

Open
lemony312 wants to merge 3 commits intoscala-steward-org:masterfrom
lemony312:embed-mill-wrapper
Open

Embed mill wrapper script#752
lemony312 wants to merge 3 commits intoscala-steward-org:masterfrom
lemony312:embed-mill-wrapper

Conversation

@lemony312
Copy link

@lemony312 lemony312 commented Jan 6, 2026

Problem

the current version pre emptivly downloads the latest version of mill into the container. If a project uses a different version of mill than the default, it can break mill's incremental compilation and start throwing compiler errors that do not exist in the repo.

Solution

Embed the mill wrapper script directly in the action. The wrapper script handles version detection and bootstrapping automatically and does not preemptively download a mill version.

Changes

  • Download and embed the mill wrapper script into /bin

github-actions bot added a commit that referenced this pull request Jan 6, 2026
@lemony312
Copy link
Author

@exoego sorry for the double pr - I realized the previous pr could be a breaking change for some users of scala-steward and mill. Thank you for your understanding

github-actions bot added a commit that referenced this pull request Jan 6, 2026
@exoego
Copy link
Contributor

exoego commented Jan 7, 2026

@lemony312 Then, please create a revert PR first so it can be reviewed quickly

@lemony312
Copy link
Author

#754.
After the revert is completed - this pr is good to go, I tested it a few scenarios:

  1. repos with an existing mill binary and repos that have no mill code but run the job for many repos

github-actions bot added a commit that referenced this pull request Jan 7, 2026
action.yml Outdated
mill-wrapper-url:
description: |
Url to download the mill wrapper script from.
default: https://raw.githubusercontent.com/com-lihaoyi/mill/refs/heads/main/mill
Copy link
Contributor

@exoego exoego Jan 8, 2026

Choose a reason for hiding this comment

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

I am afraid this may lead to security breach if attacker replaced the script with malicious one.
Such cases are happening in OSS ecosystem in recent years.
Can this download URL include git SHA or something, so that we can ensure a trusted version is used?
If user of this action is eager to use the latest, such adventure is still allowed by setting https://raw.githubusercontent.com/com-lihaoyi/mill/refs/heads/main/mill at their own risk.

Copy link
Author

Choose a reason for hiding this comment

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

that is a good call out - the only reason I opted to download it install directly embedding the mill wrapper is because of my experience with its instability - newer versions would just break the entire script.

I think I will embed the mill wrapper directly - this is commonly used in mill projects, and give an optional parameter to download a specific version just in case.
Does that sound alright?

Copy link
Author

Choose a reason for hiding this comment

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

please have a look at 3ca041f

Copy link
Author

Choose a reason for hiding this comment

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

@exoego please review when you get a chance

github-actions bot added a commit that referenced this pull request Jan 8, 2026
github-actions bot added a commit that referenced this pull request Jan 8, 2026
github-actions bot added a commit that referenced this pull request Jan 8, 2026
github-actions bot added a commit that referenced this pull request Feb 11, 2026
github-actions bot added a commit that referenced this pull request Feb 11, 2026
github-actions bot added a commit that referenced this pull request Feb 11, 2026
github-actions bot added a commit that referenced this pull request Feb 11, 2026
github-actions bot added a commit that referenced this pull request Feb 24, 2026
github-actions bot added a commit that referenced this pull request Feb 24, 2026
- Add embedded mill binary and mill-wrapper-url input
- Install mill in pre step, always add to PATH
- Use bundled mill by default, download from URL when provided
github-actions bot added a commit that referenced this pull request Feb 24, 2026
- Add mill-version input back as deprecated (no breaking changes)
- Remove mill from dist folder, keep only at repo root
- Add dist/mill to gitignore, rm from build output
- Update getBundledMillPath for correct resolution from dist/
github-actions bot added a commit that referenced this pull request Feb 24, 2026
- Remove dist from git tracking
- CI builds dist at runtime; release workflow adds it to tag commits
github-actions bot added a commit that referenced this pull request Feb 24, 2026
@github-actions
Copy link
Contributor

A snapshot release has been created as snapshots/752.

You can test it out with:

uses: scala-steward-org/scala-steward-action@snapshots/752

It will be automatically recreated on any change to this PR.

@github-actions
Copy link
Contributor

Code Coverage

Package Line Rate Branch Rate Complexity Health
core 100% 100% 0
modules 73% 88% 0
Summary 74% (529 / 712) 89% (73 / 82) 0

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.

2 participants