obudai opened a new pull-request against the project: `golist` that you are following: `` Fix not listing packages with external tests only ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` Do you have an example? ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
obudai commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` Running
``` golist --provided --tests --package-path github.com/osbuild/osbuild-composer ```
Without fix: ``` github.com/osbuild/osbuild-composer/internal/crypt github.com/osbuild/osbuild-composer/internal/distro github.com/osbuild/osbuild-composer/internal/pipeline ```
With fix: ``` github.com/osbuild/osbuild-composer/internal/crypt github.com/osbuild/osbuild-composer/internal/distro github.com/osbuild/osbuild-composer/internal/distro/fedora30 github.com/osbuild/osbuild-composer/internal/distro/rhel82 github.com/osbuild/osbuild-composer/internal/jobqueue github.com/osbuild/osbuild-composer/internal/pipeline github.com/osbuild/osbuild-composer/internal/weldr ``` ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
nim commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` That would wreck havoc on all package BRs since internal is a magic keyword that forbids exposing the result to other Go packages.
Returning anything internal in `golist` must be isolated with an '--internal' optional switch (and %gocheck should probably use this flag, once it is created. The R and BR part definitely should not) ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
obudai commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` But if I undrestand things correctly this is not connected with internal keyword in any way. It wouldn't list all packages even if they're not internal. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
nim commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` @obudai a lot of Go projects use internal in the package path to mean the same thing as the internal keyword. Welcome to the wonderful land of underspecified Go behaviour.
Therefore, to be safe, golist should filter anything with internal in the path, unless requested explicitly with a specific switch. (and upstreams that use internal to mean something else than internal should probably rename their Go package now that internal as a package name has been used as internal by many Go projects)
I know, it's clear as mud. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
jcajka commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` @nim TBH I don't think that we should be creating our own dialect of Go(even from tooling perspective). What you are suggesting is basically it. We should respect upstream specs of the Go with all its warts and possibly work with upstream to clean up or add things that make packaging hard for us.
I think that you shouldn't filter dependency list by default on some more or less arbitrary keyword(that is AFAIK not even mentioned in the guidelines). That doesn't mean that there shouldn't be feature that would filter them base on some keyword/regular expression. I can imagine that being useful in certain scenarios. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
nim commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` @jcajka I agree from a theoretical POW. I was just pointing out that from a practical POW, asking all the Go parts we package to adhere strictly to the specs, means dealing with a lot of build failures.
internal is known to be (mis)used by some projects. Do we want to ignore this (mis)use and just deal with failing builds whenever exposing internal makes things break? ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
jcajka commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` @nim I have assumed that this has been transparently handled in the new macros. I guess there should be some way whatever in the macros or document it in the guidelines that will more or less handle this transparently. Also I would expect that working with upstreams should be mentioned. I'm aware that this might pose fairly non-trivial feat.
Do you have list of packages that contain this in Fedora? ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
nim commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` @jcajka the macros do not add any policy over golist output. Any policy is manual use of override switches by packagers in their spec
As for upstreams… you realise that telling someone that named a package tree root internal that it's not internal for third party tooling is not going very far… As for actual examples, the cznic package set comes to mind
From a purely semantical POW, projects that use internal to mean something else than internal are the ones asking for trouble ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: ``
@obudai a lot of Go projects use internal in the package path to mean the same thing as the internal keyword. Welcome to the wonderful land of underspecified Go behaviour.
I don't understand where you get the idea that this is some sort of hack that non-standard-library packages are taking advantage of. The use of `internal` was described in a [design document](http://golang.org/s/go14internal) that has been in use since Go 1.4.
Therefore, to be safe, golist should filter anything with internal in the path, unless requested explicitly with a specific switch. (and upstreams that use internal to mean something else than internal should probably rename their Go package now that internal as a package name has been used as internal by many Go projects)
There is no other way for `internal` to be interpreted, so I don't know what you mean about other upstreams.
I do agree that if `golist` is specifying an internal path as something that goes in `Provides`, then that's a bug and it should be fixed.
I know, it's clear as mud.
It is clear; you're confused. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` Coming back to what this PR is actually about, there may be one more case of `TestGoFiles` that you haven't changed to add `XTestGoFiles` in `BuildArtifact`, though I'm not sure we use that for anything. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` This would affect approximately 200 packages. Since this determines which test files to run, it could have some possible consequences for the build, but probably not the final package.
I set up a [copr](https://copr.fedorainfracloud.org/coprs/qulogic/golist-pr27/) and more than half of those failed. _However_, many of them may already be failing, and, more importantly, the failures actually seem to be problems on copr. I will have to wait for it to be fixed to retry. ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` This breaks a few things, e.g., [ golang-github-pquerna-otp ](https://copr.fedorainfracloud.org/coprs/qulogic/golist-pr27/build/1217039/), [ golang-github-opencontainers-image-spec ](https://copr.fedorainfracloud.org/coprs/qulogic/golist-pr27/build/1217047/) or [ golang-github-onsi-gomega ](https://copr.fedorainfracloud.org/coprs/qulogic/golist-pr27/build/1217048/).
This is because imports from the newly added tests are not listed. You will need to add `XTestImports` alongside any `TestImports` usage to get this correct (then we need to update all these packages to match.) ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic commented on the pull-request: `Fix not listing packages with external tests only` that you are following: `` Oh, and of course, that's what the recently merged PR#26 does, so there's no need to modify this PR to do it too (unless you want to rebase.) ``
To reply, visit the link below or just reply to this email https://pagure.io/golist/pull-request/27
qulogic merged a pull-request against the project: `golist` that you are following.
Merged pull-request:
`` Fix not listing packages with external tests only ``
golang@lists.fedoraproject.org