[OpenSmalltalk/opensmalltalk-vm] Fix include order (#562)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[OpenSmalltalk/opensmalltalk-vm] Fix include order (#562)

David T Lewis
 

This enables the linux builds again, while preserving windows and macos builds.
That is a fix for #560

If this is compatible with TERF VM, then it should be merged ASAP, the builds are red for too long.
(we'll have to inquire next point of failure...)


You can view, comment on, or merge this pull request online at:

  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562

Commit Summary

  • always include <stdio.h> **BEFORE** defining fseeko/ftello
  • Discard the EXPORT from sqAssert.h for declaring VM functions
  • Make sure that config.h is included first
  • Generate source with VMMaker.oscog-nice.2957 so as to restore sqConfig.h inclusion at top
  • try and omit HAVE_CONFIG_H for compiling getVersion

File Changes

Patch Links:


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix include order (#562)

David T Lewis
 

@fniephaus approved this pull request.

LGTM, don't know about TERF. Thanks, @nicolas-cellier-aka-nice!


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#pullrequestreview-645999910", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#pullrequestreview-645999910", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix include order (#562)

David T Lewis
In reply to this post by David T Lewis
 

@marceltaeumel approved this pull request.

Maybe merge the current upstream again. Please make the Linux builds work again. Thanks Nicolas!


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#pullrequestreview-663499978", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#pullrequestreview-663499978", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>
Reply | Threaded
Open this post in threaded view
|

Eliot, need your eyes on this issue please (Fix include order (#562))

David T. Lewis
In reply to this post by David T Lewis
 
On Mon, Apr 26, 2021 at 11:54:16AM -0700, Nicolas Cellier wrote:

>  
> This enables the linux builds again, while preserving windows and macos builds.
> That is a fix for https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/560
>
> If this is compatible with TERF VM, then it should be merged ASAP, the builds are red for too long.
> (we&#39;ll have to inquire next point of failure...)
>
> You can view, comment on, or merge this pull request online at:
>
>   https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562
>

Builds for the opensmalltalk-vm for Unix/Linux have been broken since
February 8, 2001. The commit that caused the initial breakage is:

    commit aafcb78371c7e576073a8dbf2f1f32667568e05e
    Author: Eliot Miranda <[hidden email]>
    Date:   Mon Feb 8 20:49:07 2021 -0800

All subsequent versions remain broken, and it has not been possible to
build a VM for Linux since that time.

The commit notice includes the following request:

    I ask anyone wishing to alter the system here to please contact me and discuss
    before making changes.  It is extraordinarily expensive and frustrating to have
    to make changes of this kind to make sure that a signfiicantly more complex VM
    such as Terf's builds correctly.

In April 2021, Nicolas worked through the issues necessary to resolve
this problem.  He provided platforms and src fixes on GitHub in the
fix_include_order branch, along with VMMaker.oscog-nice.2957 in the
VMMaker inbox to provide the necessary code generation fix.

Technically, what we need next is:
  1) Merge Nicolas' VMMaker fix into VMMaker.oscog, regenerate sources, commit to GitHub
  2) Merge the fix_include_order branch into Cog

But we would also like to honor Eliot's request to review such changes
before committing.

Eliot, could you please take a look at this and comment?

References:
  http://source.squeak.org/VMMakerInbox/VMMaker.oscog-nice.2957.mcz
  https://github.com/OpenSmalltalk/opensmalltalk-vm/tree/fix_include_order
  https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562
  https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/560

Thanks,
Dave

Reply | Threaded
Open this post in threaded view
|

Re: [OpenSmalltalk/opensmalltalk-vm] Fix include order (#562)

David T Lewis
In reply to this post by David T Lewis
 

This issue apparently affects (some?) Windows builds in addition to unix, see issue #568


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#issuecomment-860247919", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/562#issuecomment-860247919", "name": "View Pull Request" }, "description": "View this Pull Request on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>