Report when manifests contain ineffectual constraints

[goller]

Hey there... I may have found an issue regarding ensure at a revision using master.

After running a dep ensure the manifest revision changes but the lock does not.

Here is how to reproduce

$ go get github.com/influxdata/kapacitor

$ echo 'package main

import "github.com/influxdata/kapacitor/tick/ast"

func main() {
	ast.Parse("stream|from()|alert().slack()")
}' > main.go

$ dep init
Cached github.com/influxdata/kapacitor

$ dep ensure github.com/influxdata/influxdb@e4628bb69266dbd624dc27d674b52705ce0dcbf2

manifest.json is:

{
    "dependencies": {
        "github.com/influxdata/influxdb": {
            "revision": "e4628bb69266dbd624dc27d674b52705ce0dcbf2"
        },
        "github.com/influxdata/kapacitor": {
            "branch": "master"
        }
    }
}

and lock.json is:

{
    "memo": "b9eaf92e4ba015df09c7113e8a3a11eabfaf54feaa1d8814f6441ee34b892d29",
    "projects": [
        {
            "name": "github.com/gogo/protobuf",
            "version": "v0.3",
            "revision": "909568be09de550ed094403c2bf8a261b5bb730a",
            "packages": [
                "proto"
            ]
        },
        {
            "name": "github.com/influxdata/influxdb",
            "version": "v1.2.0",
            "revision": "d40a9df8057f5f4124f5709cb8238591e2abcb8e",
            "packages": [
                "influxql",
                "influxql/internal",
                "influxql/neldermead",
                "models",
                "pkg/escape"
            ]
        },
        {
            "name": "github.com/influxdata/kapacitor",
            "branch": "master",
            "revision": "73b05c5e24439f19663ffc8f95582352309db79e",
            "packages": [
                "tick/ast"
            ]
        },
        {
            "name": "go.uber.org/atomic",
            "version": "v1.0.0",
            "revision": "0c9e689d64f004564b79d9a663634756df322902",
            "packages": [
                "."
            ]
        },
        {
            "name": "go.uber.org/zap",
            "version": "v1.0.0-rc.3",
            "revision": "4e517d38b2138e59475d6468e7a6b626eecdaa66",
            "packages": [
                ".",
                "buffer",
                "internal/bufferpool",
                "internal/color",
                "internal/exit",
                "internal/multierror",
                "zapcore"
            ]
        }
    ]
}

I was expecting the revision of github.com/influxdata/influxdb in lock.json to change to e4628bb69266dbd624dc27d674b52705ce0dcbf2.

[sdboyer]

Ahh, that was the issue.

Constraints specified in manifest.json only apply to your direct dependencies. That's by design. The problem right now is that a) we don't tell you that your constraints are ineffectual and b) dep ensure is still writing to the manifest.

I'll keep this open (but retitled) as a reminder that we need better feedback about this.

[goller]

@sdboyer Right on. I worked around this by having my desired version checked out into my go path before running dep init.

[mikegleasonjr]

I hand edited my lock.json for one of my indirect dependency, removed my ./vendor and re-ran dep ensure

[mikijov]

I will take a look at this.

[sdboyer]

@mikijov awesome! a quick pointer - there's already logic in gps that computes ineffectual constraints in more or less the way discussed here - we need to exclude such constraints from input hashing. That logic is in rootdata. getApplicableConstraints():

dep/internal/gps/rootdata.go

Line 82 in 18ebb09

func (rd rootdata) getApplicableConstraints(stdLibFn func(string) bool) []workingConstraint {

We don't want to surface that whole struct, or that method directly, as it's deeply enmeshed in gps' internal systems. However, it may be worth surfacing a standalone func that does the work, moving the body of getApplicableConstraints() into that func, and then just calling it from the method.