takari / polyglot-maven

Support alternative markup for Apache Maven POM files
Eclipse Public License 1.0
893 stars 101 forks source link

Non-whitespace warning for Ruby polyglot build file #312

Closed headius closed 5 months ago

headius commented 5 months ago

We seem to be having an issue with polyglot-ruby leading to a warning about non-whitespace being in the pom file.

https://github.com/jruby/jruby/issues/7059

The error buried in here is from our JRuby library "jar-dependencies" using a polyglot-ruby pom file to gather all Java dependencies, write them to a file, and then later proceed to vendor those jars into the Ruby gem.

The top-level polyglot file for this action is here:

https://github.com/mkristian/jar-dependencies/blob/b4e80f0d29cf5030dde566c22b4d8a32e3610681/lib/jars/gemspec_pom.rb

Two files in the same directory are evaluated at build time:

https://github.com/mkristian/jar-dependencies/blob/b4e80f0d29cf5030dde566c22b4d8a32e3610681/lib/jars/attach_jars_pom.rb

https://github.com/mkristian/jar-dependencies/blob/b4e80f0d29cf5030dde566c22b4d8a32e3610681/lib/jars/output_jars_pom.rb

Somewhere along the way this seems to sometimes produce a pom.xml output that is invalid, with the # comments possibly getting injected before the opening start tag:

https://github.com/jruby/jruby/issues/7059#issuecomment-2166963078

Oddly this seems to have just started affecting some users very recently, within the last couple of weeks.

@mkristian Do you have any thoughts on why this would suddenly start to fail?

unflxw commented 5 months ago

I have found a minimal reproduction test case, and it indicates the issue likely lies with installing Psych on recent versions of RubyGems or Bundler.

From inside a JRuby environment, such as docker run -it jruby:latest /bin/bash:

gem update --system
echo -e "source 'https://rubygems.org/'\ngem 'psych'" > Gemfile
bundle install
headius commented 5 months ago

I have done more investigation and it appears it started breaking when we updated the Ruby gems ruby-maven and ruby-maven-libs to Maven 3.8.9 from 3.3.9.

During the build, the wrappers in those gems programmatically invoke Maven passing -f with a path to a Ruby polyglot file (gemspec_pom.rb). When using ruby-maven 3.3.9 (wrapping Maven 3.3.9) this worked correctly. But with ruby-maven 3.8.9 (Maven 3.8.9) it does not appear to see this as a polyglot Ruby pom and tries to parse it as XML. That produces the error shown in jruby/jruby#7059.

The other items updated in ruby-maven during that time were JRuby itself and Polyglot Maven (to 0.7.0).

Is some combination of these preventing the target file from being recognized as a polyglot ruby pom script?

cstamas commented 5 months ago

@headius Maven 3.8.9 does not exists (3.8.8 is latest in 3.8.x line). Is it a lapsus maybe, and you mean 3.9.8? Can you give me some reproducer? Or maybe point me at some repo that shows that error?

cstamas commented 5 months ago

@headius @unflxw reproduced using provided reproducer (missed it initially): https://gist.github.com/cstamas/dbb7af31f86cad5412348d3659d95f1d

Added to reproducer export MAVEN_ARGS="-e" and got this: https://gist.github.com/cstamas/87c96aad2f2dfaee84ce5c6be885f285

So yes, it seems the default model reader is being used.

cstamas commented 5 months ago

Debug https://gist.github.com/cstamas/585644ba009c0c43119983f3ed6a2c5f

So, I don't see how polyglot-ruby comes into play here (where it is being added)? As according to debug log, it is "vanilla" Maven 3.9.6 being invoked without any extension? And vanilla Maven has no clue what polyglot POM is... I am probably missing something....

Ah, I see .mvn directory in repo w/ extensions https://github.com/jruby/ruby-maven-libs, but I don't find this in reproducer path /usr/local/bundle/gems/ruby-maven-libs-3.9.6.1. This commit is sus to me (am not a Ruby guy), as in reproducer docker I don't see .mvn directory: https://github.com/jruby/ruby-maven-libs/commit/755ab36e89657d6bb5c6a5908e8a573f5f16b59f

To me it seems the polyglot ruby extension is simply not loaded up, and vanilla Maven chokes on ruby POM.

headius commented 5 months ago

@cstamas I believe I found my culprit: https://github.com/apache/maven/pull/94

This changed logic almost 8 years ago to no longer look in the current directory for .mvn, and the ruby-maven wrapper library depends on that behavior to temporarily enable polyglot-ruby for builds. I detailed my explorations here:

https://github.com/jruby/jruby/issues/7059#issuecomment-2190953877

I will need to patch ruby-maven in some better way to always enable polyglot-ruby without a specific .mvn/extensions.xml file being tossed around.

cstamas commented 5 months ago

Maybe simplest fix would be in ruby-maven-libs, to simply stuff polyglot into /lib/ext (w/ deps) and drop .mvn/extensions.xml? That way ruby-maven-libs would always have ruby polyglot, instead to attempt to resolve it from whatever config file... or is using different polyglot versions a requirement (and/or users can alter the .mvn/extensions.xml?).

headius commented 5 months ago

Oh if that's how easy it is that would be great! Yes, it is assumed that Ruby users using ruby-maven will want ruby polyglot available by default, so it makes sense to just ship the appropriate extension jars in the right place. I'll give that a try. Thanks for saving me time!

cstamas commented 5 months ago

Yup, basically just stuff these into lib/ext:

[cstamas@blondie ~]$ mvn eu.maveniverse.maven.plugins:toolbox:gav-tree -Dgav=io.takari.polyglot:polyglot-ruby:0.7.0
[INFO] Scanning for projects...
[INFO] 
[INFO] ------------------< org.apache.maven:standalone-pom >-------------------
[INFO] Building Maven Stub Project (No POM) 1
[INFO] --------------------------------[ pom ]---------------------------------
[INFO] 
[INFO] --- toolbox:0.1.23:gav-tree (default-cli) @ standalone-pom ---
[INFO] io.takari.polyglot:polyglot-ruby:jar:0.7.0
[INFO] +- io.takari.polyglot:polyglot-common:jar:0.7.0 [compile]
[INFO] \- org.jruby:jruby:pom:9.4.5.0 [compile]
[INFO]    +- org.jruby:jruby-base:jar:9.4.5.0 [compile]
[INFO]    |  +- org.ow2.asm:asm:jar:9.2 [compile]
[INFO]    |  +- org.ow2.asm:asm-commons:jar:9.2 [compile]
[INFO]    |  |  +- org.ow2.asm:asm-tree:jar:9.2 [compile]
[INFO]    |  |  \- org.ow2.asm:asm-analysis:jar:9.2 [compile]
[INFO]    |  +- org.ow2.asm:asm-util:jar:9.2 [compile]
[INFO]    |  +- com.github.jnr:jnr-netdb:jar:1.2.0 [compile]
[INFO]    |  +- com.github.jnr:jnr-enxio:jar:0.32.16 [compile]
[INFO]    |  +- com.github.jnr:jnr-unixsocket:jar:0.38.21 [compile]
[INFO]    |  +- com.github.jnr:jnr-posix:jar:3.1.18 [compile]
[INFO]    |  +- com.github.jnr:jnr-constants:jar:0.10.4 [compile]
[INFO]    |  +- com.github.jnr:jnr-ffi:jar:2.2.15 [compile]
[INFO]    |  |  +- com.github.jnr:jnr-a64asm:jar:1.0.0 [compile]
[INFO]    |  |  \- com.github.jnr:jnr-x86asm:jar:1.0.2 [compile]
[INFO]    |  +- com.github.jnr:jffi:jar:1.3.12 [compile]
[INFO]    |  +- com.github.jnr:jffi:jar:native:1.3.12 [compile]
[INFO]    |  +- org.jruby.joni:joni:jar:2.2.1 [compile]
[INFO]    |  +- org.jruby.jcodings:jcodings:jar:1.0.58 [compile]
[INFO]    |  +- org.jruby:dirgra:jar:0.3 [compile]
[INFO]    |  +- com.headius:invokebinder:jar:1.13 [compile]
[INFO]    |  +- com.headius:options:jar:1.6 [compile]
[INFO]    |  +- org.jruby:jzlib:jar:1.1.5 [compile]
[INFO]    |  +- joda-time:joda-time:jar:2.12.5 [compile]
[INFO]    |  +- me.qmx.jitescript:jitescript:jar:0.4.1 [compile]
[INFO]    |  \- com.headius:backport9:jar:1.13 [compile]
[INFO]    \- org.jruby:jruby-stdlib:jar:9.4.5.0 [compile]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  0.639 s
[INFO] Finished at: 2024-06-26T20:33:50+02:00
[INFO] ------------------------------------------------------------------------
[cstamas@blondie ~]$ 
headius commented 5 months ago

Sounds good, I will try to coax @mkristian out of retirement to help me fix this in the build for the ruby-maven-libs gem.

I don't think there's any bug here to fix on the polyglot side.