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

Rename librubyparser to libprism #1802

Merged
merged 1 commit into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ STATIC_OBJECTS := $(subst src/,build/static/,$(SOURCES:.c=.o))

all: shared static

shared: build/librubyparser.$(SOEXT)
static: build/librubyparser.a
shared: build/libprism.$(SOEXT)
static: build/libprism.a
wasm: javascript/src/prism.wasm

build/librubyparser.$(SOEXT): $(SHARED_OBJECTS)
build/libprism.$(SOEXT): $(SHARED_OBJECTS)
$(ECHO) "linking $@"
$(Q) $(CC) $(DEBUG_FLAGS) $(CFLAGS) -shared -o $@ $(SHARED_OBJECTS)

build/librubyparser.a: $(STATIC_OBJECTS)
build/libprism.a: $(STATIC_OBJECTS)
$(ECHO) "building $@"
$(Q) $(AR) $(ARFLAGS) $@ $(STATIC_OBJECTS) $(Q1:0=>/dev/null)

Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This is a parser for the Ruby programming language. It is designed to be portabl

## Overview

The repository contains the infrastructure for both a shared library (librubyparser) and a native CRuby extension. The shared library has no bindings to CRuby itself, and so can be used by other projects. The native CRuby extension links against `ruby.h`, and so is suitable in the context of CRuby.
The repository contains the infrastructure for both a shared library (libprism) and a native CRuby extension. The shared library has no bindings to CRuby itself, and so can be used by other projects. The native CRuby extension links against `ruby.h`, and so is suitable in the context of CRuby.

```
.
Expand All @@ -21,7 +21,7 @@ The repository contains the infrastructure for both a shared library (librubypar
├── ext
│   └── prism
│   ├── extconf.rb configuration to generate the Makefile for the native extension
│   └── extension.c the native extension that interacts with librubyparser
│   └── extension.c the native extension that interacts with libprism
├── fuzz files related to fuzz testing
├── include
│   ├── prism header files for the shared library
Expand Down
12 changes: 6 additions & 6 deletions docs/build_system.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@ The main solution for the second point seems a Makefile, otherwise many of the u
## General Design

1. Templates are generated by `templates/template.rb`
4. The `Makefile` compiles both `librubyparser.a` and `librubyparser.{so,dylib,dll}` from the `src/**/*.c` and `include/**/*.h` files
4. The `Makefile` compiles both `libprism.a` and `libprism.{so,dylib,dll}` from the `src/**/*.c` and `include/**/*.h` files
5. The `Rakefile` `:compile` task ensures the above prerequisites are done, then calls `make`,
and uses `Rake::ExtensionTask` to compile the C extension (using its `extconf.rb`), which uses `librubyparser.a`
and uses `Rake::ExtensionTask` to compile the C extension (using its `extconf.rb`), which uses `libprism.a`

This way there is minimal duplication, and each layer builds on the previous one and has its own responsibilities.

The static library exports no symbols, to avoid any conflict.
The shared library exports some symbols, and this is fine since there should only be one librubyparser shared library
The shared library exports some symbols, and this is fine since there should only be one libprism shared library
loaded per process (i.e., at most one version of the prism *gem* loaded in a process, only the gem uses the shared library).

## The various ways to build prism
Expand All @@ -36,11 +36,11 @@ loaded per process (i.e., at most one version of the prism *gem* loaded in a pro

The gem contains the pre-generated templates.
When installing the gem, `extconf.rb` is used and that:
* runs `make build/librubyparser.a`
* runs `make build/libprism.a`
* compiles the C extension with mkmf

When installing the gem on JRuby and TruffleRuby, no C extension is built, so instead of the last step,
there is Ruby code using FFI which uses `librubyparser.{so,dylib,dll}`
there is Ruby code using FFI which uses `libprism.{so,dylib,dll}`
to implement the same methods as the C extension, but using serialization instead of many native calls/accesses
(JRuby does not support C extensions, serialization is faster on TruffleRuby than the C extension).

Expand All @@ -67,7 +67,7 @@ The script generates the templates when importing.
Then when `mx build` builds TruffleRuby and the `prism` mx project inside, it runs `make`.

Then the `prism bindings` mx project is built, which contains the [bindings](https://github.com/oracle/truffleruby/blob/master/src/main/c/prism_bindings/src/prism_bindings.c)
and links to `librubyparser.a` (to avoid exporting symbols, so no conflict when installing the prism gem).
and links to `libprism.a` (to avoid exporting symbols, so no conflict when installing the prism gem).

### Building prism as part of JRuby

Expand Down
37 changes: 19 additions & 18 deletions ext/prism/extconf.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,25 @@ module ExtConf
class << self
def configure
unless RUBY_ENGINE == "ruby"
# On non-CRuby we only need the shared library, so build only that and not the C extension.
# We also avoid `require "mkmf"` as that prepends the LLVM toolchain to PATH on TruffleRuby,
# but we want to use the native toolchain here since librubyparser is run natively.
build_shared_rubyparser
# On non-CRuby we only need the shared library, so build only that and
# not the C extension. We also avoid `require "mkmf"` as that prepends
# the LLVM toolchain to PATH on TruffleRuby, but we want to use the
# native toolchain here since libprism is run natively.
build_shared_prism
File.write("Makefile", "all install clean:\n\t@#{RbConfig::CONFIG["NULLCMD"]}\n")
return
end

require "mkmf"
configure_c_extension
configure_rubyparser
configure_libprism

create_makefile("prism/prism")

if static_link?
File.open('Makefile', 'a') do |mf|
mf.puts
mf.puts '# Automatically rebuild the extension if librubyparser.a changed'
mf.puts '# Automatically rebuild the extension if libprism.a changed'
mf.puts '$(TARGET_SO): $(LOCAL_LIBS)'
end
end
Expand All @@ -35,19 +36,19 @@ def configure_c_extension
append_cflags("-fvisibility=hidden")
end

def configure_rubyparser
def configure_libprism
if static_link?
static_archive_path = File.join(build_dir, "librubyparser.a")
static_archive_path = File.join(build_dir, "libprism.a")
unless File.exist?(static_archive_path)
build_static_rubyparser
build_static_prism
end
$LOCAL_LIBS << " #{static_archive_path}"
else
shared_library_path = File.join(build_dir, "librubyparser.#{RbConfig::CONFIG["SOEXT"]}")
shared_library_path = File.join(build_dir, "libprism.#{RbConfig::CONFIG["SOEXT"]}")
unless File.exist?(shared_library_path)
build_shared_rubyparser
build_shared_prism
end
unless find_library("rubyparser", "pm_parser_init", build_dir)
unless find_library("prism", "pm_parser_init", build_dir)
raise "could not link against #{File.basename(shared_library_path)}"
end
end
Expand All @@ -60,15 +61,15 @@ def configure_rubyparser
find_header("prism/extension.h", File.join(__dir__, "..")) or raise "prism/extension.h is required"
end

def build_shared_rubyparser
build_target_rubyparser "build/librubyparser.#{RbConfig::CONFIG["SOEXT"]}"
def build_shared_prism
build_target_prism "build/libprism.#{RbConfig::CONFIG["SOEXT"]}"
end

def build_static_rubyparser
build_target_rubyparser "build/librubyparser.a"
def build_static_prism
build_target_prism "build/libprism.a"
end

def build_target_rubyparser(target)
def build_target_prism(target)
Dir.chdir(root_dir) do
if !File.exist?("include/prism/ast.h") && Dir.exist?(".git")
# this block only exists to support building the gem from a "git" source,
Expand Down Expand Up @@ -99,7 +100,7 @@ def print_help

--enable-static
--disable-static
Enable or disable static linking against librubyparser.
Enable or disable static linking against libprism.
The default is to statically link.

--enable-debug-mode-build
Expand Down
2 changes: 1 addition & 1 deletion ext/prism/extension.c
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#include "prism/extension.h"

// NOTE: this file should contain only bindings. All non-trivial logic should be
// in librubyparser so it can be shared its the various callers.
// in libprism so it can be shared its the various callers.

VALUE rb_cPrism;
VALUE rb_cPrismNode;
Expand Down
2 changes: 1 addition & 1 deletion lib/prism/ffi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ module LibRubyParser # :nodoc:

# Define the library that we will be pulling functions from. Note that this
# must align with the build shared library from make/rake.
ffi_lib File.expand_path("../../build/librubyparser.#{RbConfig::CONFIG["SOEXT"]}", __dir__)
ffi_lib File.expand_path("../../build/libprism.#{RbConfig::CONFIG["SOEXT"]}", __dir__)

# Convert a native C type declaration into a symbol that FFI understands.
# For example:
Expand Down
8 changes: 4 additions & 4 deletions rust/prism-sys/build/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ fn main() {
let ruby_build_path = prism_lib_path();
let ruby_include_path = prism_include_path();

// Tell cargo/rustc that we want to link against `librubyparser.a`.
println!("cargo:rustc-link-lib=static=rubyparser");
// Tell cargo/rustc that we want to link against `libprism.a`.
println!("cargo:rustc-link-lib=static=prism");

// Add `[root]/build/` to the search paths, so it can find `librubyparser.a`.
// Add `[root]/build/` to the search paths, so it can find `libprism.a`.
println!("cargo:rustc-link-search=native={}", ruby_build_path.to_str().unwrap());

// This is where the magic happens.
Expand All @@ -23,7 +23,7 @@ fn main() {
write_bindings(&bindings);
}

/// Gets the path to project files (`librubyparser*`) at `[root]/build/`.
/// Gets the path to project files (`libprism*`) at `[root]/build/`.
///
fn prism_lib_path() -> PathBuf {
if let Ok(lib_dir) = std::env::var("PRISM_LIB_DIR") {
Expand Down
2 changes: 1 addition & 1 deletion rust/prism-sys/build/vendored.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub fn build() -> Result<(), Box<dyn std::error::Error>> {

build.files(source_files(src_dir()));
build.out_dir(&out_dir);
build.try_compile("rubyparser")?;
build.try_compile("prism")?;

std::env::set_var("PRISM_INCLUDE_DIR", include_dir());
std::env::set_var("PRISM_LIB_DIR", out_dir);
Expand Down
2 changes: 1 addition & 1 deletion rust/prism-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#[allow(non_upper_case_globals)]
mod bindings {
// In `build.rs`, we use `bindgen` to generate bindings based on C headers
// and `librubyparser`. Here is where we pull in those bindings and make
// and `libprism`. Here is where we pull in those bindings and make
// them part of our library.
include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
}
Expand Down
28 changes: 14 additions & 14 deletions test/prism/library_symbols_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ class LibrarySymbolsTest < TestCase
def setup
super

@librubyparser_a = File.expand_path("../../build/librubyparser.a", __dir__)
@librubyparser_so = File.expand_path("../../build/librubyparser.so", __dir__)
@libprism_a = File.expand_path("../../build/libprism.a", __dir__)
@libprism_so = File.expand_path("../../build/libprism.so", __dir__)
@prism_so = File.expand_path("../../lib/prism/prism.so", __dir__)
end

Expand Down Expand Up @@ -56,34 +56,34 @@ def names(symbol_lines)
end

#
# static archive - librubyparser.a
# static archive - libprism.a
#
def test_librubyparser_a_contains_nothing_globally_visible
omit("librubyparser.a is not built") unless File.exist?(@librubyparser_a)
def test_libprism_a_contains_nothing_globally_visible
omit("libprism.a is not built") unless File.exist?(@libprism_a)

assert_empty(names(visible_global_objdump_symbols(@librubyparser_a)))
assert_empty(names(visible_global_objdump_symbols(@libprism_a)))
end

def test_librubyparser_a_contains_hidden_pm_symbols
omit("librubyparser.a is not built") unless File.exist?(@librubyparser_a)
def test_libprism_a_contains_hidden_pm_symbols
omit("libprism.a is not built") unless File.exist?(@libprism_a)

names(hidden_global_objdump_symbols(@librubyparser_a)).tap do |symbols|
names(hidden_global_objdump_symbols(@libprism_a)).tap do |symbols|
assert_includes(symbols, "pm_parse")
assert_includes(symbols, "pm_version")
end
end

#
# shared object - librubyparser.so
# shared object - libprism.so
#
def test_librubyparser_so_exports_only_the_necessary_functions
omit("librubyparser.so is not built") unless File.exist?(@librubyparser_so)
def test_libprism_so_exports_only_the_necessary_functions
omit("libprism.so is not built") unless File.exist?(@libprism_so)

names(global_nm_symbols(@librubyparser_so)).tap do |symbols|
names(global_nm_symbols(@libprism_so)).tap do |symbols|
assert_includes(symbols, "pm_parse")
assert_includes(symbols, "pm_version")
end
names(local_nm_symbols(@librubyparser_so)).tap do |symbols|
names(local_nm_symbols(@libprism_so)).tap do |symbols|
assert_includes(symbols, "pm_encoding_shift_jis_isupper_char")
end
# TODO: someone who uses this library needs to finish this test
Expand Down