package-inferred-systems and primary-system-name

[Robert Goldman]

On 28 Feb 2018, at 15:46, Eric Timmons wrote:

> 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.
>
> Happy to patch it, but I wanted to check how to do it before starting.
> 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.
>
> Thoughts?

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.

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.

It looks like you could add a separate branch to the `etypecase` with 
the logic special to `package-inferred-system`.

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.

Thanks for spotting this,
best,
r

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.common-lisp.net/pipermail/asdf-devel/attachments/20180228/cae92cc1/attachment.html>