-
Notifications
You must be signed in to change notification settings - Fork 21
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
Bloop-rifle update #351
Bloop-rifle update #351
Conversation
Fantastic, thanks! I'm busy today and tomorrow - will look at it first thing after that! :) |
The whole point is actually getting a new version of bloop-rifle in, which includes a bugfix for an issues I've run into. I've fixed it locally and I'm running a self built Bleep as such is very cool so far! Thanks for making it! 👍 But I guess it still needs some love here and there. Let's see, maybe I come back with more later on. 😄 |
I'm ok with dropping 2.12 completely at this point. For context: The reason the 2.12 support is there is ironically future-proofing. By having bleep cross-compiled to 2.12, it's possible to port sbt plugins without upgrading the scala version they are written in, and then use them in bleep scripts written in 2.12. I'm willing to drop this, since porting random 2.12 code to 2.13 is normally easy. Dropping 2.13 would not be acceptable yet, it's only recently that it was possible to port the sonatype integration code to 3. Also I thought it beneficial in case it was needed to embed bleep in sbt or vice versa, but I don't see a path in that direction for now. |
Ok, so AFAIU this PR really depends on an unmerged PR in bloop-core. If the build is green I'll go ahead and merge this soon, so the next PR will be a simple version bump |
Ok, not green. I'll debug it from here 👍 |
Hi! Thanks for having a look. I can update the PR as soon as the upstream change is in. The upstream change is the solve reasons for this here… 😄 In case there will be also a bloop-rifle update for 2.12 there is no need to drop the 2.12 support here. Regarding the Native Image related stuff: Like said in the comments I'm not sure why this popped up actually. There is maybe some issue with double dependencies in different version. But it could be also that my local build got broken at some point, and wasn't really clean. So no hurry, we need to wait for upstream anyway. Than I will have a second look. |
I did some work on this, and it seems to be working well. I'll merge this first version and a small pipeline of related changed over the next days. then we'll upgrade to the version you want in a subsequent PR 👍 |
Thanks for looking into that! You found the issues with broken compile when not running over BSP? I've just wanted to dig into that. Could you push whatever you have so I could build already a local version that works? I have not noticed the breakage at the time I've submitted the PR as I was just running Please note that the version of the dependency I want still doesn't exist. We need to wait for upstream… I've opened additionally a ticket in the I understand that a lot of people don't care to have million runtimes in billion versions installed; but imho this is a Windows / MacOS style security catastrophe. All the runtimes, and actually any other stuff installed behind the back of the package manager, are ticking time bombs, just waiting to be exploited in some attack, as this stuff never gets security updates. (And I don't even talk about all the issues with the external downloaders, like that they don't support any package signatures, which opens the floodgates to "supply-chain attacks" by simple DNS manipulation.) So I think it's important that things work fine with a system provided runtime properly installed through package management. |
I found a bunch of smaller things, but not this in particular. it does seem to work however.
Absolutely. https://github.com/oyvindberg/bleep/compare/bloop-rifle-update-2?expand=1 If you're ok with it I'll just push some of the commits which are direct fallout from this to your branch. then create a followup PR or two and merge everything. I'm busy sunday, but monday I have time for this.
There is also some friction between security on one side and repeatable builds/consistent setup on the other side. Personally I focus more on the latter. That said though, I really want to invest in tooling a la |
This sounds great!
Whenever you have time. Thanks for looking into this! Great to see such quickly responding OpenSource project.
I don't think this has much to do with reproducible builds. Actually it's even counter the idea of reproducible builds, imho: The point of reproducible builds is to be able to build stuff independently, likely even with other toolchains, and arrive at the very same result. Forcing your toolchain on everybody is more like the Docker approach: Exactly this is the reason why there is more then enough JVM stuff that works only on "Oracle Java 1.5"… 😉 Of course I know that there are people who like to follow the Windows / MacOS approach because "it's convenient". Likely the same people who don't understand why Windows / MacOS is full of malware, whereas Linux has no issues, even it's technically the same trash as everywhere. (I may say that, I'm using Linux for over two decades so I know that they don't do magic; it's just the much more sane approach to dependencies and software management in general that keeps things reasonable safe. Of course there are people who try to break even that; bullshit like Flatpak exists…)
Before people jump to building their own package managers, it would be good to first make it possible to actually package Scala stuff. It has reasons why there are no official distri Scala package anywhere, and you can't A build tool that doesn't "download the internet", but works finally in some sane way, would be one of the puzzle pieces to fix that. Of course I'm not going to hold you back building the next package manager that installs stuff behind the back of the system. But we have already more then enough of such, imho. Anyway, having Scala be properly integrated into the OpenSource world is frankly currently out of scope, as it's impossible to build the Scala compiler from source. So there is currently no alternative to execute random binaries downloaded form the internet and hope the best… But at least I don't want to double-down on that. What can be provided by trusted sources (like JVMs built by Debian) should come from there. This doesn't mean I don't like the current project dependency update feature. It's much better than the one of the competition. The approach to have a tool re-writable config is very good! Smart design decision, which I applaud. |
Thanks for sharing! Frankly I've encountered the same bug as with my previous local builds.
The issue is that something in the directory structure of In the folder
and fills both folders with compilation results. (In the later under an additional sub-folder like But the new version built from the above branch creates
and the former folder stays empty. I guess that's a changed behavior in I don't really know how to debug that as I don't know where exactly |
Interesting! I did a Have you tried to explicitly shut down the compile server and tried again? Possibly a mismatch somewhere. It's by the way a well-known thing with bloop this behavior. By default it puts your classes in weird directories unless you declare that you "own" the build, in which case class files should end up in I'll try to look a bit further tonight or tomorrow, would be nice to ship that branch and being back to current |
I'll did all that. When testing I'm restarting the compile server anyway. (Otherwise I wouldn't even run into any issues with bloop-rifle, as this "works fine" as long an old instance is running.) I also clean cache directories while testing… This one here appears even with a complete clean build where everything gets downloaded fresh. As I didn't mention that until now: I'm testing the At the time I've submitted this PR I also didn't know about that issues. The BSP server works fine. Also one could build the Do you know actually why |
Indeed @SlowBrainDude . I could reproduce it now. It was I who forgot to clean :D Thanks for noticing, I would definitely have shipped this |
Thanks for having a second look at it. 👍 At least upstream is now ready: https://github.com/scala-cli/bloop-core/releases In case I can help here somehow just tell. But like said above, I have no clue at the moment where to look. In case I have time for hat I would try to bisect the upstream lib. Maybe the behavior change happened after the package rename. Otherwise it's going to be a little bit complicated to look for the change. Of course, In case you know where to look it could be also quite simple to fix that, without looking for the upstream cause. I hope it's like that. |
I think bisection is the simplest option for now actually. I dug into upstream last night, but couldn't spot anything which was wrong. The logic controlling this lives here
You're supposed to set that the build tool owns the build, and the class files should then go to the correct place. Another place to look is the scala-cli changes, as it's very likely the bug has appeared there as well I'll find some time for this, but it'll be a bit on and off, so if you find something that's helpful |
Upstream moved, and some follow-up changes need to be included as well. Graal Native Image also needed some adjustments.
Context: The reason the 2.12 support is there is ironically future-proofing. By having bleep cross-compiled to 2.12, it's possible to port sbt plugins without upgrading the scala version they are written in, and then use them in bleep scripts written in 2.12. I'm willing to drop this, since porting random 2.12 code to 2.13 is normally easy. Dropping 2.13 would not be acceptable yet, it's only recently that it was possible to port the sonatype integration code to 3. Also I thought it beneficial in case it was needed to embed bleep in sbt or vice versa, but I don't see a path in that direction for now.
7239001
to
3bc5763
Compare
ok, got it! the native-image configuration lost the entry for Bumped the bloop version as well, so this is ready to go now 🎈 |
OH NO! It was me… 💩 Sorry for that trouble. That's why you should not touch things even everything looks like it "works fine" without them… |
3bc5763
to
2dba82a
Compare
Me idiot forgot to place a comment there to ask what this actually is, and why it compiles without it. Now I remember I wanted to do that. But just forgot. Really sorry! 😞 |
2dba82a
to
a61ecb3
Compare
oh don't worry, stuff like that happens. so yeah, those files instruct graalvm native image to generate code to support reflection for some types. and the whole bsp4j thingie we inherit from bloop-rifle is ultimately based on that. ugh. |
It should not. This was really stupid. When I'm touching code I'm not sure about I should at least ask. But in this case I've overlooked this place in the end myself… 😞
I've got that. The native image crashed and was complaining about not marked reflective access as I've first tried. That's was the reason I've touched stuff there in the first place. But I touched too much… Strange that the GraalVM didn't crash or at least complain about these classes here. But I guess I just don't know enough about Native Image currently. Maybe there are some switches to let it search for such cases as here? |
yes, there is an agent you can give to the JVM so that it'll record all reflections and so it encounters. I haven't really needed it that much, so I haven't looked into it. |
I've heard about that. But that's something you do before you build your native images. But I was more thinking about what happened here. The the native image didn't crash or complain. It just silently failed. (For the other reflective calls it just crashed, with a helpful error message.) |
yeah, that would have been nice. I guess it is so common with reflection in java that crashing is likely a bad choice. complaining would have been 👍 |
This has come up before, that when building bleep while it changes bloop release in the middle is complicated. try to first compile and build a dev-script with the old bloop, turn it off, and then run the devscript version
576e38a
to
3c40861
Compare
🥳 |
Does the last commit need some external documentation? I also had trouble building it locally. Now just trying what I've read in the CI config. But in case this is expected to happen again in the future when updating bloop, maybe this magic incantation ("turning it off and on again" 😁) should become a documented build step? |
Thank you so much for pushing it over the finish line! ❤️ |
So i think this only applies when upgrading bloop. There is some functionality within bloop-rifle to check bloop version when it's connecting, and if bloop is too old it shuts it down (check usage of I don't intend to dig in and fix this anytime soon (it's the magical combination of difficult and boring). If anything, we could try to patch the error message to something which asks you to turn it off and on again.
And thanks for the contribution! More people is really needed for bleep to reach it's potential :) |
Upstream moved, and some follow-up changes need
to be included as well.
Graal Native Image also needed some adjustments.
Important:
This PR depends on scala-cli/bloop-core#153.
I'm not entirely sure about the build changes. But there seem to be some dependencies missing for 2.12. Should support for Scala 2.12 be dropped maybe altogether?