Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings from assembly files in golang.org/x/* #120

Open
drigz opened this issue Apr 8, 2022 · 8 comments
Open

Warnings from assembly files in golang.org/x/* #120

drigz opened this issue Apr 8, 2022 · 8 comments

Comments

@drigz
Copy link
Member

drigz commented Apr 8, 2022

When run on projects that refer to x/sys or x/crypto, go-licenses prints warnings like:

W0329 16:23:18.204626  221077 library.go:86] "[golang.org/x/sys/unix](http://golang.org/x/sys/unix)" contains non-Go code that can't be inspected for further dependencies:
/home/drigz/go/pkg/mod/[golang.org/x/[email protected]/unix/asm_linux_amd64.s](http://golang.org/x/[email protected]/unix/asm_linux_amd64.s)

It might print 10-20 warnings per run, which can be confusing the first time you see them, but the tool is operating as expected as these files don't introduce external dependencies (as far as I'm aware - I'm not sure how go mod would handle an external dependency from a non-go file).

This doesn't block use of the tool but it's nice to avoid printing warnings in the normal case, as it teaches people to ignore warnings.

I can imagine the following alternatives although I'm not sure how to choose between them:

  • Completely ignore OtherFiles: go mod will ignore these files so they are unable to pull in external dependencies. Instead, those projects should vendor the external dependencies and record this in their LICENSE file.
  • Ignore .[sS] files in OtherFiles, as assembly doesn't have a way of modelling external dependencies, whereas plausibly other files might.
  • Ignore a hardcoded list of .[sS] files from golang.org/x/*, after manually reviewing them to check that they don't have external dependencies.
  • Allow the user to ignore individual files with a similar approach to the --ignore flag in Add --ignore option #46. This would allow users to record in a script which files they have manually reviewed. This would be useful if we expect users to manually review these files, rather than ignoring the warnings.

I do not know how I would review the files listed in the "non-Go code" warning for external dependencies that should be listed. For example, asm_linux_amd64.s has no reference to an external repository (that I can see), but includes textflag.h, which has the same license header. Is this sufficient to conclude that the warning can be ignored? What is the user expected to do when they see this warning? Can it be automated?

@Bobgy
Copy link
Collaborator

Bobgy commented Apr 8, 2022

Good points raised for "ignorable warnings teaches people to ignore all warnings".

I think the main decision comes from -- "how does OtherFiles include extra dependencies into the built go binary?"

Ignore .[sS] files in OtherFiles, as assembly doesn't have a way of modelling external dependencies, whereas plausibly other files might.

I have never worked with Go + assembly files, but if this is true, we can safely ignore .[sS] files then.

@drigz
Copy link
Member Author

drigz commented Apr 8, 2022

I too am unsure, so I sent a question to https://groups.google.com/g/golang-nuts (I guess it's waiting to get through moderation at the moment).

@Bobgy Bobgy assigned drigz and unassigned drigz Apr 10, 2022
@drigz
Copy link
Member Author

drigz commented Apr 20, 2022

I got an answer to this on an intranet message board:

If you are building using go build, then an assembly file can't introduce additional dependencies.

In general, as you suggest, the go tool will only pull in new dependencies based on the contents of Go files. The go tool is aware of non-Go files, and it will build them, but it will never use them to identify dependencies.

That said it's perhaps worth noting that although the go tool won't pull in dependencies because of a C file, a C file can of course #include and link against libraries that are already installed on your system. The go tool won't bring them in, but if they are already there then it is possible that it will build a binary that uses them.

I would expect that for .s files we can skip the warning. For other suffixes including .S, the C preprocessor might include otherwise-licensed files from the host filesystem, so we could like to a README entry that describes this. How does that sound?

@Bobgy
Copy link
Collaborator

Bobgy commented Apr 20, 2022

That sounds good!
Ideally, we can link to a public go documentation for this, but I think it's optional.

@Bobgy
Copy link
Collaborator

Bobgy commented Apr 20, 2022

So I guess we can avoid the warning when .s files are detected.

@yusufozturk
Copy link

Is there any workaround for this? Is it possible to ignore .s files with current release?

Thanks.

@drigz
Copy link
Member Author

drigz commented Oct 10, 2022

I'm afraid not, I never got around to implementing the suggested approach :(

@iwahbe
Copy link

iwahbe commented Oct 30, 2023

Has there been any work done here?

oxzi added a commit to Icinga/icinga-notifications that referenced this issue Dec 6, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
oxzi added a commit to Icinga/icinga-notifications that referenced this issue Dec 6, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
oxzi added a commit to Icinga/icinga-notifications that referenced this issue Dec 6, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
oxzi added a commit to Icinga/icinga-notifications that referenced this issue Dec 7, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
oxzi added a commit to Icinga/icinga-notifications that referenced this issue Dec 7, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
julianbrost pushed a commit to Icinga/icinga-notifications that referenced this issue Dec 7, 2023
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
Al2Klimov pushed a commit to Icinga/icinga-kubernetes that referenced this issue Jan 16, 2024
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

Closes #139.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/
Al2Klimov pushed a commit to Icinga/icinga-kubernetes that referenced this issue Jan 16, 2024
By utilizing the neat go-licenses[0] tool, scanning the cached Go
dependencies against an allow list of licenses, which is currently
leaned from Icinga DB, works quite like a charm.

This, however, only includes Go code and produces warnings for
(transitive) included Go Assembly code[1]. If we are planning to include
other non-Go artefacts in the future, those also might need to be
identified - REUSE[2] might help there.

[0] https://github.com/google/go-licenses
[1] google/go-licenses#120
[2] https://reuse.software/

(cherry picked from commit 10d5b1b09f6ab81734a24c5a32bd63a1138750c0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants