[OpenSmalltalk/opensmalltalk-vm] Minheadless SqueakSSL plugin compilation bug fix (#419)
I fixed a mistake in the compilation of the SqueakSSL plugin in the minheadless VM for Mac. You can view, comment on, or merge this pull request online at:
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/419
-- Commit Summary --
* I fixed a mistake in the compilation of the SqueakSSL plugin in the minheadless VM for Mac.
-- File Changes --
M cmake/PluginsCommon.cmake (2)
-- Patch Links --
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/419.patch https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/419.diff
Please explain.
On the minheadless VM that is built using cmake, the add_vm_plugin_sources cmake macro requires specifying the the plugin sources explicitly, which is omitting the src/plugins/SqueakSSL/SqueakSSL.c in the compilation of the plugin. By using the other macro (add_vm_plugin_auto), the platform specific files are automatically found with a glob pattern. This is a mistake that I introduced myself. This problem can be reproduced in Pharo using the minheadless vm of this repository on OS X with the following script:
´´´smalltalk url := 'https://google.com' asZnUrl. ZnClient new url: url; get; response ´´´
CI errors on travis are not related to this PR.
Ok, but then I don't understand why Mac needs the explicit `${SqueakSSL_Sources}`. Apparently, Windows and Unix don't, but also have such specific sources.
In the case of the minheadless VM for OS X, we are treating the VM as it were an unix since we are removing all of the platform specific windowing code in this VM variant. For this reason, in the cases where OS X is different than another unix, the plugin code has to be added manually.
In the case of the minheadless VM for OS X, we are treating the VM as it were an unix since we are removing all of the platform specific windowing code in this VM variant. For this reason, in the cases where OS X is different than another unix, the plugin code has to be added manually.
Side note: I would've liked to see the explanation to be part of the commit message. It'll be rather hard to find this discussion if we need to in the future. But it's not too late: we can still add the info to the merge commit.
On 2019-08-22, at 5:07 AM, Fabio Niephaus notifications@github.com wrote:
Side note: I would've liked to see the explanation to be part of the commit message. It'll be rather hard to find this discussion if we need to in the future. But it's not too late: we can still add the info to the merge commit.
Generically we should include actually meaningful comments and explanations to every change, vm or image. But we don't. This makes me sad.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Runs squares around the competition.
@ronsaldo pushed 1 commit.
23c3d109be15eacd53d11b04c071267d2b1529a5 On the minheadless VM that is built using cmake, the add_vm_plugin_sources cmake macro requires specifying the the plugin sources explicitly, which is omitting the src/plugins/SqueakSSL/SqueakSSL.c in the compilation of the plugin. By using the other macro (add_vm_plugin_auto), the platform specific files are automatically found with a glob pattern. This is a mistake that I introduced myself. This problem can be reproduced in Pharo using the minheadless vm of this repository on OS X with the following script:
Merged #419 into Cog.
vm-dev@lists.squeakfoundation.org
-
Fabio Niephaus -
Nicolas Cellier
-
Ronie Salgado
-
tim Rowledge -
Tobias Pape