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

fix invalid dependencies #605

Merged
merged 2 commits into from
Sep 9, 2019
Merged

fix invalid dependencies #605

merged 2 commits into from
Sep 9, 2019

Conversation

pierrre
Copy link
Contributor

@pierrre pierrre commented Jul 10, 2019

Some dependencies have invalid date.

Fixes #581 #595

@CLAassistant
Copy link

CLAassistant commented Jul 10, 2019

CLA assistant check
All committers have signed the CLA.

@pierrre
Copy link
Contributor Author

pierrre commented Jul 10, 2019

I don't know why some vendors have been deleted.

@pierrre
Copy link
Contributor Author

pierrre commented Jul 10, 2019

Strangely, the vendored dependencies are not the same if I'm using Go 1.13beta or 1.12.x.
Now they're added correctly.

@pierrre
Copy link
Contributor Author

pierrre commented Jul 15, 2019

Is someone available to review this PR ?

@ww-daniel-mora
Copy link

I am also experiencing this issue

@Helcaraxan
Copy link
Contributor

Yes would be great if this could indeed be merged and a 1.17.2 patch release tagged (which would also contain the fix for #612 (i.e. #594).

FWIW. There are 2 other PRs (#583, #614) that address the same issue but I think this is the one that should be merged given it's the only one that actually updates go-critic and only go-critic to a recent commit.

@Helcaraxan Helcaraxan mentioned this pull request Jul 18, 2019
Copy link

@dajohi dajohi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this PR, i now get

$ env GOPROXY=direct GO111MODULE=on go build
go: github.com/golangci/errcheck@v0.0.0-20181003203344-ef45e06d44b6: invalid pseudo-version: does not match version-control timestamp (2018-12-23T08:41:20Z)

@pierrre
Copy link
Contributor Author

pierrre commented Jul 23, 2019

@dajohi which version of Go are you using ?
I'm using 1.13beta1, and it was working fine.
Can someone else confirm ?

@bcmills
Copy link

bcmills commented Jul 25, 2019

@pierrre, the version check is performed when downloading the module from version control. If an invalid version is already present in your module cache, it will continue to be available to builds using that cache.

To test this change, you'll probably want to start from a clean module cache (either run go clean -modcache first or set GOPATH=$(mktemp -d)) and set GOPROXY=direct.

@pierrre
Copy link
Contributor Author

pierrre commented Jul 26, 2019

Hi, sorry for the delay.
It should be fixed now.

Some dependencies have invalid date.

Fixes #581 #595
@pierrre
Copy link
Contributor Author

pierrre commented Jul 26, 2019

The build doesn't pass, but I don't understand the error.
Can someone help me ?

@pierrre
Copy link
Contributor Author

pierrre commented Jul 26, 2019

It seems that docs/demo.svg is generated differently.
But it's not related to my PR.
I guess that master is also broken 🤷‍♂️

@Helcaraxan
Copy link
Contributor

Helcaraxan commented Jul 26, 2019

Just checking: did you run go run scripts/gen_readme/main.go? I've just run it on master and there does not seem to be a diff which would imply it is not broken.

EDIT: also ran it on your branch and no diff.

CI seems to agree with me as well.

@pierrre
Copy link
Contributor Author

pierrre commented Jul 26, 2019

➜  golangci-lint git:(bug/fix-581) go run scripts/gen_readme/main.go
2019/07/26 10:35:10 Successfully generated README.md
➜  golangci-lint git:(bug/fix-581) git --no-pager diff

there is no diff on my computer

@Helcaraxan
Copy link
Contributor

Yes. Apologies. Edited my message above as well after I tested your branch myself. 👍

Having a look if I can figure out what's going on.

@Helcaraxan
Copy link
Contributor

Ah got it, although not the root cause. You can regenerate the file locally by running:

make -B docs/demo.svg

The -B forces to regenerate the thing despite make thinking it's up-to-date. And it actually generates a diff.

My guess: given this relies on npm black magic I suppose some thing got updated somewhere and induced this hidden change. The npm environment is not checked / understood by make and thus is not picked-up as "changed".

@pierrre
Copy link
Contributor Author

pierrre commented Jul 26, 2019

I'm sorry, but my PR is only about broken Go modules dependencies.
It wouldn't be correct to submit a change related to docs/demo.svg.

@Helcaraxan
Copy link
Contributor

I'll have a look if I can make a sensible PR to fix it separately. 👍

@Helcaraxan
Copy link
Contributor

@pierrre (and maintainers) proposed fix is up in PR #625. Once that one is merged we can rebase this PR on top of master and the CI should pass.

@dahu33
Copy link
Member

dahu33 commented Aug 28, 2019

Any update from maintainer?

@Helcaraxan
Copy link
Contributor

No. The maintainer has been inactive for 6 weeks now and there's no news available at the moment.

See #647.

@@ -1,11 +1,13 @@
module github.com/golangci/golangci-lint

go 1.11

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this PR specify go 1.11? I actively don't want this.

Copy link
Contributor

@Helcaraxan Helcaraxan Sep 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain what issue this raises for you?

This statement is not a hard requirement. It's merely an indication used by the go toolchain and you should not need to worry about it.

Tl;dr: This tells the Go compiler that if the build fails and you are using a < 1.11 version it will tell you it might be due to a too old Go version... however given that versions < 1.11 are not module aware they will not even bother / know how to interpret this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference see the Go 1.12 release notes.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding is that this specifier causes the files in the module to be built with an older version of Go?

As mentioned here: https://golang.org/doc/go1.12#modules

The go directive in a go.mod file now indicates the version of the language used by the files within that module.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually a bit different.

the version of the language used by the files within that module

refers to the version for which the code was written (as it might be using a new function introduced in that version... but not necessarily), not the one with which the files should be built. As the sentence following the one you quote indicates:

If the go directive for a module specifies a version newer than the toolchain in use, the go command will attempt to build the packages regardless, and will note the mismatch only if that build fails.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still feel that the writing is a bit ambiguous here, but I think I see what you're saying. I don't object if it's true that it's only a guideline. I just don't want tools like this to force-build on an old version, if that won't happen, I retract my objection :-)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will always have full control over your build process. There is no version enforcement going, I can guarantee you that. 🙂

I'll forward the point about the feeling of ambiguity around the phrasing. I can imagine myself in your place and understand it can introduce confusion for people not familiar with the relatively new syntax of go.mod files.

@jirfag jirfag merged commit cdeefb5 into golangci:master Sep 9, 2019
@jirfag
Copy link
Member

jirfag commented Sep 9, 2019

Guys sorry for the slow response. @pierrre thank you for the fix. I've merged it.
Some CI flaps, I will fix them later.

@pierrre
Copy link
Contributor Author

pierrre commented Sep 9, 2019

Thank you !

@diovann
Copy link

diovann commented Sep 22, 2019

Microsoft Windows [Version 6.2.9200]
(c) 2012 Microsoft Corporation. All rights reserved.

C:\Users\diovann>cd c:\go-work\HOME\core\

c:\go-work\HOME\core>go build
go: github.com/golangci/golangci-lint@v1.17.1 requires
github.com/go-critic/go-critic@v0.0.0-20181204210945-1df300866540: inval
id pseudo-version: does not match version-control timestamp (2019-05-26T07:48:19
Z)

c:\go-work\HOME\core>

how to fix this error??

@Helcaraxan
Copy link
Contributor

@diovann you need to upgrade your dependency on golangci-lint to v1.18.0. That version contains the fix.

@diovann
Copy link

diovann commented Sep 22, 2019

how to upgrade golangci-lint to v1.18.0 ??

@Helcaraxan
Copy link
Contributor

It depends.

  1. If you are simply trying to build golangci-lint, and supposing you have cloned it as a Git repository, in the same folder do git fetch && git checkout v0.18.0 followed by go build or go install.
  2. If golangci-lint is a dependency of your own project then go get -u github.com/golangci/golangci-lint@v1.18.0 && go mod tidy should do it.

NB: If it's someone else's project you can still do 2. but you will need to raise a pull request against the project in question, or you need to file an issue with the project for the maintainers to upgrade their dependency.

@diovann
Copy link

diovann commented Sep 22, 2019

@Helcaraxan thanks for this bro. !!

@tpounds tpounds added the bug Something isn't working label Oct 5, 2019
@ldez ldez added this to the v1.18 milestone Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to build golangci-lint v1.17.1 when enable GO111MODULE