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
