gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
18.11k stars 759 forks source link

Removed or moved files are not tracked by the compiler #3862

Closed sbergen closed 20 hours ago

sbergen commented 6 days ago

How to reproduce:

Create project with the following main module:

import module

pub fn main() {
  module.hello()
}

add src/module.gleam as:

import gleam/io

pub fn hello() {
  io.println("Hello!")
}

Now, from the command line, you get the following unexpected output:

$ gleam run
  Compiling unmovable
   Compiled in 0.26s
    Running unmovable.main
Hello!

$ rm src/module.gleam 

$ gleam run
   Compiled in 0.01s
    Running unmovable.main
Hello!

Doing a gleam clean && gleam run will make things work normally again, giving

error: Unknown module
  ┌─ <omitted>/unmovable.gleam:1:1
  │
1 │ import module
  │ ^^^^^^^^^^^^^

I'm running Gleam version 1.6.1, in Ubuntu 24.04 in WSL 2 on Windows 11.

sbergen commented 6 days ago

I did a bit of poking around at the compiler, and found that if I pass existing_modules to PackageLoader, and then do

for (_, deps) in deps.iter() {
    for dep in deps {
        if !inputs.contains_key(dep) && !self.existing_modules.contains_key(dep) {
            tracing::error!(missing = ?dep, "Module missing:");
        }
    }
}

after this line, I'll get the missing module logged.

sbergen commented 6 days ago

Adding dep to self.stale_modules seems to actually solve the issue.

Also, the looping could also be done over inputs and dependencies() directly, instead of deps. Not sure which would be better.

I've been looking around more to try to see if there would be a way to avoid having to pass existing_modules around, but all other options I've considered have felt more complicated and harder to reason about. But it could be I'm missing a more obvious place to do this.

bentomas commented 6 days ago

I tend to constantly reorganize my code (don't know what to say 🤷‍♂️) and this is constantly happening to me. I have to gleam clean a lot.

sbergen commented 6 days ago

I was thinking about this a bit more, and perhaps storing package metadata (similarly to module metadata) with a list of modules would be a better way to approach fixing the issue? This way we could just check that list against the source files to figure out which modules have been removed, and mark them as stale.

lpil commented 5 days ago

Oh dear! I thought we fixed this. Thanks folks

perhaps storing package metadata (similarly to module metadata) with a list of modules would be a better way to approach fixing the issue?

We already module's dependencies in the metadata and I thought we performed cache invalidation when dependencies were removed, but either it's buggy or I'm misremembering.

sbergen commented 5 days ago

perhaps storing package metadata (similarly to module metadata) with a list of modules would be a better way to approach fixing the issue?

We already module's dependencies in the metadata and I thought we performed cache invalidation when dependencies were removed, but either it's buggy or I'm misremembering.

As far as I could see, we only check that none of the dependencies were updated, and it's done in the correct order based on the topological sort. But I couldn't find any place where removed modules would be added to the list of stale modules.