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

Fails to install gem on mswin64 #3273

Closed
mame opened this issue Dec 3, 2024 · 10 comments · Fixed by #3289
Closed

Fails to install gem on mswin64 #3273

mame opened this issue Dec 3, 2024 · 10 comments · Fixed by #3289

Comments

@mame
Copy link
Member

mame commented Dec 3, 2024

gem install prism for mswin64 Ruby fails due to No such file or directory - make.

(Calling make during extconf.rb seems like an extremely bad idea to me.)

C:\Users\YusukeEndoh>.\ruby\local\bin\ruby -v
ruby 3.4.0dev (2024-12-03T06:49:57Z master f6b62d001a) +PRISM [x64-mswin64_140]

C:\Users\YusukeEndoh>.\ruby\local\bin\gem install prism
Building native extensions. This could take a while...
ERROR:  Error installing prism:
        ERROR: Failed to build gem native extension.

    current directory: C:/Users/YusukeEndoh/scoop/persist/ruby/gems/gems/prism-1.2.0/ext/prism
C:/Users/YusukeEndoh/ruby/local/bin/ruby.exe extconf.rb
checking for prism.h in C:/Users/YusukeEndoh/scoop/persist/ruby/gems/gems/prism-1.2.0/include... yes
checking for prism/extension.h in C:/Users/YusukeEndoh/scoop/persist/ruby/gems/gems/prism-1.2.0/ext... yes
checking for whether -fvisibility=hidden is accepted as CFLAGS... no
*** extconf.rb failed ***
Could not create Makefile due to some reason, probably lack of necessary
libraries and/or headers.  Check the mkmf.log file for more details.  You may
need configuration options.

Provided configuration options:
        --with-opt-dir
        --with-opt-include=${opt-dir}/include
        --without-opt-include
        --with-opt-lib=${opt-dir}/lib
        --without-opt-lib
        --with-make-prog
        --srcdir=.
        --curdir
        --ruby=C:/Users/YusukeEndoh/ruby/local/bin/ruby
        --enable-build-debug
        --disable-build-debug
        --enable-build-minimal
        --disable-build-minimal
extconf.rb:51:in 'Kernel#system': No such file or directory - make (Errno::ENOENT)
        from extconf.rb:51:in 'block in Object#make'
        from extconf.rb:50:in 'Dir.chdir'
        from extconf.rb:50:in 'Object#make'
        from extconf.rb:136:in '<main>'
Running make build/libprism.a with {"SOEXT" => "dll", "MAKEDIRS" => "$(COMSPEC) /E:ON /C $(top_srcdir:/=\\)\\win32\\makedirs.bat", "RMALL" => "$(COMSPEC) /C $(top_srcdir:/=\\)\\win32\\rm.bat -f -r"}

To see why this extension failed to compile, please check the mkmf.log which can be found here:

  C:/Users/YusukeEndoh/scoop/persist/ruby/gems/extensions/x64-mswin64-140/3.4.0+1/prism-1.2.0/mkmf.log

extconf failed, exit code 1

Gem files will remain installed in C:/Users/YusukeEndoh/scoop/persist/ruby/gems/gems/prism-1.2.0 for inspection.
Results logged to C:/Users/YusukeEndoh/scoop/persist/ruby/gems/extensions/x64-mswin64-140/3.4.0+1/prism-1.2.0/gem_make.out
@eregon
Copy link
Member

eregon commented Dec 3, 2024

Do you have make installed? (if so, how would it get added to PATH?)
Or only nmake?
https://github.com/ruby/prism/blob/main/Makefile needs make/gmake, at least in the current state. It contains the logic to build libprism.

Calling make during extconf.rb seems like an extremely bad idea to me.

I think it is common and fine, here it is used to build libprism.so/dylib/dll, I think sqlite3 and nokogiri and plenty of other gems do the same when they need to build a regular dynamic library not linking to any Ruby API, and then an extension library linking to that library. Notably it enables having other cflags than what CRuby uses.
Note that we cannot just "flatten" all files for libprism in the extension easily, because the FFI backend and other cases needs a libprism.so/dylib/dll.

@tenderlove
Copy link
Member

@eregon do we really need to call make in the CRuby case though? It feels like extconf.rb could just compile the C files on its own without building the .a file first. I don't understand why this step is necessary (on CRuby anyway).

@kddnewton
Copy link
Collaborator

@flavorjones would probably have a little more context on this since he helped put this together. But it's my understanding that extconf's responsibility (in the current setup) is to compile the extension, whereas we rely on make to compile the static library. I think we could change it, truthfully I'm not sure of the implications.

@tenderlove
Copy link
Member

AFAICT, we would have to copy the Prism source in to the ext folder when we compile the extension. But I don't think that's a problem.

@eregon
Copy link
Member

eregon commented Dec 4, 2024

That's what I meant by "flatten" above. One concern is we would stop being in control of the cflags, which I think on its own is a non-trivial concern, it seems best to always compile prism non-extension C files with a consistent set of flags and a unique way to describe how to compile them (the Makefile at the root).
For example I think different flags like optimization levels or some -f flags could break Prism.
Different warnings flags could miss important warnings or cause compilation to fail if -Werror is added somehow.
Right now, no matter how Prism is used (C extension, FFI, in JRuby, in TruffleRuby, etc), it's always compiled with the same flags, I think this is valuable.
(there is one exception, the copy of Prism in CRuby doesn't use Prism's Makefile, this has already caused problems such as incorrect incremental compilation due to missing dependencies and added complexity)

Note the libprism.a is also used by TruffleRuby, so we would not be able to remove that logic but we'd have yet another way to build Prism, which seems bad complexity-wise.

But I don't think that's a problem.

One consequence I can think of is in development one would see 2 copies of each .c file in their editor/IDE. That's a potential to waste some time editing the wrong file.

@mame Could you confirm on your system you only have nmake, no make or gmake?
If that's the case, indeed it's annoying and it might be worth the unfortunate workaround of copying the prism C files under ext/ in extconf.rb.
Or maybe maintaining a Makefile.win32 or so in the prism gem for nmake.
OTOH, I suspect other gems require make/gmake, so one might consider an environment without make to be incomplete to install gems with C extensions (in general).

@tenderlove
Copy link
Member

For example I think different flags like optimization levels or some -f flags could break Prism.

This seems bad doesn't it? It feels like Prism should work with any optimization level or -f flag. If it doesn't, it seems like we should fix it.

Different warnings flags could miss important warnings or cause compilation to fail if -Werror is added somehow.

We should maintain a Makefile in Prism and test compilation with these different flags.

Note the libprism.a is also used by TruffleRuby, so we would not be able to remove that logic but we'd have yet another way to build Prism, which seems bad complexity-wise.

It seems like the Prism gem is treating the Prism C library (libprism.a) as if it's a "package that hasn't been installed yet", and is taking on the responsibility of compiling / installing that package on the user's system. In other words, we're taking on the complexity of knowing how to correctly install a C library on all systems. This seems fraught to me (and is a problem that mkmf is designed to solve).

Is TruffleRuby using the .a file that gets built along side this gem? Presumably you can install TruffleRuby without the Prism gem?

@mame
Copy link
Member Author

mame commented Dec 4, 2024

I don't have make or gmake because it is a pure Windows with Visual C++. nmake is available. I followed this instruction to build ruby.

I agree with @tenderlove . Calling the make command on your own could require the reinvention of some parts of mkmf. It would waste your (and users') time.

tenderlove added a commit that referenced this issue Dec 5, 2024
This commit teaches mkmf about the libprism sources and allows it to
compile them directly

Fixes #3273
tenderlove added a commit that referenced this issue Dec 5, 2024
This commit teaches mkmf about the libprism sources and allows it to
compile them directly

Fixes #3273
tenderlove added a commit that referenced this issue Dec 5, 2024
This commit teaches mkmf about the libprism sources and allows it to
compile them directly

Fixes #3273
tenderlove added a commit that referenced this issue Dec 5, 2024
This commit teaches mkmf about the libprism sources and allows it to
compile them directly

Fixes #3273
@flavorjones
Copy link
Contributor

Calling "make" from the extconf was intended to be a temporary solution to help transition the project from "just build a static archive" to "ship a static archive and a shared object file and also make a gem out of it".

There were a few different opinions on how to proceed, and it wasn't clear how TruffleRuby integration or CRuby integration was going to be implemented. My recollection is that once we had something working, we all moved onto more urgent tasks rather than argue about how to finish untangling it.

@tenderlove
Copy link
Member

tenderlove commented Dec 5, 2024

There were a few different opinions on how to proceed, and it wasn't clear how TruffleRuby integration or CRuby integration was going to be implemented.

I think my solution in #3289 continues to support CRuby and non-CRuby implementations correctly. IMO we should think about the Prism repository as actually containing 2 different projects:

  1. A C library used for parsing Ruby code: libprism
  2. A Ruby gem that wraps the C library: prism

I think the goals for libprism are as follows:

  1. It can parse Ruby code and produce an AST
  2. It is an independent, stand-alone parser (it has no dependencies on CRuby, or hopefully any other library)

The second goal means that fully embedding the source of libprism in a gem (say a gem called "prism"), or even in a programming language runtime (say "CRuby") should be a supported / valid use cases.

Since libprism is a stand-alone C library which can be embedded for use in any project, it's important that we maintain a Makefile (or configure scripts, etc) so that people who want to build libprism can do so, and also so that we can run CI with different C compilers and different compilation flags.

Anyway, I don't think anything I've written above is contrary to anything anyone else here has said. But it's the mental framework I used for the changes in #3289, and why I think it's ok for the gem to stop producing an intermediate .a file, and just compile / link against the libprism C source files directly.

@eregon
Copy link
Member

eregon commented Dec 8, 2024

We should maintain a Makefile in Prism and test compilation with these different flags.

Right, the Makefile is at least tested in Prism CI when installing the prism gem on JRuby & TruffleRuby (e.g. https://github.com/ruby/prism/actions/runs/12221401729/job/34090404723?pr=3293#step:5:1).
That's libprism.so though, I'm not sure building libprism.a is tested anymore in Prism's CI (EDIT: it seems tested by rust/ruby-prism-sys/build/main.rs, and also just make from rake compile builds it).

It seems like the Prism gem is treating the Prism C library (libprism.a) as if it's a "package that hasn't been installed yet", and is taking on the responsibility of compiling / installing that package on the user's system. In other words, we're taking on the complexity of knowing how to correctly install a C library on all systems. This seems fraught to me (and is a problem that mkmf is designed to solve).

I see .a as "basically a .tar of .o files", so that part seems fairly simple and portable.
I wish we could reuse the root Makefile and delegate to it in order to avoid duplication, but C build systems are still a PITA notably due to the tiny common subset of *make features.

I think your PR is fine and agree it's a straightforward way to address this issue.

Is TruffleRuby using the .a file that gets built along side this gem? Presumably you can install TruffleRuby without the Prism gem?

It's used when building TruffleRuby itself, libprism.a is used there as the only Ruby parser in TruffleRuby (https://github.com/ruby/prism/blob/main/docs/build_system.md#building-prism-as-part-of-truffleruby for more details, I'll fix the broken link there).

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

Successfully merging a pull request may close this issue.

5 participants