nxp-mcuxpresso/spsdk

Support bincopy > 17.10 (issues with test_elftosb_mbi.py)

Closed this issue · 7 comments

dvzrv commented

Hi! I'm currently attempting to package this project for Arch Linux.
As we provide python-bincopy 17.10.1 it would be great for spsdk to be able to use bincopy >= 17.10.

I can currently only disable the tests that fail due to some incompatibility with the encoding of the test files (they seem not utf-8 encoded).

This seems to mainly occur in tests/elftosb/test_elftosb_mbi.py:

(excerpt)

/usr/lib/python3.10/site-packages/bincopy.py:1026:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <encodings.utf_8.IncrementalDecoder object at 0x6ac96b67fb20>
input = b'\x00\x00\x02 \x1d#\x00`\x99#\x00`\xd1G\x00`\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\...00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00r\x00w\x00d\x00\x00\x00\x00\xa4x\x1f'
final = True

    def decode(self, input, final=False):
        # decode input (taking the buffer into account)
        data = self.buffer + input
>       (result, consumed) = self._buffer_decode(data, self.errors, final)
E       UnicodeDecodeError: 'utf-8' codec can't decode byte 0x99 in position 8: invalid start byte

/usr/lib/python3.10/codecs.py:322: UnicodeDecodeError

During handling of the above exception, another exception occurred:

file_path = '/build/python-spsdk/src/spsdk-1.6.0/tests/mboot/blhost/data/iled_blinky_ide_1060_60002000.bin'

    def parse_image_file(file_path: str) -> List[SegmentInfo]:
        """Parse image.

        :param file_path: path, where the image is stored
        :raises SPSDKError: When elf/axf files are used
        :raises SPSDKError: When binary file is used
        :raises SPSDKError: When unsupported file is used
        :return: SegmentInfo object
        """
        with open(file_path, "rb") as f:
            data = f.read(4)
        if data == b"\x7fELF":
            raise SPSDKError("Elf file is not supported")
        try:
>           binfile = bincopy.BinFile(file_path)

Full build log:
python-spsdk-1.6.0-build.log

Although, we don't support (test!) Arch Linux, I see the benefit of migrating to bincopy 17.10.
Comparing to 17.9 they added some error handling related to issue you described.
So, with only small change on our end, we'll get the ELF support which we will use soon.
In relation to our other discussion in #35 I don't feel comfortable skipping the upper boundary, but rather setting it to <17.11 (allowing some head room for patches)

dvzrv commented

Thanks for the reply!

Although, we don't support (test!) Arch Linux, I see the benefit of migrating to bincopy 17.10.
Comparing to 17.9 they added some error handling related to issue you described.

In general these are things that can be tested in a CI pipeline (e.g. here on github) which attempts to build and test against the latest available versions of the respective dependencies or even on specific operating systems. This way it is easy to stay on top of things and fix regressions early (without the need to pin on minor version releases or specific versions).

So, with only small change on our end, we'll get the ELF support which we will use soon.

Would this be made available in a separate commit? For downstreams it is not at all feasible to wait for "big bang" releases (e.g. commit for 1.6.0 with tens of thousands of lines changed without even a commit message).

Without developing changes and features openly and incrementally no downstream will want to deal with integrating spsdk long term though, as it would require developing patches that you may have added already but which were just not yet released and not being able to even make out the reason behind a change because it is done in huge diff that is neither documented nor comprehensible at all. :S

Hi @dvzrv,
thanks for your suggestions. The project is developed internally and has to comply with company QA processes which currently prevents us from fully moving to GitHub. During the development and testing, there are tools and infrastructure which cannot be migrated to GH (for example system testing running on HW boards). It all allows us to ensure that the SPSDK is working properly and may be suitable for production.
As part of the project, we include a simple SW unit test (no need to use HW boards) which allows our users or potential contributors to check if their changes comply and we can run thorough QA before pushing them back into GH master. This ensures that new features will work on platforms the project support.
We also closely monitor the project and are happy for such feedback allowing us to move the project in the right direction and if possible to bring desired features as soon as possible even though as part of big-bang releases.

dvzrv commented

@vhamersky I would probably agree, but actions speak louder than words.

We also closely monitor the project and are happy for such feedback allowing us to move the project in the right direction and if possible to bring desired features as soon as possible even though as part of big-bang releases.

From what I can see, all of my proposed fixes (not related to this particular issue) have been ignored so far and now require further fixes. I am not getting paid to work on the integration of spsdk and do this in my free time. I am under the impression that external contribution is neither wanted nor valued.

Hi @dvzrv,
we are trying to address the items you brought and will definitely look at them in the next deliverables. Please be sure we are not ignoring them and we are happy for your feedback. Please be patient with us. Sorry for the complications.

dvzrv commented

we are trying to address the items you brought and will definitely look at them in the next deliverables.

Thanks. As there is not really a lot of transparency around what is actually being done/addressed where/when it is not really possible for external contributors to interact with this project though. Where there is no open development and knowledge sharing there can be no collaboration.

Meanwhile bincopy has been upgraded to 17.14 and I will continue to apply patches (now for myself).

Hi @dvzrv I have created PR to move the bincopy version to <17.15.