https://pantsbuild.org/ logo
w

witty-crayon-22786

08/27/2021, 5:19 PM
@happy-kitchen-89482: re: the config parsing changes: it would be really great at some point soon to migrate to consuming the rust code for this
because it’s possible that we could switch bootstrap options parsing entirely to rust as an incremental step before Subsystems are supported… and that might not be far off
👍 1
h

happy-kitchen-89482

08/27/2021, 5:21 PM
That would be great for the native client as well
w

witty-crayon-22786

08/27/2021, 5:21 PM
right
h

happy-kitchen-89482

08/27/2021, 5:21 PM
To have there be just one mechanism
w

witty-crayon-22786

08/27/2021, 5:21 PM
right.
to be clear, i have not actually triaged how much work it would be to switch bootstrap over… but i could imagine that switching over just Config might be an even smaller incremental step before that.
h

hundreds-father-404

08/27/2021, 6:10 PM
Oh that project sounds fun! I'd love to do it if others don't get to it first. (ik John laid a lot of ground work)
e

enough-analyst-54434

08/27/2021, 6:41 PM
The main work of switching over has ~0 to do with toml - is implementing map parsing. Currently just our list parsing is implemented: https://github.com/pantsbuild/pants/blob/main/src/rust/engine/client/src/options/parse.rs
w

witty-crayon-22786

08/27/2021, 6:44 PM
ah, got it.
… well. hm. there is no map parsing currently, is there? i think that that is what they are proposing to add
python dicts are currently embedded as strings, iirc
e

enough-analyst-54434

08/27/2021, 6:45 PM
And so how does rust read those?
So - there you go.
w

witty-crayon-22786

08/27/2021, 6:46 PM
yea. assuming an incremental migration though, the Python-side
Config
class could probably consume the existing facility and then parse
e

enough-analyst-54434

08/27/2021, 6:47 PM
Ah - ok. I find getting that piecemeal can be more effort than just doing it. But however someone wants to parcel this up that's not me - sounds fine.
w

witty-crayon-22786

08/27/2021, 6:47 PM
… and then switch to toml-native maps in one place rather than two
e

enough-analyst-54434

08/27/2021, 6:51 PM
I thought I remembered more. Its small, but + is special for dicts. No - apparently though.
w

witty-crayon-22786

09/08/2021, 3:59 PM
made some progress on this over the weekend. might have something i can post next weekend