unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.81k stars 271 forks source link

`delete.namespace` reportedly flags dependents in `lib` (why?) #5446

Closed aryairani closed 1 week ago

aryairani commented 2 weeks ago

Describe and demonstrate the bug

I don't have a reproduction; here's an example that doesn't repro:

project.create
fork lib.base up.base
delete.namespace up.base

First step is to reproduce and explain the issue better

Screenshots

Environment (please complete the following information):

Additional context

ceedubs commented 2 weeks ago

Transcript that reproduces the issue:

```ucm
fresh/main> builtins.merge
foo : Text
foo = "I am going to be deleted"

lib.mylibrary.bar : Text
lib.mylibrary.bar = foo ++ foo
fresh/main> add
fresh/main> delete.term foo
ceedubs commented 2 weeks ago

The way that this comes up in practice is that a library has a reference to an unnamed definition (probably due to footguns in ucm, a lot of which have been resolved) and you have a name for that definition. The name might be in your project directly, or it might be in an old version of base that you still depend on (perhaps transitively).

aryairani commented 2 weeks ago

Ok, so then it sounds like the logic is working as intended after all, but we need a workaround for the possibly-outdated bug of creating unnamed definitions?

Then is the thinking that namespace.dependents doesn't include lib, so that gives the clean report after delete.namespace.force? We should probably have a namespace.dependents.all or something then to help figure out what's going on.

ceedubs commented 2 weeks ago

possibly-outdated bug of creating unnamed definitions?

There are definitely still ways to end up in this situation, and fairly easily. #5044 and #5381 are two that quickly come to mind.

FWIW I don't think that it ever really makes sense to force people to deal with this. If the library has an unnamed dependency then that's an issue for the library, not for the person using it.