Daniel, I added some convenience methods, but you were ultimately correct! :) Big honkin' patch incoming.
Jon 2012/3/23 Daniel Dai <[email protected]> > One confusion I have is do we really need require "pig.jar"? > PIG-2317-8.patch+PIG-2317-8_plus.patch works for me last time, and I > drop require "pig.jar". Did we change anything in PIG-2317-9.patch? > > Daniel > > On Fri, Mar 23, 2012 at 2:50 PM, Jonathan Coveney <[email protected]> > wrote: > > Alan, > > > > My idea wasn't to have a patched version included with Pig, just one > > specific file that needed to be patched, that way it would be earlier in > > the classpath than the version in JRuby. If you think it is cleaner to > just > > have a patched version of JRuby that we depend on, it would accomplish > the > > same thing. But I definitely agree that we shouldn't include JRuby itself > > with Pig. But right now I have a file at src/org/jruby/path/to/file.java, > > and since there will be this lone JRuby file, in the case that it gets > > called, we are good > > > > Daniel, > > > > The issue only cropped up because, in a sense, another bug was hiding > it. I > > thought 1.9 was the default version of Ruby that JRuby intepreted, but I > > was wrong (1.8 was the default). I fixed this, and then the issue cropped > > up. > > > > For people who aren't rubyists and don't know much about Ruby versions: > > 1.8.7 is the equivalent of Python 2.7, and 1.9.2 is the equivalent of > > Python 3.3, the difference being that most everyone with new projects > uses > > 1.9.2 because it is better in many ways, and didn't break the language > like > > Python did (this is where the analogy breaks down). Another way to think > of > > it might be that we want to use Pig 0.9 (1.9) instead of Pig 0.8 (1.8), > > especially because 1.9 is getting all of the active development, so in > the > > future most bug fixes are going to be for 1.9 etc. That said, the 1.9 > piece > > has this one bug (much as people using Pig 0.9 instead of Pig 0.8 ran > into > > some weird bugs b/c of the parser changes). > > > > The bug is as follows: > > If you have a file on your classpath, and then "require" a resource with > > that exact same name, you get a String index error. This crops up when, > > internally to Pig, it loads the pigudf.rb file into the ruby runtime on > the > > same JVM. pig.jar is on the classpath, but the script has to 'require > > pig.jar' because it needs to know about things like DataBags and whatnot. > > This causes an error because of a subtle bug that rarely comes up in pure > > JRuby projects, since pure JRuby projects usually don't set the classpath > > on the jvm level. This is a pretty difficult bug to workaround: pig.jar > has > > to be on the classpath, and requiring it in ruby is also necessary from > > within pig. > > > > Either way, the fix is trivial, but 1.7.0 won't be out for a month or > two, > > and in the meantime, it's pretty bad. I can try to come up with some > weird > > hack workaround, but depending on a fork of JRuby seems like it would be > a > > lot more reasonable. Having the one .java file that needs to be fixed is > > also easy, but might be ugly. I personally would really prefer to just go > > with the fixed version, because the workaround is going to be pretty > > difficult (since the error is baked deep in there) and ugly. If we have a > > forked dependency, when 1.7.0 comes out we can just depend on that and > bam. > > > > Sorry if that's a bit long, I just wanted to thoroughly document the > issue. > > Jon > > > > 2012/3/23 Alan Gates <[email protected]> > > > >> Won't a lot of people already have their version of JRuby and not want a > >> special one? I'm fine with having a patched version on github and > >> referring it in our release notes. I'm not wild about including a > version > >> of JRuby with Pig, for both licensing reasons and because our tar file > is > >> bloated enough as it is. > >> > >> Alan. > >> > >> On Mar 23, 2012, at 11:38 AM, Daniel Dai wrote: > >> > >> > Hi, Jonathan, > >> > What bug is it? Last time when I try, it seems work well for me. We > >> > can leave a small hole and describe the limitation clearly in release > >> > notes/code comments/javadocs, we can also provide a link to the ticket > >> > tracking the issue. I remember we did something similar for javacc > >> > before. However, I don't think we shall include a JRuby patch in Pig. > >> > > >> > Daniel > >> > > >> > On Fri, Mar 23, 2012 at 10:01 AM, Jonathan Coveney < > [email protected]> > >> wrote: > >> >> First off: JRuby patch is almost done. It's passing tests, I have > some > >> more > >> >> to add, but I think the definitive version to work off will be out > today > >> >> (assuming we can reconcile what follows :) > >> >> > >> >> I hit a bug in JRuby that is pretty impossible to avoid (it's a bug > in > >> the > >> >> way files were found on the classpath). I figured out the bug and let > >> the > >> >> JRuby devs know and they patched master, but that means that our > >> version is > >> >> still buggy. I put a patched version of the file in the Pig project > >> pending > >> >> a new JRuby release, and this works, but there are two issues: > >> >> 1) Is this how we want this to be structued? It's weird to have this > >> random > >> >> file in there, but on the other hand, it's a clean and clear fix. > >> >> 2) Is this legal? JRuby has a kind of odd triple license and I think > you > >> >> can choose 1 for pieces that aren't explicitly GPL (of which there > are > >> very > >> >> few). One of those licenses is the CPL, which Apache says is kosher > as > >> long > >> >> as you're explicit, but I don't know. Is this fine? Should I talk to > >> JRuby > >> >> or Apache legal? > >> >> > >> >> I suppose the alternative would be to publish a patched version of > JRuby > >> >> (we could fork it on Github) and depend on that. > >> >> > >> >> I appreciate your comments > >> >> Jon > >> > >> >
