search

tiiuae / sbomnix

A suite of utilities to help with software supply chain challenges on nix targets
125 stars 22 forks source link

feat: add darwin support #81

Closed olebedev closed 1 year ago

olebedev commented 1 year ago

Motivation

Add support for folks who run it on macOs.

henrirosten commented 1 year ago

Hello @olebedev.

This topic was earlier discussed in https://github.com/tiiuae/sbomnix/pull/58. Back then, @nikitawootten mentioned there was an issue with test_nixgraph_csv_graph_inverse on macOS. Is this no longer the case?

Other nitpicks from github actions checks: DCO: you should signoff the commit with git --signoff.

olebedev commented 1 year ago

Hi @henrirosten, thanks for having a look.

Back then, @nikitawootten mentioned there was an issue with test_nixgraph_csv_graph_inverse on macOS. Is this no longer the case?

I haven't noticed any problems with the change, I had successfully ran:

λ nix run github:olebedev/sbomnix/oleg/add-darwin-support#sbomnix  -- /nix/store/31xy014z9yng8zp5j3xk1f3q513b5dz5-wget-1.21.4 
WARNING  Command line argument '--meta' missing: SBOM will not include license information (see '--help' for more details)
INFO     Loading runtime dependencies referenced by '/nix/store/31xy014z9yng8zp5j3xk1f3q513b5dz5-wget-1.21.4'
INFO     Wrote: sbom.cdx.json
INFO     Wrote: sbom.spdx.json
INFO     Wrote: sbom.csv
olebedev commented 1 year ago

Hi @henrirosten, is there anything I can do to get it merged?

Cheers, Oleg

henrirosten commented 1 year ago

Hello @olebedev.

Can you make sure the tests pass on both x86_64-darwin and aarch64-darwin by running the make pre-push target on both? E.g.:

$ git clone https://github.com/tiiuae/sbomnix sbomnix_tmp
$ cd sbomnix_tmp
$ nix develop
$ make pre-push
olebedev commented 1 year ago

Getting an error with test_nixgraph_csv_graph_inverse:

>       assert Path(csv_out_inv).exists()

on aarch64-darwin. I haven't tried it on x86_64-darwin though.

henrirosten commented 1 year ago

@olebedev : sorry for the back and forth, I currently don't have an aarch64-darwin system I could test this myself.

I think commit https://github.com/tiiuae/sbomnix/commit/b5740aa85b8bfdb3436adc2d4cb0c86855138f90 should fix the issue you saw earlier.

Can you rebase your PR and manually re-run the tests locally to check if they now pass after https://github.com/tiiuae/sbomnix/commit/b5740aa85b8bfdb3436adc2d4cb0c86855138f90 on aarch64-darwin.

olebedev commented 1 year ago

Hi @henrirosten,

Yeah, it's now getting passed for the aarch64-darwin:

27 passed in 359.11s (0:05:59)

Thank you for the fix 👍

I haven't tried it for the x86_64-darwin because I have no anymore. Happy to remove it from the change. Let me know what is your preference.

henrirosten commented 1 year ago

@olebedev :

Yeah, it's now getting passed for the aarch64-darwin:

Thank you for the confirmation.

I haven't tried it for the x86_64-darwin because I have no anymore. Happy to remove it from the change. Let me know what is your preference.

Let's keep it for now, I'll try to test this on x86_64-darwin in the next few days.

henrirosten commented 1 year ago

I managed to test this on x86_64-darwin, and after https://github.com/tiiuae/sbomnix/pull/84, all the tests passed. We can now merge this change.