Fix CI build

[jrochkind]

CI build started failing with no changes to local code.

See #373.

We don't entirely understand why; clearly some change with latest release of dependencies or ruby itself (since there were no changes to local code), but we don't entirely understand what/why.

• Only Rails 6.1+ works with psych 4, although older rails doens't know it, being unmaintained, and needs a local pin to psych 3

  • Don't know why this at one point passed, even after psych 4 had been released.

• Fix test of raw serialized JSON

  • In some versions of ruby/dependencies, there is a literal & in raw serialized JSON source; in other versions, it's a \u0026 (which should mean the same thing once parsed). We don't understand why this differs between different ruby/rails versions, or why it started differing at some point in past with no changes to local code. (I don't really even understand where this String being tested comes from, or why the method under test is returning raw unparsed serialized JSON!)

    But it seems safe to have the test not care whether the serialized JSON contains & or \u0026 -- should be semantically equivalent once parsed. So change the test code to test against parsed value rather than raw value.

[jrochkind]

WOW still failing let me see if I can repro locally....

OK, crap, this is a mess.

Bundler could not find compatible versions for gem "psych":
  In Gemfile:
    psych (< 4)

    linkeddata was resolved to 3.2.1, which depends on
      yaml-ld (~> 0.0) was resolved to 0.0.1, which depends on
        psych (~> 4.0)

OK, I think I solved the answer of why this is started failing.

linkeddata 3.2.1 was released on August 9th 2022. linkeddata 3.2.0 did NOT have a dependency on yaml-ld at all. linkeddata 3.2.1 for the first time has a dependnecy on yaml-ld. yaml-ld has only on release, 0.0.1 on Aug 4 2022. It expresses a dependency on Psych 4.x specifically.

Rails older than 6.1 cannot work with psych 4.x (so far as I know). So linkeddata gem 3.2.1 is no longer compatible with Rails older than 6.1, although linkeddata 3.2.0 was.

(Mystery is why bundler isn't smart enough to resolve to linkedata 3.2.0 instead, it ought to be. But still).

Not sure what to do about this. @cjcolvar @eddierubeiz

[jrochkind]

Another mystery is why linkeddata is expressed only as a development dependency, not a runtime dependency, in the first place. Unless we only use linkeddata in tests and not in real code.... this seems like a bug in the first place? That consuming apps will have to manually express a dependency on linkeddata? Or maybe this was intentional as an "optional" dependency?

Here's the commit where it was changed from a runtime dependency to a development-only dependency. f1f0263#diff-90aba9ede100941abdf2280d8ec1fca20284307ff2c47ea09abf5d3712244350L32 . @awead , do you recall whether this was intentional or what the motivation was?

This is really a separate issue... except that it's part of what's making it really hard to tell what the right thing to do here is.

[jrochkind]

OK, I have the build passing!

I neither entirely underestand nor love what I had to do to get it there.

Rails versions before 6.1 are locked to old linkeddata (for now?), and old psych (which is only necessary cause linkeddata expresses an explicit dependency on default gem psych, which was bringing in an incompatible version).

TOTALLY don't understand that json escaping thing, I'm just kind of guessing that it wasn't a real bug.

What do you think @cjcolvar ?

[jrochkind]

While a yaml-ld 0.0.2 was just released allowing psych 3.x... I still can't get the build to pass if I relax the extra restriction.

Will try again tomorrow.

Honestly, I am half consideirng making a fork of questioning_authority that removes all the linkedata/RDF stuff. I don't understand to what extent it is working/being used, and it's been a maintenance headache (with maintenance being done by people who don't use it, I think, but use other parts of questioning_authority).

[jrochkind]

OK, with yaml-ld 0.0.2 released, it's possible main branch will build again with no changes at all? Trying to figure out how test that.

I don't have the ability to re-run a build in CircleCI ("You need write permission to trigger builds"). @cjcolvar, could you trigger a build on main and see if it passes now?

[jrochkind]

Even though yaml-ld 0.0.2 is released, at least some build, when otherwise unconstricted, are insisting on using 0.0.1, with it's psych 4.x dependendency.

In Gemfile:
    psych (< 4)

    linkeddata was resolved to 3.2.1, which depends on
      yaml-ld (~> 0.0) was resolved to 0.0.1, which depends on
        psych (~> 4.0)

Not totally sure the right way to fix this... I guess I have to put a dependency in gemfile extra saying we need yaml-ld of at least 0.0.2---even though we don't really need yaml-ld at all ourselves here, we just need to keep 0.0.1 from being used as a dependency of linkeddata! Gah.

[jrochkind]

I am having trouble reproducing the ruby 2.5 builds locally, but that's in part because my laptop is seeming to have trouble dealing with ruby 2.5 at all, I can't seem to get nokogiri installed under ruby 2.5. :(

But it's still very weird that it's failing -- in those ruby 2.5 builds, bundler is having problems resolving the dependnecy tree that I don't believe it should. It may be due to a bug in bundler (maybe fixed in bundlers that come with future ruby versions? I can try forcing a more recent bundler maybe...)... but may be due to something involving bundler caching on CircleCI specifically? I don't know if there's any way to reset the CircleCI cache used for gem dependencies, if there is I probablly don't have permissions for it.

[jrochkind]

OK, the only way I was able to get this green was by replacing the (development-only) linkeddata dependency with the individual sub-dependencies necessary for tests to pass. See #375

This way we don't include the un-used yaml-ld in our dependency tree, that was causing problems for bundler being able to resolve a consistent dependency tree, for reasons we only partially understand.

I don't understand enough about what's going on to know the possible implications of this change in our development dependencies... but as they are only development dependencies so, as I understand it, already didn't and won't actually have an effect on actual apps using qa gem, and tests are passing...

No idea. This is a whole lot of things I only semi-understand, to be honest. But... it's green? @cjcolvar ?

[cjcolvar]

This looks good to me. I wonder if after this is merged we should cut a release (v5.9.1) then move these development dependencies to regular dependencies and cut a major release (or maybe it could be a minor release if it is considered a bugfix?). What do you think?

[jrochkind]

I would want to understand what's really going on before moving the development dependencies to a runtime dependency.

Moving them to a major dependency could cause dependency hell for real users, instead of just devs trying to get CI to build.

Does the code they are used in actually work? Is anyone using it? Are most users of qa using it?

With this legacy code where, as far as I know, nobody still around working on maintenance really has the big picture or understands the history or context, my inclination is: First, do no harm. And: If it isn't broken, don't fix it.

Is anyone asking for the linked data dependencies to be made runtime dependencies, is this fixing a problem someone is having?

Another option would be moving all the linked data stuff (and it's dependencies) to a separate gem, say, questioning_authority-linkeddata -- that would be my preferred course of action since I don't use any of the linked data stuff, but get stuck maintaining it and dealing with it's dependency-challenges anyway, the bulk of my maintainance work on qa is then on that stuff I don't understand or use. Of course, that would also be an argument for keeping it in this gem, or it might not be maintained at all!

I would want to ideally find one or two users of this gem (not a fork!) who actually are using the linkeddata stuff, to do some "[developer-]user analysis" before making any major changes. (Do you use that func, @cjcolvar?) (another option, if nobody uses it, is removing it....)

[jrochkind]

Thanks for the review and approval @cjcolvar! I honestly don't love how little I understand what I'm doing here, but I will merge!

[jrochkind]

PS: This particular PR does not require a release, because it only effected test code, it doesn't effect runtime use of the gem at all, no changes to runtime code. Since there don't appear to be any other changes since 5.9.0, I don't think there's a reason for a 5.9.1 release, at present 5.9.1 would be identical to 5.9.0 for users of the gem (I believe development dependencies in gemspec do not effect actual runtime use of the gem).