kimgr / asn1ate

A Python library for translating ASN.1 into other forms.
Other
69 stars 41 forks source link

Add ASN1_SOURCE constant to output #63

Closed rtweeks closed 6 years ago

rtweeks commented 6 years ago

This will allow consumers of the generated Python code to provide the ASN.1 definition(s) programmatically to any connected software or documentation system.

kimgr commented 6 years ago

Thank you for contributing this!

I'm a little concerned about the size of the generated Python module. The ASN.1 specs I've used have been ~60-100K, so there's a risk this would swamp the generated pyasn1 code, which is what we'd really like to debug when things go wrong.

Can you sell me the benefits a little more? :)

I also don't fully understand the need for value vs list, can you explain?

Thanks!

rtweeks commented 6 years ago

The intended benefit is that, in addition to the pyasn1 code, the program consuming this generated Python will have programmatic access to the ASN.1 code from which the pyasn1 code was generated. This in turn allows the consuming program to provide the ASN.1 to any client (possibly written in a different language) that wants to consume these messages.

My particular use case is for converting JSON to an ASN.1 encoding to leverage ASN.1's DER so I can get a hash (SHA256) for mapping test cases that are encoded in JSON to database fixture data (where the database fixture data needs to be keyed by a distinctive string for each test case). My company has components written in several languages, and I'll want each of them to have the ASN.1 for my encoding so they can all generate the same hashed keys from the same JSON, though Python is the primary language for this use case.

A secondary benefit is that there is no question about which ASN.1 was used to generate the pyasn1 code -- it's the ASN.1 present in the same file. This means it is always possible to regenerate that code by regurgitating the ASN.1 and running it through asn1ate again.

If you want, I'd be happy to code an option flag to activate this functionality, having the default be to not generate these constants.

On the value vs. list front, I was just trying to minimize the changes to the existing code. When --split is given, it makes sense to me to put the ASN.1 for each module into the generated Python module. But without --split, I could either put the ASN.1 for all modules at the top, at the bottom, or split it up by module and store it in a list. My thinking was that splitting it up better allows the result to show which ASN.1 resulted in which pyasn1 code and allowed me, in the non---split case, to put the ASN.1 string after the comment identifying the module. But I also see the value of having all the ASN.1 together in a single string, which is why the list is also ''.join'ed into a single string at the end.

kimgr commented 6 years ago

I think I got the gist of this patch now. One thing that occurred to me: if you're willing to generalize the constant to always be a dict, the implementation should be much simpler at the expense of some complication in use. I have some other ideas for simplifying, so I'll hack up an alternative patch and post as a PR for you to review. Thanks!

kimgr commented 6 years ago

Hmm. I'm reminded that when people suggest additions to the command-line driver I often respond that it's probably easier for them to build their own that does what they want. I wonder if this is true in this case too...?

The command-line driver included is mostly intended as an example, the meat of pyasn1gen.py is the backend that translates from the ASN.1 sema model to Python, and that's importable and reusable.

That said, I've found and fixed a couple of bugs while looking at this, so I might as well try and finish it up and see how it comes out.

kimgr commented 6 years ago

Please take a look at https://github.com/kimgr/asn1ate/pull/65. I committed a series of refactors before this, to make it easier to bolt on, and I think it turned out simpler than your approach. Let me know on there what you think.

rtweeks commented 6 years ago

Closing this in preference to #65