bedrocklinux-userland icon indicating copy to clipboard operation
bedrocklinux-userland copied to clipboard

brp improvements

Open yshui opened this issue 9 years ago • 2 comments

Main contribution of this patch is:

  • Make use of chroot() for stratum path resolution
  • Rewrite the config parser

Plus a couple of other changes.

yshui avatar Jun 20 '16 01:06 yshui

@paradigm Is this useful for Poki? Should it be closed? Was this maybe addressed by the fork?

Mouvedia avatar Jun 06 '18 00:06 Mouvedia

All the development happens in github.com/bedrocklinux rather than my personal repo, and I check there for things like PRs. I completely missed this. I saw some of @yshui's other stuff and commented but then forgot about it as, again, I'm not in the habit of coming here. That's on me - I should have been watching this. It almost certainly notified me and I apparently just missed it.

Not only do I feel bad for not having been responded in terms of just being professional, but also because this is obviously a non-trivial amount of work (I didn't want to write a C config parser because it was too much work, and @yshui did so here), and even more so because this is very, very good stuff. The other big change here, using chroot(), is something I realized would be a good idea, independently, something like 18 months after this was made. Had I seen this a lot of time could have been saved. Poki might have been out by now.

I don't think we can use this as-is. Poki's equivalent has drifted too much; I did a clean re-write with some new ideas. I'm using chroot() already, and I moved the heavy lifting for configuration out of the C code to make it more flexible as I fiddle with finalizing the configuration changes. I think I could nab the use of unlikely() - that's not something I've been taking advantage of, but as @yshui demonstrates here it makes sense. I'll have to carefully review it to see if there's anything else.

I really, really hope I didn't discourage @yshui off from further contribution, as he/she was clearly ahead of me here and there's plenty of potential for further benefit. I'm greatly saddened I didn't watch github.com/paradigm more closely or that @yshui didn't reach out to any of the contact methods listed on the website that I do watch more actively.

@yshui If you're still interested in contributing there's definitely stuff we could use your help with. Let me know and I can try to catch you to where the project is now and highlight some areas that could use your touch. If you've lost interest in Bedrock in the last couple years, I don't blame you in the slightest.

@Mouvedia Thanks for bringing this to my attention. Wish you saw it two years ago.

I'll leave this open until either @yshui responds otherwise or I get time to review this in detail and see if there's other things like unlikely() that the current code base should adopt.

paradigm avatar Jun 06 '18 02:06 paradigm