efabless / openlane2

The next generation of OpenLane, rewritten from scratch with a modular architecture
https://openlane2.readthedocs.io/
Apache License 2.0
168 stars 30 forks source link

Rewrite code for parsing Tcl config files #473

Closed htfab closed 1 month ago

htfab commented 1 month ago

The current implementation for parsing Tcl config files fails to catch environment variables defined in subordinate .tcl files included from the main .tcl with the source command. This breaks openlane1 compatibility for almost every user design submitted to Tiny Tapeout.

Tcl config files are parsed using TclUtils._eval_env(). The current version

The last step is there to filter out entries inherited from the environment, but it also filters out entries defined in included files. This python-tcl interface relying on os.environ modifications and rewriting ::env to numbered variables also feels brittle in some other ways. I'm proposing to rewrite it in the following manner:

This makes the code more robust and only saves environment variables explicitly set by the Tcl config, making it unnecessary to use the problematic filtering step.

donn commented 1 month ago

@htfab Do you foresee any problems with us merging this to main?

htfab commented 1 month ago

I have run a few tests with the patch rebased to main, and it looks ok.

donn commented 1 month ago

Would you mind creating that PR? I would do it myself, but it would assign authorship of the PR to me which would be unfair.

htfab commented 1 month ago

Ok, created #474.