package-inferred-systems and primary-system-name

[Faré]

Dear Eric, Robert,

>:Eric
> If a have a package-inferred-systems "a" and "a/b/c", the following
> code used to return "a":
>
> (primary-system-name (find-system "a/b/c"))
>
> But after commit 069cd2a6 it returns nil.
>
Thanks for finding that bug. Sorry we didn't have regression tests for that.

> The root cause right now is that system-source-file returns nil for
> the inferred systems. The easiest fix would be to have it return the
> asd file for the primary system. That's probably not *technically*
> correct since the system isn't actually defined in that file, but it's
> probably correct in the principle of least surprise sense.
>
This is probably technically correct enough, since that's the file that,
when it was loaded, implicitly defined the system, and that, if modified,
is likely to cause the system to be redefined.

The "obvious" solution is then that in package-inferred-system.lisp, instead of
:source-file nil
the defsystem form in sysdef-package-inferred-system-search should say
:source-file ,(system-source-file top)

>: Robert
> Looking over the change, I believe the issue is that, unfortunately, the "/"
> character is being used for two different purposes. In the "slashy" systems,
> it is used to identify subsystems, but the use in package-inferred systems
> is subtly different, because the location of their definitions as systems is
> different (indeed the package-inferred systems don't have explicit
> definitions). Also, I don't know that multiple slashes are ("a/b/c") are
> really supported in the case of systems that are defined in a.asd, but I
> haven't checked.
>
> I agree with you, though, that it's reasonable to treat a package-inferred
> system "a/b/c" as having "a" as its primary system name.
>
> I believe that this is the diff that caused the change in that commit:
>
> -      (component (primary-system-name (coerce-name (component-system
> system-designator))))))
> +      (component (let* ((system (component-system system-designator))
> +                        (source-file (physicalize-pathname
> (system-source-file system))))
> +                   (and source-file
> +                        (equal (pathname-type source-file) "asd")
> +                        (pathname-name source-file))))))
>
> But I confess that I don't know the rationale for that change, so I don't
> know what collateral damage there will be to changing it.
>
I recommend against changing this function. It plays an important role
in supporting systems that are not named after the file they are
defined in.

> If you are going to patch this, will you please make a test case? I believe
> it could be easily added to package-inferred-system-test.script. I will be
> happy to help, if you would like.
>
Yes, that's the place where a test case would fit.

> It looks like you could add a separate branch to the etypecase with the
> logic special to package-inferred-system.
>
I think it's better to correctly set the system-source-file.

> If you have access to the cl.net gitlab, then a merge request would be a
> great way to supply a patch, but if that doesn't work for you, sending a git
> patch or just a regular old diff patch would also be fine.
>
Eric, will you send a MR?

—♯ƒ • François-René ÐVB Rideau •Reflection&Cybernethics• http://fare.tunes.org
The trouble with opportunity is that it always comes disguised as hard work.
                — Herbert V. Prochnow