Incorrect dependancy pulling

[GCHQDeveloper211]

With these example core files: osvvm.zip

It seems if you for example try to set your dependancy to be >= 2020.05, only 2021.10 will be pulled. (and not 2022.01 or 2022.02) Although if you change the version of the core file to be 2022.11 or anything such that the second minor number is greater than 10. It will assume that that core file is the next highest. I can see that it can see the higher versioned core files as if I say ==2022.01 it will pull from it directly.

[imphil]

Thanks for your report. I'm not quite sure I fully understand your problem. Can you show the core file in which you specify the dependency to osvvvm? And by "pulling", you mean the dependency is wrong, or the git provider is doing something wrong?

[GCHQDeveloper211]

Sorry. The dependancy is wrong. So our core file that depends on OSVVM is as follows:

CAPI=2:

name: :fpga-cores:tb-helpers:0.0.0

filesets:
  rtl:
    depend:
      - ">=::osvvm:2020.05"
    files:
      - source/packages/tb_helpers.vhd
    file_type: vhdlSource-2008
    logical_name: fpga_cores

  test:
    depend:
      - ">=::osvvm:2020.05"
    files:
      - source/test/test_tb_helpers.vhd
    file_type: vhdlSource-2008
    logical_name: fpga_cores

targets:
  default:
    filesets: [rtl]

  sim:
    default_tool: modelsim
    filesets: [rtl, test]
    toplevel:
      - fpga_cores.test_tb_helpers

And when running a fuses run --target sim. This will use the osvvm core 2021.10, as opposed to the expected 2022.02

[imphil]

Given your constraint (">=::osvvm:2020.05"), 2021.10 is a valid choice, isn't it? I think we had some confusion there in the past, are you expecting the newest available core to be used if multiple are valid solutions?

[GCHQDeveloper211]

I am expecting the newest core to be used, which would be 2022.02. Is that not the expected behaviour?

[GCHQDeveloper211]

Equally if I say (">=::osvvm:2022.01") it will still use 2021.10 as its dependancy (As long as that has been installed) otherwise it will not pull any dependancies.

[GCHQDeveloper963]

This behaviour is not due to anything that fusesoc is doing, but is instead caused by a dependency of simplesat, okonomiyaki, and is the expected behaviour within okonomiyaki. We can work around it by making a modification to our core files.

The issue is that the OSVVM version numbers can have a leading zero in the month field (i.e. "2022.01"); okonomiyaki treats leading zeroes as being invalid, raising an error. If this error is raised then the version is marked as worked around. Then in the actual comparison, if both of the versions being compared are worked around then the __le__ method attempts a comparison. If self is marked as worked around and the other is not then the method always returns True, otherwise False is always returned. Okonomiyaki actively discourages the use of years as version numbers, which is why leading zeroes are considered invalid.

I have put together some examples to help demonstrate this.

Where other is marked as worked around, False is returned

from okonomiyaki.versions import EnpkgVersion
ev1 = EnpkgVersion.from_string("2021.11-0")
ev2 = EnpkgVersion.from_string("2022.03-0")
print(ev1 < ev2)  # prints False

Where self is marked as worked around, True is returned

ev1 = EnpkgVersion.from_string("2021.01-0")
ev2 = EnpkgVersion.from_string("2022.13-0")
print(ev1 < ev2)  # prints True
ev2 = EnpkgVersion.from_string("2020.1-0")
print(ev1 < ev2)  # prints True

Where both self and other are marked as worked around, the parts of the version number that are not zero prefixed are compared.

ev1 = EnpkgVersion.from_string("2021.01-0")
ev2 = EnpkgVersion.from_string("2022.01-0")
print(ev1 < ev2)  # prints True
ev2 = EnpkgVersion.from_string("2020.01-0")
print(ev1 < ev2)  # prints False

We can work around this within our OSVVM core file by changing the core name to not have a leading zero on the month portion of the version number, e.g.

CAPI=2:

name: ::osvvm:2020.1
# ...
provider:
  name: git
  repo: https://github.com/OSVVM/OSVVM
  version: 2020.01

[imphil]

Thanks for the outstanding investigation @GCHQDeveloper963, that's much appreciated! The behavior you're seeing is unexpected (to me) -- version numbers like 2020.04 are certainly valid according to semver, and should be treated as equally valid by FuseSoC. I don't share the "precaution" in https://github.com/enthought/okonomiyaki/blob/51b8b4fa8d17255e13c097402691726545cf5b4c/okonomiyaki/versions/pep386.py#L76-L87. (Instead, FuseSoC explicitly references semver for its version numbering, see e.g. https://fusesoc.readthedocs.io/en/stable/user/build_system/dependencies.html#semantic-versioning-semver)

Since I'm short on time and you seem to have looked into this extensively @GCHQDeveloper963, do you have an idea for a PR on the FuseSoC side to solve this?

[olofk]

Thanks for the report and subsequent investigation. It's not an ideal situation to rely on okonomiyaki and simplesat since it was created for a different use case and just been shoehorned in. But since no one has the time to create a purpose-made dependency manager, it will have to do. With that said, it would be great if we can overcome these kind of unexpected problems. Given that the simplesat project (and potentially okonomiyaki) seems to be pretty much abandoned by upstream now, I think there's a case for bring them directly into the FuseSoC code base if we want to make modifications. Not sure that's the best approach but I just want to say I'm open to that if it is the easiest way forward.

[GCHQDeveloper963]

Given that simplesat and okonomiyaki seem to be abandoned, it seems sensible to move away from using them directly. I suppose there are three options:

1. Copy simplesat and okonomiyaki into the core fusesoc codebase. This is technically the simplest, but they are both licensed under a BSD 3-clause license while fusesoc is licensed under a BSD 2-clause license which from my (very much non-expert) reading would be incompatible.
2. Fork simplesat and okonomiyaki, leaving the license as BSD 3-clause and stating in the main license file that the code is copyright to Enthought except where otherwise stated and then mark all changes as copyright fusesoc contributors. The issue with this would be that currently the fusesoc dependencies are pulled in from PyPi and the existing packages will not be able to be updated to the forked version. I suppose it would be possible to publish new packages under a different name.
3. Stop using simplesat and okonomiyaki entirely, and just use a different sat solver. This would bypass the licensing and package manager issues, but would require more significant code changes (especially if a custom sat solver needs to be written).

Unfortunately I'm also short on time at the moment, so this isn't something that I'm able to commit to implementing right now.

[olofk]

Very much agree those three are all reasonable ways forward for different reasons. A few more things to note. Simplesat was always intended to be a temporary solution and it doesn't support all the things I really want, like local use flags (i.e. cores requiring certain flags being set or unset in their dependencies, a gentoo-derived feature) and the dependency error messages are completely unreadable. And for that reason, I have tried to keep it reasonably separated in the code with the hope that there shouldn't be too much work to plug in an alternative solution.

With that said, it has served well because there was no way I would had found the time to do a dependency manager from scratch and there's no way I'll be able to do it now without dedicated funding. Back when I brought this in initially, I was looking for different solutions but was surprised at how little I could find. This really was the most fitting one I found. Since Gentoo was a big inspiration for the dependency mechanism, I was originally hoping to perhaps reuse some code from Portage or pkgcore (especially the latter) but I never got anywhre trying to understand those code bases.

So, all in all, I think it's worth looking around at what alternatives that can be found in 2022. Perhaps the situation is much better and best case we can even find one that adds the missing features. If not, it would be great if someone could write a purpose-fit manager, but I think the chances for that are extremely slim so realistically the best thing then is to start modifying a local version simplesat and we'll check whether bringing it into the repo or keeping it separate is the best solution.