Skip to content
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

chore: un-vendor TCL/Tk dependency #28

Closed
thoughtpolice opened this issue Feb 6, 2020 · 4 comments
Closed

chore: un-vendor TCL/Tk dependency #28

thoughtpolice opened this issue Feb 6, 2020 · 4 comments

Comments

@thoughtpolice
Copy link
Contributor

While we work to integrate the Bluespec compiler into Nixpkgs, I'd like to see the Bluespec build move away from vendoring dependencies, and I think TCL/Tk is the best starting point for this kind of work. TCL/Tk are widely distributed and easily obtainable on almost any modern machine, so I think just requiring it to be installed is pretty reasonable. This will both improve build times and generally make the overall compile process simpler.

It would appear that there is a binding for bluetcl that binds the Haskell runtime to Tcl (which is impressive and very neat!) I'd suggest leaving that here, and just moving out the actual tcl build itself, of course.

Bluespec currently uses TCL 8.5, but the latest version is 8.6. My understanding is that many distros support both of these versions of Tcl, and so does Nixpkgs -- so supporting new tcl versions and de-coupling the current vendored build can both be done independently. (itcl supports 8.6, so it might only be the Haskell bindings that need updating to support 8.5/8.6 concurrently.)

I believe this will make the build system simpler, and also allow us to more easily create "non-GUI" builds of the Bluespec compiler later on that do not rely on X11, etc being available. (These are useful to make builds faster e.g. for CI systems, where you don't want to run the "install bluespec compiler" phase and pull in a bunch of GUI dependencies for mesa, etc in.)

@bpfoley
Copy link
Collaborator

bpfoley commented Feb 7, 2020

It's a bit worse than that. bsc currently vendors 8.5.4 which is from 2008. The latest version of 8.5 is 8.5.19 from 2016.

From talking to a former bsc developer, I believe the vendored Tcl/Tk has patches to the core Tcl code to allow the Bluesim/Haskell integrationrather than doing it with a Tcl extension. We need to verify if this is the case, but if it is, we need to resolve this before we can use the/a system Tcl.

@thoughtpolice
Copy link
Contributor Author

There seems to be two main differences between a stock version of tcl 8.5.4 and what is shipped in this repository:

diff --no-dereference -u -r ./upstream/tcl8.5.4/generic/tclIO.c ./tcltk8.5.4/tcl8.5.4/generic/tclIO.c
--- ./upstream/tcl8.5.4/generic/tclIO.c 2008-05-23 16:10:43.000000000 -0500
+++ ./tcltk8.5.4/tcl8.5.4/generic/tclIO.c       2020-02-06 13:52:29.621490343 -0600
@@ -6232,6 +6232,7 @@
        SetFlag(statePtr, CHANNEL_EOF);
        statePtr->inputEncodingFlags |= TCL_ENCODING_END;
     } else if (nread < 0) {
+        if (result == 512) result = EAGAIN; /* work around Linux kernel bug */
        if ((result == EWOULDBLOCK) || (result == EAGAIN)) {
            SetFlag(statePtr, CHANNEL_BLOCKED);
            result = EAGAIN;

and

diff --no-dereference -u -r ./upstream/tcl8.5.4/unix/Makefile.in ./tcltk8.5.4/tcl8.5.4/unix/Makefile.in
--- ./upstream/tcl8.5.4/unix/Makefile.in        2008-08-14 12:31:38.000000000 -0500
+++ ./tcltk8.5.4/tcl8.5.4/unix/Makefile.in      2020-02-06 13:52:29.682491436 -0600
@@ -6,6 +6,12 @@
 #
 # RCS: @(#) $Id: Makefile.in,v 1.229.2.8 2008/08/13 23:07:16 das Exp $

+# Bluespec added:
+# Use vfork instead of fork, as this is much more efficient especially
+# for large memory images in bluewish.
+# fork/vfork is used in the exec tclcommand, to launch other processes.
+EXTRA_CFLAGS += -DUSE_VFORK
+
 VERSION                = @TCL_VERSION@
 MAJOR_VERSION          = @TCL_MAJOR_VERSION@
 MINOR_VERSION          = @TCL_MINOR_VERSION@

The "linux kernel bug" is entirely unclear, and the usage of vfork(2) is more obvious but still somewhat unexplained by what parameters it can be triggered under, as the Linux memory manager probably changed a lot since this was initially patched.

At the very least, this is hopefully an indication that using the system tcl isn't impossible/extremely difficult, but some more investigation/testing would be needed.

@bpfoley
Copy link
Collaborator

bpfoley commented Feb 21, 2020

I'm working on this and have gotten to the point where I can use the system version of tcl/tk 8.6 itcl/itk 3.4 on Ubuntu. I've some polishing up to do, but hopefully it shouldn't take too much longer.

@bpfoley
Copy link
Collaborator

bpfoley commented Feb 28, 2020

Done by PR #78. This will need to be tested with OSes other than Ubuntu 16.04/18.04

@quark17 quark17 closed this as completed Feb 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants