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

Minor bugs in configure steps regarding flexdll bootstrap #7923

Open
vicuna opened this issue Feb 19, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@vicuna
Copy link

commented Feb 19, 2019

Original bug ID: 7923
Reporter: @mlasson
Assigned to: @dra27
Status: assigned (set by @dra27 on 2019-02-19T18:59:12Z)
Resolution: open
Priority: normal
Severity: minor
Category: configure and build/install
Monitored by: @nojb @gasche

Bug description

Hello,

On current trunk (4.08), the configure step is buggy for those who want to bootstrap flexdll.

There are two issues:

  1. The "IFLEXDIR" environment variable is set at configure steps using the the output of "flexlink" found in the path and the user should manually set it to "-I$(ROOT_DIR)/flexdll" in Makefile.config.

  2. The definition of "FLEXDLL_DIR" in the root Makefile should be moved to utils/Makefile or it won't be used (the build will run normally but the resulting compiler will complain that it cannot locate flexdll libraries).

Marc.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 19, 2019

Comment author: @mlasson

Also I made small POC fix for this issue, alas I don't have the time to test it right now
so I didn't want to make a pullrequest, but it is available at:

mlasson@e03c5ba

It adds a --enable-flexdll-bootstrap option to configure to set the value of IFLEXDIR.

Also it moves the variable FLEXDLL_DIR in the root Makefile into utils/Makefile.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 19, 2019

Comment author: @dra27

Odd, I thought I'd tested that while we were working on the original PR, although perhaps it was working when flexlink wasn't in PATH. Regardless, thanks for the poc - I'll have a look at this tomorrow.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 20, 2019

Comment author: @mlasson

Note that the change related to "runstate" in the configure file in my POC does not seem to be related to my change in the configure.ac (may be my cygwin version of autoconf ?).

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 20, 2019

Comment author: @dra27

Indeed - the autoconf maintainers have failed to release a new version, despite that patch being accepted many years ago, I think, and Cygwin chooses not to have it. It's very tedious, as one has to revert those changes when developing with Cygwin's autoconf before committing.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 20, 2019

Comment author: @dra27

Regarding your first point, I believe that's "intended" behaviour inasmuch as that's also how the old manual Makefiles worked - if you have flexlink in PATH and you wish to bootstrap, you had to override IFLEXDIR. The second point I haven't looked into yet.

I'm working on a full fix, including extending the flexdll bootstrapping support to the Cygwin ports of OCaml.

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

Comment author: @mlasson

Personally, I think the bootstrap is a neat way to build on windows (it allows you skip the step of finding the right flexdll binaries, find a place to install them, add them in the path).

I like building the msvc port by following the steps in README.win32, it is not far from being followable by someone who is not an expert. But it is still a bit rough around the edges.

I think not requiring the user to dig into the makefiles is a big plus (or we should at least document that the value for IFLEXDIR should be "-I$(ROOTDIR)/flexdll" to do the bootstrap).

Another example of roughness is the "prefix" that should be express using forward slashes (does it work if we escape the back slashes ? I've never tried). Maybe the configure script could take care of this normalization ?

@vicuna

This comment has been minimized.

Copy link
Author

commented Feb 21, 2019

Comment author: @dra27

I agree, it's why I wrote it :o) I entirely agree about the rough edges - the first stage was to get the configure script able to do anything for Windows at all.

The second part is most certainly a bug which was introduced by the addition of the Dune files - see #2259

@nojb nojb added the windows label Mar 15, 2019

@vicuna vicuna added the bug label Mar 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.