search

olofk / fusesoc

Package manager and build abstraction tool for FPGA/ASIC development
BSD 2-Clause "Simplified" License
1.19k stars 245 forks source link

File names get corrupted by mutiple calls to _get_fileset_files in edatool #142

Closed benreynwar closed 7 years ago

benreynwar commented 7 years ago

On line 152 in edatool.py the file names are updated to be relative to the base path. If _get_fileset_files is called twice then this update is done twice so the file paths become incorrect. Presumably normally this only gets called once, but I wanted to be able to retrieve both the synth and sim filesets.

https://github.com/olofk/fusesoc/blob/8fcb357aa3fc7a86ab2e14487583f83c9ebb1ae4/fusesoc/edatool.py#L152

olofk commented 7 years ago

I assume this is related to https://github.com/benreynwar/fusesoc/commit/6b5594ee0f97a4b800224edddbf1acca69575827#diff-8e599b63168306ba1aae17cbefaa5229R28 .In that case I think you need to create a variant of the function. It was mostly meant as a helper function, and backends are free to not use it.

I briefly thought about letting _get_fileset_files return two objects, (one for sim and one for synth), but unfortunately it's a bit more complex than that, as some files are only valid for certain tools, such as xci files for vivado or qsys files for quartus. I'm also working on further changes to allow support for use flags, which will further complicate this. So I think the best idea right now is to make a copy of _get_fileset_files with the behaviour you're looking for

olofk commented 7 years ago

ah ok. I see now that it would perhaps be a cleaner solution to just return new objects instead of changing file.name in the existing objects

olofk commented 7 years ago

Call it karma, but today some of the unit tests failed because of corruptions in the paths caused by _get_fileset_files. This gave me a good reason to fix the bug you reported. Please verify that _get_fileset_files can be called multiple times now with https://github.com/olofk/fusesoc/commit/98da223de4fa9342549b589371bbefb215e07838 applied

benreynwar commented 7 years ago

Ha! Yes, that fixed the issue I was having. Thanks!