zigmod icon indicating copy to clipboard operation
zigmod copied to clipboard

make deps.zig compatible with gyro

Open mattnite opened this issue 2 years ago • 4 comments

This patch adds an addAllTo() function under the pkgs namespace, this is so that package maintainers can more easily use both gyro and zigmod. It only adds zig packages to a LibExeObjStep, so it is functionally equivalent to gyro as well.

looks like zig fmt picked up some other fixes along the way.

mattnite avatar Jan 30 '22 10:01 mattnite

why not have users do if (@hasDecl(deps, "addAllTo") deps.addAllTo(exe) else deps.pkgs.addAllTo(exe); or polyfill gyro to move addAllTo up into the root namespace given that if the purpose of this was to avoid the if statement, users should prefer the more featureful function?

nektro avatar Jan 31 '22 06:01 nektro

my thinking was that:

  • deps.addAllTo: add all dependencies
  • deps.pkgs.addAllTo: add all packages

and so with a reduced scope you get reduced functionality since zigmod's root addAllTo does more than just add packages.

mattnite avatar Feb 02 '22 02:02 mattnite

right and why would you only want to add packages when dependencies depend on the other things also being added?

nektro avatar Feb 02 '22 10:02 nektro

It’s for those who only depend on pure zig packages, they can explicitly reduce the scope. It’s better this way because the same function with the same scope has the same behaviour, where if I were to polyfill gyro, the implementation would be the different because it doesn’t deal with C libraries the same way.

Users who aren’t aware of the differences could end up with link errors if I polyfill gyro, and that’s far more unsettling than a compiler error if you’re not used to this stuff. What I could do is generate a root declaration of addAllTo that results in a nice compiler error that gives context.

mattnite avatar Feb 03 '22 08:02 mattnite