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.


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,

Reply via email to