Skip to content

Conversation

@melsman
Copy link
Owner

@melsman melsman commented Jan 12, 2026

Sometimes MLKit recompiles a source file f even when f has not changed and none of its dependencies have changed. The reason is that by mistake the lnk-file (representing the target code objects for f) was not written if the content would not change.

Copilot AI review requested due to automatic review settings January 12, 2026 23:31
@melsman melsman self-assigned this Jan 12, 2026
@melsman melsman added the bug label Jan 12, 2026
Copy link

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 pull request fixes an issue where MLKit unnecessarily recompiled source files even when neither the file nor its dependencies had changed. The root cause was that lnk-files (which represent target code objects) were not being written when their content remained unchanged, causing the build system to lose track of whether recompilation had occurred.

Changes:

  • Modified pickleLnkFile function to always write lnk-files regardless of content changes, serving as a witness that compilation occurred
  • Added test case for pretty-printing record lists with different field orderings
  • Updated NEWS.md with the fix description

Reviewed changes

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

File Description
src/Manager/Manager.sml Removed conditional check to always write lnk-files, ensuring file timestamps reflect compilation
test/repl/pretty.cmd Added test case for pretty-printing a list of records with reordered fields
test/repl/pretty.out.ok Added expected output for the new pretty-printing test case
NEWS.md Added changelog entry documenting the recompilation fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@melsman melsman merged commit 502979b into master Jan 13, 2026
10 checks passed
mikkelwillen pushed a commit to mikkelwillen/mlkit that referenced this pull request Jan 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants