Fix multiple imports
asn1c: fix IMPORTS of same name from multiple modules
ASN.1 supports importing the same name from different modules as well as allowing that name to be in the current module - refer to ITU-T X.680 and https://stackoverflow.com/a/67910959.
Rework the IMPORTS tracking in imp from dict name -> module dict name -> list(module, ...), and resolve types that way.
Fixes #56
Thanks for this PR: this looks like an ambitious one.
I tested the changes against the entire set of ASN.1 specs, but the JSON code generator now breaks on module IETF_PKI_RFC5958. You can test the compilation of all modules with the cmd below:
python -m pycrate_asn1c.asnproc
I will need to investigate your changes further to understand the root cause of the issue. If you want to solve this on your side, here are the steps I'd follow when touching the ASN.1 compiler:
- re-compiling all the ASN.1 modules
- running the unit tests
You can otherwise set the globals TEST_ASN1C_ALL_COMP and TEST_ASN1C_ALL_LOAD to True in the main unit test file test_pycrate.py. This re-compiles all the ASN.1 modules and test their loading, each time the unit tests are run.
Apologies for not running python -m pycrate_asn1c.asnproc - I missed that step in the README for running all the tests.
I've updated the branch and made the following improvements:
- Regenerated pycrate_asn1dir/**.json per the change I made in commit 1939a8e to ensure deterministic node and link ordering.
- Extended my IMPORTS .asn tests to attempt to reproduce the problem. (Didn't hit the bug)
- Fixed JSONDepGraphGenerator to prevent the problem that the full recompile hit. (This was actually a TODO I'd added).
I can now successfully run all the tests on the branch:
python -m unittest test.test_pycrate
python ./test.py
python -m pycrate_asn1c.asnproc
@mitshell : if it's easier, I can break up this PR into two new PRs to make it easier to review the parts separately:
-
Change JSONDepGraphGenerator to order the nodes in the output, and the regeneration of the .JSON files. The reason for this commit is that prior to python 3.7 dict ordering isn't deterministic. From python 3.7, insert order is maintained, but it can be hard to "eyeball verify" output files when they're not sorted. Sorting the results from processing a dict ensures there's deterministic output across tests.
-
The import fix, which changes how
_imp_dict value from a single module name to a collection of modules. This was the core fix for #56, and resolves problems compiling and parsing some complex ASN.1 specs that have complex import dependencies.
Thoughts?
No worries, I'll take some time to review this PR as is: I don't have much time right now and I'll do my best. Thanks anyway for this PR, which loolks pretty nice.
So, everything runs fine. Thanks for your work.
Just a quick question: why use collections.defaultdict instead of a normal dict for storing the imports of an ASN.1 module? What avantage does it bring?
I converted mod['_imp_'] from a dict (key is the object name, value is module name) to a defaultdict(list) (with key still as object name, value changed to a list of strings (of N module names), and it's much easier to handle the missing key case when appending a module to a new object key.
I.e, with a defaultdict(list) can create or append a new entry for key obj_name with
module['_imp_'][obj_name].append(imp['name'])
without needing to first ensure that module['_imp_'][obj_name] exists and if not create an empty list.
Thanks for your work @lukem. Much appreciated.