NREL / OpenStudio

OpenStudio is a cross-platform collection of software tools to support whole building energy modeling using EnergyPlus and advanced daylight analysis using Radiance.
https://www.openstudio.net/
Other
484 stars 185 forks source link

OS 3.8 broke json-schema validation #5208

Closed shorowit closed 1 month ago

shorowit commented 1 month ago

Issue overview

OpenStudio 3.7 works (validation error returned):

openstudio -e 'require "json-schema"; require "stringio"; schema = { "type" => "object", "required" => ["a"], "properties" => { "a" => {"type" => "integer"} } }; begin; JSON::Validator.validate!(schema, { "a" => "taco" }); rescue JSON::Schema::ValidationError => e; puts e.message; end'
The property '#/a' of type String did not match the following type: integer

OpenStudio 3.8 fails (ruby exception):

openstudio -e 'require "json-schema"; require "stringio"; schema = { "type" => "object", "required" => ["a"], "properties" => { "a" => {"type" => "integer"} } }; begin; JSON::Validator.validate!(schema, { "a" => "taco" }); rescue JSON::Schema::ValidationError => e; puts e.message; end'
terminate called after throwing an instance of 'RubyException'
  what():  NameError: uninitialized constant IO::StringIO
Aborted (core dumped)
shorowit commented 1 month ago

Maybe it's as simple as updating the gem? The OS CLI has json-schema 2.7.0 which was released in 2016 (!!). The latest version is 4.3.0. I see that 2.7.0 doesn't support ruby 3.

wenyikuang commented 1 month ago
(os 3.8.0) :004 > Gem.loaded_specs['json-schema'].version
 => Gem::Version.new("2.7.0")

Take a really quick deep dive, this old-dated package comes from:

    tbd (3.4.1)
      json-schema (~> 2.7.0)
      osut (~> 0)
      topolys (~> 0)

https://github.com/rd2/tbd/blob/develop/tbd.gemspec#L33

shorowit commented 1 month ago

<rant>I still don't understand why gems like these are packaged into the OS CLI. There are lots of other OS gems/measures that are generally useful, but they aren't packaged into the CLI. If someone wants to use the tbd gem, they can add it for their workflow.</rant>

jmarrec commented 1 month ago

@shorowit Tell that to openstudio-standards. https://github.com/NREL/openstudio-standards/blob/c29f1e599799112295d3b1ef0b116a7c6632d6ba/openstudio-standards.gemspec#L52

This is used for BTAP / NECB. The Canadian codes take thermal bridging into account.

jmarrec commented 1 month ago

Also, this works fine if you read the error message and just reorder the imports

-openstudio -e 'require "json-schema"; require "stringio"; ...
+openstudio -e 'require "stringio"; require "json-schema"; ...

I'm not saying the StringIO thing isn't an issue. It's one I had already encountered outside of this gem I think.

But also, that's because the gem uses StringIO and doesn't actually require 'stringio', cf https://github.com/voxpupuli/json-schema/blob/4aed27d505aa3e31445bf60e0718203af2d40f5f/lib/json-schema/validator.rb#L441 . I think it works on regular ruby but not on ours because of the static initialization we have. Perhaps we're missing an Init_ call somewhere. Anyways, I opened an upstream PR at https://github.com/voxpupuli/json-schema/pull/512

jmarrec commented 1 month ago

The easiest might just be to do the same as here: https://github.com/NREL/OpenStudio/blob/cdee54c5fbbd53f2bbaca7bb2828cfa08fc89b4f/ruby/engine/openstudio_cli.rb#L45

eg put it here: https://github.com/NREL/OpenStudio/blob/cdee54c5fbbd53f2bbaca7bb2828cfa08fc89b4f/ruby/bindings/InitRubyBindings.cpp#L474


There is a "Init_stringio" https://github.com/ruby/ruby/blob/e51014f9c05aa65cbf203442d37fef7c12390015/ext/stringio/stringio.c#L1864

Except it is supposedly called already via ext/extinit.c. On my mac:

$ cat /Users/julien/.conan2/p/b/rubyd95c0c271f785/p/lib/ext/extinit.c

#include "ruby/ruby.h"

#define init(func, name) {  \
    extern void func(void); \
    ruby_init_ext(name".so", func); \
}

void ruby_init_ext(const char *name, void (*init)(void));

void Init_ext(void)
{
    init(Init_bigdecimal, "bigdecimal");
    init(Init_escape, "cgi/escape");
    init(Init_continuation, "continuation");
    init(Init_coverage, "coverage");
    init(Init_date_core, "date_core");
    init(Init_digest, "digest");
    init(Init_bubblebabble, "digest/bubblebabble");
    init(Init_md5, "digest/md5");
    init(Init_rmd160, "digest/rmd160");
    init(Init_sha1, "digest/sha1");
    init(Init_sha2, "digest/sha2");
    init(Init_escape, "erb/escape");
    init(Init_etc, "etc");
    init(Init_fcntl, "fcntl");
    init(Init_fiddle, "fiddle");
    init(Init_console, "io/console");
    init(Init_nonblock, "io/nonblock");
    init(Init_wait, "io/wait");
    init(Init_generator, "json/ext/generator");
    init(Init_parser, "json/ext/parser");
    init(Init_monitor, "monitor");
    init(Init_nkf, "nkf");
    init(Init_objspace, "objspace");
    init(Init_openssl, "openssl");
    init(Init_pathname, "pathname");
    init(Init_psych, "psych");
    init(Init_pty, "pty");
    init(Init_cparse, "racc/cparse");
    init(Init_sizeof, "rbconfig/sizeof");
    init(Init_readline, "readline");
    init(Init_ripper, "ripper");
    init(Init_socket, "socket");
    init(Init_stringio, "stringio");
    init(Init_strscan, "strscan");
    init(Init_syslog, "syslog");
    init(Init_zlib, "zlib");
}