[Solved] 'moduledefs' broken

[hippy]

There is something broken or funky with 'moduledefs'. I am currently using 'v1.19.1-18' but it may be an earlier issue I hadn't noticed before.

I have been adding my own C Extension Modules to my clone of the 'rp2' build and everything seemed to be working fine. I added another and I noticed something had gone wrong; 'import' reported no such module, 'help("modules")' showed it wasn't there but all my other modules were.

After hours of banging my head against the wall I eventually managed to get it to appear, same code I had which did not appear now present and working as expected.

Then, with no changes to my source code, I did a clean build and it was gone again

During hours of trying to figure things out it seemed to me everything should be working but it just wasn't appearing as a module, and 'moduledefs' started to emerge as a culprit.

Having suspected that, I wrote a verification program to check things were how they should be when it had worked. I could then run that to see what was different when things stopped working. When it works, for my "core1" module, these files exist with the contents as shown below ...

genhdr/module/....__own__modcore1.c.module
MP_REGISTER_MODULE(MP_QSTR_core1, mp_module_core1);

genhdr/moduledefs.collected
MP_REGISTER_MODULE(MP_QSTR_core1, mp_module_core1);

genhdr/moduledefs.h
extern const struct _mp_obj_module_t mp_module_core1;
#undef MODULE_DEF_MP_QSTR_CORE1
#define MODULE_DEF_MP_QSTR_CORE1 { MP_ROM_QSTR(MP_QSTR_core1), MP_ROM_PTR(&mp_module_core1) },
MODULE_DEF_MP_QSTR_CORE1 \

When it doesn't work that 'genhdr/module/....__own__modcore1.c.module' file isn't created, and the expected entries in 'genhdr/moduledefs.collected' and 'genhdr/moduledefs.h' are absent.

So it seems to me whatever handles 'moduledefs' is sometimes including the module, but, for me, most times not. I have no idea why it seems to affect this single module but not others. It's not the "core1" name because I see the same with that changed. Everything for 'qstrs' and 'qstrdefs' seems fine, 'MP_QSTR_core1" is appearing there even when the module isn't appearing as it should.

My gut feeling is it relates to "module registrations not updated" when it perhaps should have been but I don't know how things are meant to work so that may be wrong. And, for a clean build it should be creating everything but that's when it seems to lose the module.

Anyone have any ideas on how to figure out what's going wonky ?

I am not able to post an issue to the github or contribute there, so reporting it here is the best I can do.

[hippy]

Digging further the oddness is this one particular file and it not being identified in 'genhdr/moduledefs.collected' seems to be the issue. Manually inject its reference into that and it gets included as expected in the build output.

So why is it not appearing in 'genhdr/moduledefs.collected' and why no 'genhdr/module/....modcore.c.module' file ?

Especially when it is appearing in 'genhdr/qstrdefs.collected.h' and 'genhdr/qstrs/....modcore.c.qstrs' exists.

Time to look at what generates 'genhdr/moduledefs.collected' I guess - 'py/makeqstrdefs.py cat module _ ....' presumably - to see why it's not passing it on or why it's not present or not detected in whatever that uses as input.

[hippy]

This seems to have been the culprit -

Code: Select all

MP_REGISTER_MODULE(MP_QSTR_core1, mp_module_core1)

With that corrected things seem to be working now so onwards and upwards.

[jimmo]

With that corrected things seem to be working now so onwards and upwards.

So what was the actual "correction" that solved it?

[hippy]

So what was the actual "correction" that solved it?

Sorry - It seems I plain forgot to type that bit

Basically a missing semicolon at the end of the definition which should have been -

Code: Select all

MP_REGISTER_MODULE(MP_QSTR_core1, mp_module_core1);

It compiles without error whether it is there or not but, if it's not included, it's not determined to be an actual registration, gets silently dropped in the build.

That's the "why it's not present or not detected in whatever that uses as input" I suspected earlier. I determined the registration was present in the data used, 'qstrs.i.last' so it then fell to why it was being ignored, not detected as a registration. I happened to notice the lack of semicolon and that adding it fixed things so my attention then turned to why that was. That turns out to be -

https://github.com/micropython/micropyt ... efs.py#L92

Code: Select all

re_match = re.compile(r"MP_REGISTER_MODULE\(.*?,\s*.*?\);")

That's what demands the semicolon must be present. The subsequent matching never succeeding if it's missing.

I am not sure what the solution should be. Simply removing the semicolon still has it not match, silently ignoring things like -

Code: Select all

MP_REGISTER_MODULE(MP_QSTR_core1,
                   mp_module_core1);

If I were looking for a pragmatic solution I might match on just "MP_REGISTER_MODULE", see if the full match works, and report an error if not.

Of course, if I'd known where the issue was it would have been obvious why my module registration was being ignored and what my mistake had been, but not being familiar with the processing steps, order, code or files used, I had to chase effect to cause.

I wasn't entirely correct about 'clean build and it disappeared'; it was a bit more complicated than that. My module is actually generated and in trying to figure out what was going on I generated a cut-down module cloned from one which worked and that did work. I eventually ended up with two things generating my module and it was luck of the draw which ran last which gave me two different files, one working, one not. Spotting that mistake led me to note the semicolon difference which was key to identifying the solution.

[jimmo]

If I were looking for a pragmatic solution I might match on just "MP_REGISTER_MODULE", see if the full match works, and report an error if not.

Yes, I think something like this is likely to be a good improvement. Similar to what was done in https://github.com/micropython/micropyt ... dfe8b676e9

This thread is a good reminder that I need to update https://docs.micropython.org/en/latest/ ... /qstr.html -- I initially wrote that before string compression and now module registration became part of the same process. Also the slight differences between the CMake and Makefile variants of this process are worth describing.

[hippy]

Thanks. I have marked as "solved" on the basis it will be.

As to documentation - I found it a little too detailed, think it could benefit from a more user friendly intro, showing what the high level steps are and what they create and use, for people interested in the process but not so much the actual details. Something like, E&OE ...

Code: Select all

#   makeqstrdefs.py   pp                Determine QSTR and moduledefs
#   makeqstrdefs.py   split qstr        Create qstr.split
#   makeqstrdefs.py   cat qstr _        Create qstrdefs.collected.h
#   makeqstrdata.py                     Create qstrdefs.preprocessed.h
#                                       Create qstrdefs.generated.h
#   makeqstrdefs.py   split module      Create moduledefs.split
#   makeqstrdefs.py   cat module _      Create moduledefs.collected
#   makemoduledefs.py                   Create moduledefs.h

Appreciating that it's a lot easier to casually say what there should be than having to provide it