-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Lispworks feature fix #128
base: master
Are you sure you want to change the base?
Conversation
Is the change backwards-compatible? I.e., if this version of Drakma is pulled as a dependency, are dependent projects going to build and work without needing to add the new features?
November 8, 2022 at 3:07 PM, "Raymond Wiker" ***@***.***> wrote:
…
Simplified Lispworks-specific code:
* Introduced two new features, :lw-simple-char and :lw-use-comm. These indicate whether to use lw:simple-char instead of cl:character, and whether to use the comm package instead of cl+ssl.
* These features (and the pre-existing :lw-does-not-have-write-timeout) are only present during a quicklisp compile, but can be added statically for the current session when working on the code.
* Added ssl verification for Lispworks, when using the comm package.
Tested on macos Ventura on Intel Macbook, with Lispworks 7.1.3 and 8.0.1. Also tested using SBCL 2.2.9.
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
#### You can view, comment on, or merge this pull request online at:
#128
#### Commit Summary
* 7d47963 7d47963 Simplified feature tests for Lispworks.
* 91ad4a8 91ad4a8 Changes for Lispworks 7.
#### File Changes
(3 files https://github.com/edicl/drakma/pull/128/files )
* **M** drakma.asd https://github.com/edicl/drakma/pull/128/files#diff-02dfaddb9a75c5c9d33b4a0de0f3728ce6b7595aa711c4243f9dcee298ef52f8 (25)
* **M** request.lisp https://github.com/edicl/drakma/pull/128/files#diff-be1ad545ce6233743fe6709800cde0c9b361ebc350ef3d3781f422c1a13e9abb (72)
* **M** util.lisp https://github.com/edicl/drakma/pull/128/files#diff-2f8942ca6c9ae363a6bdbb599874703d3d55d048a5af788af1f7327d2016fb8e (4)
#### Patch Links:
* https://github.com/edicl/drakma/pull/128.patch
* https://github.com/edicl/drakma/pull/128.diff
—
Reply to this email directly, view it on GitHub #128 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSZHKXOQFFEISUVKUUMQ43WHJNCDANCNFSM6AAAAAAR2LOVGQ .
You are receiving this because you are subscribed to this thread.
|
I think so - the only problem I can think of is if somebody is using the feature :lw-does-not-have-write-timeout (which I think is unlikely).
Note that the current version (2.0.9, or the master branch) does not work at all for the current version of Lispworks (8.0.1).
The new features are only defined while building drakma under Lispworks; they should not be visible afterwards, and use of drakma should not require those features once drakma itself has been built.
… On 8 Nov 2022, at 15:41, phoe ***@***.***> wrote:
Is the change backwards-compatible? I.e., if this version of Drakma is pulled as a dependency, are dependent projects going to build and work without needing to add the new features?
November 8, 2022 at 3:07 PM, "Raymond Wiker" ***@***.***> wrote:
>
> Simplified Lispworks-specific code:
>
> * Introduced two new features, :lw-simple-char and :lw-use-comm. These indicate whether to use lw:simple-char instead of cl:character, and whether to use the comm package instead of cl+ssl.
>
> * These features (and the pre-existing :lw-does-not-have-write-timeout) are only present during a quicklisp compile, but can be added statically for the current session when working on the code.
>
> * Added ssl verification for Lispworks, when using the comm package.
>
> Tested on macos Ventura on Intel Macbook, with Lispworks 7.1.3 and 8.0.1. Also tested using SBCL 2.2.9.
>
> ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
>
> #### You can view, comment on, or merge this pull request online at:
>
> #128
>
> #### Commit Summary
>
>
>
> * 7d47963 7d47963 Simplified feature tests for Lispworks.
>
>
> * 91ad4a8 91ad4a8 Changes for Lispworks 7.
>
> #### File Changes
>
>
>
> (3 files https://github.com/edicl/drakma/pull/128/files )
>
>
>
> * **M** drakma.asd https://github.com/edicl/drakma/pull/128/files#diff-02dfaddb9a75c5c9d33b4a0de0f3728ce6b7595aa711c4243f9dcee298ef52f8 (25)
>
>
> * **M** request.lisp https://github.com/edicl/drakma/pull/128/files#diff-be1ad545ce6233743fe6709800cde0c9b361ebc350ef3d3781f422c1a13e9abb (72)
>
>
> * **M** util.lisp https://github.com/edicl/drakma/pull/128/files#diff-2f8942ca6c9ae363a6bdbb599874703d3d55d048a5af788af1f7327d2016fb8e (4)
>
> #### Patch Links:
>
>
>
> * https://github.com/edicl/drakma/pull/128.patch
>
>
> * https://github.com/edicl/drakma/pull/128.diff
>
> —
> Reply to this email directly, view it on GitHub #128 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSZHKXOQFFEISUVKUUMQ43WHJNCDANCNFSM6AAAAAAR2LOVGQ .
> You are receiving this because you are subscribed to this thread.
>
—
Reply to this email directly, view it on GitHub <#128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDQOUSWFGGRGS7GID475TWHJRAVANCNFSM6AAAAAAR2LOVGQ>.
You are receiving this because you authored the thread.
|
I ran a quick build check for all quicklisp projects that depend on drakma, on my Intel Macbook Pro, running Lispworks 8.0.1. I used the following code:
(defun try-compile (system)
(handler-bind ((uiop/lisp-build:compile-file-error (lambda (e)
(return-from try-compile (list system e))))
(error (lambda (e)
(return-from try-compile (list system e)))))
(progn
(ql:quickload system)
nil)))
(defun try-compile-dependents (system)
(loop for sys in (ql:who-depends-on system)
for error? = (try-compile sys)
when error?
collect error?))
#||
(length (ql:who-depends-on "drakma"))
(defvar *drakma-dependents-build-errors*
(try-compile-dependents "drakma"))
(loop for (system error) in *drakma-dependents-build-errors*
do (format t "~&~a:~% ~a~%" system error))
||#
The number of dependent systems is 113; the number of systems that fails to build is 36 (may be less, as there are some packages that break builds of other packages in the same lisp session). None of the build failures appear to be caused by (my patched version of) drakma.
I also ran this test using SBCL (2.2.9); the number of failed systems was 9. Most of these are from missing libraries/programs. The last build failure ("maiden-lookup") was fixed by downloading a more recent version of the package “maiden”).
… On 8 Nov 2022, at 16:05, Raymond Wiker ***@***.***> wrote:
I think so - the only problem I can think of is if somebody is using the feature :lw-does-not-have-write-timeout (which I think is unlikely).
Note that the current version (2.0.9, or the master branch) does not work at all for the current version of Lispworks (8.0.1).
The new features are only defined while building drakma under Lispworks; they should not be visible afterwards, and use of drakma should not require those features once drakma itself has been built.
> On 8 Nov 2022, at 15:41, phoe ***@***.***> wrote:
>
>
> Is the change backwards-compatible? I.e., if this version of Drakma is pulled as a dependency, are dependent projects going to build and work without needing to add the new features?
>
> November 8, 2022 at 3:07 PM, "Raymond Wiker" ***@***.***> wrote:
>
>
> >
> > Simplified Lispworks-specific code:
> >
> > * Introduced two new features, :lw-simple-char and :lw-use-comm. These indicate whether to use lw:simple-char instead of cl:character, and whether to use the comm package instead of cl+ssl.
> >
> > * These features (and the pre-existing :lw-does-not-have-write-timeout) are only present during a quicklisp compile, but can be added statically for the current session when working on the code.
> >
> > * Added ssl verification for Lispworks, when using the comm package.
> >
> > Tested on macos Ventura on Intel Macbook, with Lispworks 7.1.3 and 8.0.1. Also tested using SBCL 2.2.9.
> >
> > ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
> >
> > #### You can view, comment on, or merge this pull request online at:
> >
> > #128
> >
> > #### Commit Summary
> >
> >
> >
> > * 7d47963 7d47963 Simplified feature tests for Lispworks.
> >
> >
> > * 91ad4a8 91ad4a8 Changes for Lispworks 7.
> >
> > #### File Changes
> >
> >
> >
> > (3 files https://github.com/edicl/drakma/pull/128/files )
> >
> >
> >
> > * **M** drakma.asd https://github.com/edicl/drakma/pull/128/files#diff-02dfaddb9a75c5c9d33b4a0de0f3728ce6b7595aa711c4243f9dcee298ef52f8 (25)
> >
> >
> > * **M** request.lisp https://github.com/edicl/drakma/pull/128/files#diff-be1ad545ce6233743fe6709800cde0c9b361ebc350ef3d3781f422c1a13e9abb (72)
> >
> >
> > * **M** util.lisp https://github.com/edicl/drakma/pull/128/files#diff-2f8942ca6c9ae363a6bdbb599874703d3d55d048a5af788af1f7327d2016fb8e (4)
> >
> > #### Patch Links:
> >
> >
> >
> > * https://github.com/edicl/drakma/pull/128.patch
> >
> >
> > * https://github.com/edicl/drakma/pull/128.diff
> >
> > —
> > Reply to this email directly, view it on GitHub #128 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSZHKXOQFFEISUVKUUMQ43WHJNCDANCNFSM6AAAAAAR2LOVGQ .
> > You are receiving this because you are subscribed to this thread.
> >
> —
> Reply to this email directly, view it on GitHub <#128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDQOUSWFGGRGS7GID475TWHJRAVANCNFSM6AAAAAAR2LOVGQ>.
> You are receiving this because you authored the thread.
>
|
Argh. Two corrections: the number of dependents of drakma was 118, not 113, and the “maiden-lookup” problem was fixed by downloading a more recent version of “plump”.
… On 11 Nov 2022, at 10:09, Raymond Wiker ***@***.***> wrote:
I ran a quick build check for all quicklisp projects that depend on drakma, on my Intel Macbook Pro, running Lispworks 8.0.1. I used the following code:
(defun try-compile (system)
(handler-bind ((uiop/lisp-build:compile-file-error (lambda (e)
(return-from try-compile (list system e))))
(error (lambda (e)
(return-from try-compile (list system e)))))
(progn
(ql:quickload system)
nil)))
(defun try-compile-dependents (system)
(loop for sys in (ql:who-depends-on system)
for error? = (try-compile sys)
when error?
collect error?))
#||
(length (ql:who-depends-on "drakma"))
(defvar *drakma-dependents-build-errors*
(try-compile-dependents "drakma"))
(loop for (system error) in *drakma-dependents-build-errors*
do (format t "~&~a:~% ~a~%" system error))
||#
The number of dependent systems is 113; the number of systems that fails to build is 36 (may be less, as there are some packages that break builds of other packages in the same lisp session). None of the build failures appear to be caused by (my patched version of) drakma.
I also ran this test using SBCL (2.2.9); the number of failed systems was 9. Most of these are from missing libraries/programs. The last build failure ("maiden-lookup") was fixed by downloading a more recent version of the package “maiden”).
> On 8 Nov 2022, at 16:05, Raymond Wiker ***@***.***> wrote:
>
> I think so - the only problem I can think of is if somebody is using the feature :lw-does-not-have-write-timeout (which I think is unlikely).
>
> Note that the current version (2.0.9, or the master branch) does not work at all for the current version of Lispworks (8.0.1).
>
> The new features are only defined while building drakma under Lispworks; they should not be visible afterwards, and use of drakma should not require those features once drakma itself has been built.
>
>> On 8 Nov 2022, at 15:41, phoe ***@***.***> wrote:
>>
>>
>> Is the change backwards-compatible? I.e., if this version of Drakma is pulled as a dependency, are dependent projects going to build and work without needing to add the new features?
>>
>> November 8, 2022 at 3:07 PM, "Raymond Wiker" ***@***.***> wrote:
>>
>>
>> >
>> > Simplified Lispworks-specific code:
>> >
>> > * Introduced two new features, :lw-simple-char and :lw-use-comm. These indicate whether to use lw:simple-char instead of cl:character, and whether to use the comm package instead of cl+ssl.
>> >
>> > * These features (and the pre-existing :lw-does-not-have-write-timeout) are only present during a quicklisp compile, but can be added statically for the current session when working on the code.
>> >
>> > * Added ssl verification for Lispworks, when using the comm package.
>> >
>> > Tested on macos Ventura on Intel Macbook, with Lispworks 7.1.3 and 8.0.1. Also tested using SBCL 2.2.9.
>> >
>> > ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
>> >
>> > #### You can view, comment on, or merge this pull request online at:
>> >
>> > #128
>> >
>> > #### Commit Summary
>> >
>> >
>> >
>> > * 7d47963 7d47963 Simplified feature tests for Lispworks.
>> >
>> >
>> > * 91ad4a8 91ad4a8 Changes for Lispworks 7.
>> >
>> > #### File Changes
>> >
>> >
>> >
>> > (3 files https://github.com/edicl/drakma/pull/128/files )
>> >
>> >
>> >
>> > * **M** drakma.asd https://github.com/edicl/drakma/pull/128/files#diff-02dfaddb9a75c5c9d33b4a0de0f3728ce6b7595aa711c4243f9dcee298ef52f8 (25)
>> >
>> >
>> > * **M** request.lisp https://github.com/edicl/drakma/pull/128/files#diff-be1ad545ce6233743fe6709800cde0c9b361ebc350ef3d3781f422c1a13e9abb (72)
>> >
>> >
>> > * **M** util.lisp https://github.com/edicl/drakma/pull/128/files#diff-2f8942ca6c9ae363a6bdbb599874703d3d55d048a5af788af1f7327d2016fb8e (4)
>> >
>> > #### Patch Links:
>> >
>> >
>> >
>> > * https://github.com/edicl/drakma/pull/128.patch
>> >
>> >
>> > * https://github.com/edicl/drakma/pull/128.diff
>> >
>> > —
>> > Reply to this email directly, view it on GitHub #128 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ADSZHKXOQFFEISUVKUUMQ43WHJNCDANCNFSM6AAAAAAR2LOVGQ .
>> > You are receiving this because you are subscribed to this thread.
>> >
>> —
>> Reply to this email directly, view it on GitHub <#128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDQOUSWFGGRGS7GID475TWHJRAVANCNFSM6AAAAAAR2LOVGQ>.
>> You are receiving this because you authored the thread.
>>
>
|
I think the changes look good and I tested the rwiker:lispworks-feature-fix branch together with the latest chunga quicklisp version (chunga-20221106-git) on my Apple M1 Max with macOS 12.6.1 and LWM 8.0.1. It worked fine! It would be great to have this in a new Drakma release in Quicklisp! |
Wait a second! My suggested solution is that:
is changed to:
Another solution could have been to run the |
…lable to the defsystem form.
Nice catch - I’ve updated the PR with your suggestion.
It would be nice if asdf:defsystem had an argument “additional-features” for helping out with this kind of situation.
… On 15 Nov 2022, at 23:04, svenemtell ***@***.***> wrote:
Wait a second!
I just noticed that there is a problem in drakma.asd.
The :around-compile function is run when each of the :components is compiled, but not when the dependencies in :depends-on are resolved. Therefore :lw-use-comm is not set in the following code and cl+ssl will always be compiled for LispWorks even though it will not be used.
My suggested solution is that:
#-(or :lw-use-comm (and :allegro (not :allegro-cl-express)) :mocl-ssl :drakma-no-ssl)
:cl+ssl)
is changed to:
#-(or (and lispworks (not (or lispworks4 lispworks5 lispworks6))) (and :allegro (not :allegro-cl-express)) :mocl-ssl :drakma-no-ssl)
:cl+ssl)
Another solution could have been to run the setup-lw-features function before the defsystem and keep :lw-use-comm in :depends-on but that would put the unnecessary litter :lw-use-comm in *features*.
—
Reply to this email directly, view it on GitHub <#128 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAIDQOSYD2KCCKSDQAV4GZTWIQCHNANCNFSM6AAAAAAR2LOVGQ>.
You are receiving this because you authored the thread.
|
Is there anything missing for this PR to make it into a QuickLisp release? |
Simplified Lispworks-specific code:
:lw-simple-char
and:lw-use-comm
. These indicate whether to uselw:simple-char
instead ofcl:character
, and whether to use thecomm
package instead ofcl+ssl
.:lw-does-not-have-write-timeout
) are only present during a quicklisp compile, but can be added statically for the current session when working on the code.comm
package.Tested on macos Ventura on Intel Macbook, with Lispworks 7.1.3 and 8.0.1. Also tested using SBCL 2.2.9.