text:
Improve code flow in ScriptServer::init_languages()

This PR improves code flow in ScriptServer::init_languages() by removing two unnecessary scopes.
The execution order has changed, languages are now initialized before global script classes are added.
I tested this change by creating a scene with two nodes, one with a C# script and the other with a GDScript script.
This did not generate any error, warning or unintended behaviour on latest commit of master branch 040f49ed6e.
The values of _language_count and _languages are unaffected by this change, and the only other side-effect apart from the initialization of the languages is the function call add_global_class() at script_language.cpp:203, which only affects the variable ScriptServer::global_classes. Neither C# or GDScript's init() methods access any ScriptServer variables or methods.

Can you explain what you mean by improving the code flow?

The original function had the first section scoped in for no good reason. No variable was declared in this scope directly, and it did not improve readability.
This same section was also scoped in an if statement with no else or return.
This can be rewritten as I did to discard a scope and to increase readability.
There is no technical issue with adding the global variables before initializing the languages, it is purely a matter of style.

If we do change the order, should the order also be changed in finish_languages to clear them before finishing the languages?

This is not necessary. Reversing the order does not change the behaviour of finish_languages.

This code change is more of a "stylistic change" since it has no meaningful impact at runtime and changes the behavior of the application which could break stuff for users. We should avoid changing things without a clear benefit. I recommend this PR be closed.

This code change is more of a "stylistic change" since it has no meaningful impact at runtime and changes the behavior of the application which could break stuff for users. We should avoid changing things without a clear benefit. I recommend this PR be closed.

Even if this behavioural change is truly undesired, the pointless scope from line 187 to 200 should still be removed considering the reasoning above.

authors: RolandMarchand gamedevsam


text:
Fix scroll grabber can't reach ends in default theme

All scrollbars in the screenshots are showing their min/max value:

Before
After

Interesting. It looks like this also fixes my (just created) issue here: https://github.com/godotengine/godot/issues/70866

Thanks!

source: gharchive/pull-request

authors: Maran23 akien-mga timothyqiu

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Change .gitignore's bin/ to bin

Allows bin/ to be ignored even when it's a symlink.

Thanks!

authors: akien-mga myaaaaaaaaa


text:
[3.x] Improve touchpad and mouse support for the Android editor

Fix issues with using a touchpad to click, drag, interact with the navigation controls, etc..
Fix issues with panning with 2+ fingers
Fix issues with using double-tap to magnify on the spatial editor

main version

Have been testing by building Android editor, it does seem the 3D controls are better after the PR. In particular the controls were getting stuck in a movement before even though I wasn't touching the 3D controls, this seems fixed after the PR.
I'm seeing some kind of graphical corruption in what appears to be the 3D grid, but that's a separate issue unrelated to the input.

Thanks!

authors: akien-mga lawnjelly m4gr3d


text:
doc: Clarify description of SceneTree.create_timer()

Added the 'emit' and the 'signal' word to the 'create_timer()' method description, regarding the timer behavior.

Force pushed to do the suggested changes and make the commit message more explicit.


text:
Add export option for the Web to ignore DevTools shortcuts

This PR adds an option for the Web build in order to ignore common shortcuts for summoning web developer tools.
If set to true, the Godot Javascript bindings will simply omit to send F12, Ctrl-Shift-I and Meta-Shift-I to Godot.

Fixes #86175

Closing PR. Just realized that the ideal way to go forward with this would be for Godot to realize that an input was unhandled, and then make the browser process the input as usual.
I realized that Godot games interrupt everything, even shortcuts to quit the browser and such.


text:
MFI gamepad support for iPhone

This is a straight port of the work I did in Godot 2.x with a few things changed due to the renaming of things in Godot 3.0
I haven't been able to test this on Godot 3.0 as the iPhone build is still having some issues but I think it should work.
As Godot 3.0 has iOS 9 as a minimum I have enabled MicroGamepad support here. Hopefully Travis will be happy with that.

@akien-mga Hmm, travis is again complaining about MicroGamePad being unknown. I can disable it again but this time I'm wondering whether travis is compiling against older iOS frameworks?

(just returned to using spaces and disabled microgamepad for now just to make travis happy until we set this up right)


text:
Force editor progress dialog size update.

Fixes https://github.com/godotengine/godot/issues/92828

Tested, it works

Is the Main::iteration() (2 loops) call prevents the loading from happening? I'm curious.

Is the Main::iteration() (2 loops) call prevents the loading from happening? I'm curious.

Two loops do to fix it, some value might do (10 iterations seems to), but it's better to just update rect directly.

Thanks!

authors: adamscott akien-mga bruvzg passivestar


text:
🚀 [Feature]: My (simple) SSEFiber Package (for featuring on extensions page)

Feature Description
A very simple package to add SSE support for fiber
https://github.com/jsorb84/ssefiber
https://pkg.go.dev/github.com/jsorb84/ssefiber
Additional Context (optional)
No response
Code Snippet (optional)
// Example of usage, see package links for more info

package main

import (
"github.com/gofiber"
"github.com/jsorb84/ssefiber"
)

func main() {
  app := fiber.New(...)
  sse := ssefiber.New(app, "/sse")
  channelOne := sse.CreateChannel("Channel One", "/one") // /sse/one
  channelOne.SendEvent("name", "data")
}

Checklist:

[X] I agree to follow Fiber's Code of Conduct.
[X] I have checked for existing issues that describe my suggestion prior to opening this one.
[X] I understand that improperly formatted feature requests may be closed without explanation.

@jsorb84 Can you submit a PR to list your middleware on this repo?

source: gharchive/issue

repo: gofiber/awesome-fiber


text:
[EN]: Update golangci-lint to Version v1.57.2

Description:

Update golangci-lint version from v1.55.2 to v1.57.2

Additional Information:

Remove all deprecated linter flags

WARN [config_reader] The configuration option linters.govet.check-shadowing is deprecated. enable shadow instead.
WARN [config_reader] The configuration option linters.gci.local-prefixes is deprecated, using prefix() inside linters.gci.sections.
WARN [config_reader] The configuration option linters.gomnd.settings is deprecated. using the options linters.gomnd.checks,linters.gomnd.ignored-numbers,linters.gomnd.ignored-files,linters.gomnd.ignored-functions.

Checklist:

[x] I have formatted my code using  goimport  and  golangci-lint.
[x] All new code is covered by unit tests.
[x] This PR does not decrease the overall code coverage.
[x] I have reviewed the code comments and documentation for clarity.

Thank you for your contribution!

@surajitpore0 can you fix the linters - we can merge after that!

repo: gofr-dev/gofr


text:
go build打包后执行打包文件提示找不到配置文件

首先已确认config下是有配置文件的，本地用的是gfcli 1.16.4  ，gf版本1.16.6

已处理

repo: gogf/gf


text:
Ability to pull request between "unrelated" repos

If I have two independently created repositories with similar content, I currently need to do something of a dance to allow a pull request to be made between them (i.e. delete my repository, fork remote repository, push my content to new repository, pull request).
It would be nice if the pull request page would allow instead to specify an arbitrary repository to pull request against, so these types of merges could be done with less friction.

What would such "workarounds" be?

Any updates on possible workarounds?

authors: cicuz kostrykin wrouesnel

repo: gogits/gogs


text:
custom conf file doesn't work

We DO NOT take questions or config/deploy problems on GitHub, please use our forum: https://discuss.gogs.io
Please take a moment to search that an issue doesn't already exist.
For bug reports, please give the relevant info:

Gogs version (or commit ref): 0.9.25.0506
Git version: 1.7.1
Operating system:CentOS 6.2
Database: MySQL
Can you reproduce the bug at http://try.gogs.io:
Description

...
遵照官方文档说明，在/opt/gogs/custom/conf/下创建app.ini文件修改自定义配置，重启了gogs服务，这些配置没有生效。查看issue，好像已经改了读取自定义配置文件的方式是吗？改成/etc/gogs/conf/app.ini？至少我修改这里的文件是生效的，修改/etc/gogs/custom/conf/app.ini也不行。

There is no fixed place to put your custom app.ini, it is based on your installation.
It is defaulted to path/to/gogs/custom/conf/app.ini.

用yum安装的，一开始纠结了很久，直到看到你回复其他人包管理器安装的自定义配置文件路径是/etc/gogs/conf/appi.ini

I installed gogs with yum too. And https://gogs.io/docs/advanced/configuration_cheat_sheet  confuses me.

建议文档里面写下yum安装后的配置文件路径 @Unknwon

authors: Unknwon grunmin kinglionsoft


text:
add support for file encryption/decryption

Apart from raw text, allow user to encrypt/decrypt files

0e0635b29f7b5a9139d7270d96d0aa9d16b24c54

repo: gogopavl/kesar-cli


text:
Can't index .Site.Author with Page.Params.author

Doing {{ index .Site.Author .Params.author }} causes an error:
execute of template failed at <index site.Author .Params.author>: error calling index: value is nil; should be of type string

I checked, and no value is nil. See below. Indexing with an identical string literal does work.
config.yaml:
author:
  george_bluth:
    display_name: George Bluth
  michael_bluth:
    display_name: Michael Bluth

Front matter:
+++
author = "michael_bluth"
foo = "michael_bluth"

Template:

.Site.Author: {{ .Site.Author }}

printf "%T" .Site.Author: {{ printf "%T" .Site.Author }}

.Params.author: {{ .Params.author }}

printf "%T" .Params.author: {{ printf "%T" .Params.author }}

eq .Params.author "michael_bluth": {{ eq .Params.author "michael_bluth" }}

index .Site.Author "michael_bluth": {{ index .Site.Author "michael_bluth" }}

Output:
.Site.Author: map[george_bluth:map[display_name:George Bluth] michael_bluth:map[display_name:Michael Bluth]]

printf "%T" .Site.Author: map[string]interface {}

.Params.author: michael_bluth

printf "%T" .Params.author: string

eq .Params.author "michael_bluth": true

index .Site.Author "michael_bluth": map[display_name:Michael Bluth]

Note that {{ index .Site.Author .Params.foo }} also results in the same error.
Sorry in advance if I'm missing something obvious, but after many minutes of debugging, I just can't see it.
What version of Hugo are you using (hugo version)?

$ hugo version
hugo v0.108.0+extended darwin/amd64 BuildDate=unknown

Does this issue reproduce with the latest release?
Yes

@willfaught Please use the forum (https://discourse.gohugo.io/) for questions and troubleshooting. We prefer to use GitHub for verified bugs and vetted enhancements. Thanks.

authors: jmooring willfaught

repo: gohugoio/hugo


text:
New Theme : Hugo UILite Portfolio

Theme submission
Before submitting a theme please make sure you've read the README.md. It describes in detail what information are needed and which requirements have to be fulfilled for the submission. The check list below can help you.
Link to my theme repository: https://github.com/uicardiodev/hugo-uilite
I made sure that...

[x] the repository contains a good README.md describing my theme
[x] an open source license has been added to LICENSE.md
[x] all metadata have been added to theme.toml
[x] screenshots have been added in the images/ folder with the required dimensions
[x] in case I'm using a customized demo via the exampleSite folder that
[x] https://example.com is set as base url in exampleSite/config.{toml, yaml, json} to avoid the abuse of unused domains
[x] the site's content is as neutral as possible (contains no ads etc.) and reflects the features of my theme
[x] I tested my theme against the gohugoio/HugoBasicExample
[x] I've checked the developer tools' console in my browser for error messages
[x] in case my theme is using Hugo Pipes features like toCSS and PostCSS that I have committed the /resources directory with all generated assets, for my theme to work in the basic version of Hugo

Feel free to ask questions. We're glad to help.

Hi Arvind,
great to see more submission from. Keep up with the good work!
As usual I've checked your theme and noticed that

the specified image for the avatar in the sidebar can't be found in the repo
the horizontal scrollbar of my browser (Firefox 62 and Chromium 69 on Linux) are to wide which allows me to move the different main sections (skills, contact etc.) way to much to the left and right

@digitalcraftsman Hi,

I've added sim avatar in the images folder, thanks for noticing :)
Yes, actually chrome was not showing any horizontal scroller until it is scrolled horizontally.

I've fixed both of the issues please have a look again
Cheers

Your theme should appear soon on Hugo's theme site. As usual I'll promote your theme on Hugo's official Twitter account in a moment.

authors: digitalcraftsman heyarviind

repo: gohugoio/hugoThemes


text:
adapter.go:172:29: undefined: stm

Hello, I made a mistake in executing this order:
go get github.com/going/casbin-postgres-adapter
E:\GoWorks\src\github.com\going\casbin-postgres-adapter\adapter.go:172:29: undefined: stm

@xiocode can you help solve this bug?

authors: hsluoyz wzbooks

repo: going/casbin-postgres-adapter


text:
Add ingestion job tracking ID

Summary: This issue tracks the addition of a job/dataset Id to track the ingestion of batch data through the Python SDK so that a dataset can be removed from a warehouse database by the user.
Currently there is no easy way for users to undo a failed ingestion. Once data is ingested into Feast through the Python SDK it is written to both feature warehouses and online stores. The current workaround is to reingest the same data (the same entities and timestaps). Feast will allow the user to retrieve the last inserted value and deduplicate results by itself.
In the case of historical data, however, this could lead to large amounts of failed ingestions remaining in the historical store. In an ideal case the user would be able to remove this data after ingestion.
The easiest way to do this is to add an identifier to an ingestion job that tracks all feature rows that are inserted. The user can then go directly to the database and drop these rows.

Another suggestion here is to maintain a list of datasets that have been invalidated. These datasets would be filtered out during retrieval (batch, online), and would eventually be cleaned up from stores.

repo: gojek/feast


text:
Add UI/SDK support to pyfunc ensemblers

Context
In PR #164 and PR #165, a real-time ensembler engine was introduced to process live ensembling requests, using an ensembler that is predefined in the mlflow pyfunc flavour. In PR #170, a process was introduced in Turing API to automatically orchestrate the image building process of a user specified pyfunc ensembler (identified by its given project_id and ensembler_id). Turing API then runs containers based on that generated image as an ensembler service as part of a Turing Router deployment.
This PR introduces user interface changes in order to incorporate the minor tweaks associated with a Turing Router deployment with the inclusion of pyfunc ensemblers.
Minor additions/fixes were also made to Turing SDK to incorporate the pyfunc_config object created from PRs #164, #165 and #170.
Not to be merged before PR #170.
Features

Creation of a panel in the router creation/editing page to select a pyfunc ensembler to deploy from a list of created ensemblers stored in Turing:

Creation of a panel on the router details page showing details of the pyfunc ensembler deployed, with the Ensembler ID and Ensembler Name shown:

Changing pyfunc ensemblers in an existing Turing Router will be reflected as expected for all Turing Router configuration on the diff page:

Main Modifications

ui/src/components/form/service_account_combo_box/ServiceAccountComboBox.js - Renaming of ServiceAccoutComboBox.js to the more correctly spelled ServiceAccountComboBox.js
ui/src/router/components/configuration/components/EnsemblerConfigSection.js - Addition of a panel to display pyfunc ensembler configuration
ui/src/router/components/configuration/components/pyfunc_config_section/* - Addition of a panel to display pyfunc ensembler configuration
ui/src/router/components/form/components/ensembler_config/typeOptions.js - Addition of pyfunc ensemblers to the list of available ensembler options
ui/src/router/components/form/components/pyfunc_config/* - Addition of a new form to fill for pyfunc ensemblers
ui/src/router/components/form/validation/schema.js - Update to schema validation for pyfunc ensemblers
ui/src/services/ensembler/PyFuncEnsembler.js - Addition of new pyfunc ensembler service

Minor Changes to Turing SDK

sdk/turing/generated/* - Changes to OpenAPI auto-generated SDK classes after changes to OpenAPI specs in PR #170
sdk/turing/router/config/router_ensembler_config.py - Addition of a new SDK class to support pyfunc_config
sdk/turing/router/router.py - Minor changes to functions after changes to OpenAPI specs in PR #170
sdk/turing/session.py - Minor changes to functions after changes to OpenAPI specs in PR #170

thanks, lgtm!

authors: deadlycoconuts leonlnj

repo: gojek/turing


text:
Check: errcheck

Original report by @shawnps:
errcheck checks whether a package is checking errors returned by functions. For example, it detects both of these:
recover()

and
_ := recover()

but unfortunately:
shawnps [9:59 PM]
but it runs at the package level :disappointed:
shawnps [9:59 PM]
and the package needs to be in your gopath too so we'd have to like clone the repo into repos/src/github.com/org/reponame and then run it on there
We could still use it but we'd have to hack the repos dir a bit and set GOPATH to it. Then we'd have to clone the GitHub repo into repos/src/github.com/orgname/reponame and run errcheck on there.

The GOPATH thing is probably not a problem anymore, but I get a giant traceback when using errcheck locally, and then it says error: failed to check packages: could not type check: couldn't load packages due to errors: runtime
Will probably investigate a bit more and file an issue if I can't figure it out

tried getting this working a few times but no luck...closing

Where does goreportcard clone/copy packages now, if not a GOPATH workspace?

authors: hermanschaaf shawnps shurcooL

repo: gojp/goreportcard

license_source: bigquery


text:
External file-based registry

What does this do / why do we need it?
This is an example of an approach to #174
Projects building behind a corporate firewall need a way to deal with local projects - and, potentially, locally-cached versions of upstream projects.
Such builds may exist behind a strict firewall; additionally, it's not always the case that the import path "example.com/xyz/bar" actually has the go vanity metadata being served from it.
For these builds, we want a local way to intercept and redirect the actions of dep, to get it to fetch from alternative locations - without first attempting the HTTP(s) dereference of the importpath location.
We do this by introducing a registry. The only functionality that this object must supply is that of the Deducer interface (which is now exported).
The example code only has one kind of registry to resolve these import paths. It reads from the top-level file `Gopkg.reg, which should consist of lines containing whitespace-separated elements. These are:

the import path to match; this is a Golang regexp
the registry root corresponding to that import path. This may contain
references to capture groups in the import path pattern.
one or more git URLs; these may also contain $1-style backreferences.

Example content:
^example\.com/xyz/([^/]*)(/.*)?$ example.com/xyz/$1 ssh://git@local-repo.org/foo/$1

Imports of "example.com/xyz/bar" will be cloned from "local-repo.org/foo/bar".
The current implementation is just a POC: its parsing of Gopkg.reg is
rudimentary. It only supports git repositories. However, the principle
is demonstrated: it requires minimal code-change to get the registry
behaviour into dep. Registry implementations that talk to alternative
repository APIs are, of course, possible.
Note: the behaviour specified in Gopkg.reg is not inherited by any
importing packages; it is a function solely of the local environment.
What should your reviewer look out for in this PR?
Mostly, if this is a suitable approach; the code is just a quick sketch.
Do you need help or clarification on anything?
Which issue(s) does this PR fix?
(partially) fixes #174
NOTE:
README needs fixing prior to merge; you might or might not want to drop the Dockerfiles.

CLA signed

Couple of notes on this.

The registry lookup should come ahead of the wka lookup.

Registry, wka and http are all basically deducers. A second refactor has the top-level deducer calling out to implementations in order; the actual instances can be passed in (so configured at a higher level). We can maintain the Once-like behaviour of http for all of the pieces. As a refactor that's a reasonable second step but I'm not planning on moving ahead unless you think this has legs.

Dep was officially deprecated earlier this year, and the proposal to archive this repository was accepted. As such, I'm closing outstanding issues before archiving the repository. For any further comments, please use the proposal thread on the Go issue tracker. Thanks!

authors: jan-g mvdan

repo: golang/dep


text:
cmd/go: exclude vendor directories from wildcard matches?

What version of Go are you using (go version)? What operating system and processor architecture are you using?

go version go1.5beta1 darwin/amd64

What did you do?

using this repo/branch as a test of vendor/: https://github.com/heroku/log-shuttle/tree/vendor2
GO15VENDOREXPERIMENT=1 go install -v ./...
What did you expect to see?
GO15VENDOREXPERIMENT=1 go install -v ./...
github.com/heroku/log-shuttle/vendor/github.com/pebbe/util
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics
github.com/heroku/log-shuttle/vendor/github.com/heroku/slog
github.com/heroku/log-shuttle/vendor/github.com/bmizerany/aws4
github.com/heroku/log-shuttle/vendor/github.com/nu7hatch/gouuid
github.com/heroku/log-shuttle
github.com/heroku/log-shuttle/cmd/log-shuttle

What did you see instead?
GO15VENDOREXPERIMENT=1 go install -v ./...
github.com/heroku/log-shuttle/vendor/github.com/nu7hatch/gouuid
github.com/heroku/log-shuttle/vendor/github.com/bmizerany/aws4
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics
github.com/heroku/log-shuttle/vendor/github.com/heroku/slog
github.com/heroku/log-shuttle/vendor/github.com/pebbe/util
github.com/influxdb/influxdb/influxql
github.com/heroku/log-shuttle/vendor/github.com/bmizerany/aws4/dydb
github.com/boltdb/bolt
github.com/gogo/protobuf/proto
github.com/heroku/log-shuttle/vendor/github.com/pebbe/util/isatty
github.com/heroku/log-shuttle
github.com/heroku/log-shuttle/cmd/log-shuttle
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/cmd/metrics-bench
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/cmd/metrics-example
github.com/armon/go-metrics
github.com/hashicorp/go-msgpack/codec
github.com/influxdb/influxdb/meta/internal
github.com/influxdb/influxdb/toml
golang.org/x/crypto/blowfish
github.com/influxdb/influxdb/snapshot
golang.org/x/crypto/bcrypt
github.com/golang/protobuf/proto
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/librato
github.com/hashicorp/raft
github.com/stathat/go
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/stathat
github.com/influxdb/influxdb/tsdb/internal
github.com/hashicorp/raft-boltdb
github.com/influxdb/influxdb/meta
github.com/influxdb/influxdb/tsdb
github.com/influxdb/influxdb/client
github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/influxdb
# github.com/heroku/log-shuttle/vendor/github.com/rcrowley/go-metrics/influxdb
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:19: undefined: client.ClientConfig
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:38: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:44: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:52: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:60: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:70: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:82: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:93: undefined: client.Series
vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go:106: client.WriteSeries undefined (type *client.Client has no field or method WriteSeries)

I expected go to skip un-needed stuff (non imported packages) in the vendor/ directory. Perhaps this is a problem with my expectations, but it was a surprise.

What package imports github.com/influxdb/influxdb/influxql ?

@adg it's pulled in from vendor/github.com/rcrowley/go-metrics/influxdb/influxdb.go, which is not imported by the application. Technically I guess I could remove the file from vendor/, but I still think the behavior of ./... should not consider packages in vendor/ unless they are transitively imported by packages outside.

That would require special-casing vendor in package pattern matching, which is something we decided not to do (at this point anyway).

When I run go test ./..., it runs the tests for all of the packages in vendor/ as well as my project. Are you suggesting that this is the desired behavior (for now), @adg?

@technosophos +1 on that problem.
@adg IMO that makes the vendor experiment nearly useless (for me anyway). I'll either need to make godep much more intelligent wrt package handling and/or try to discern what to build through additional package introspection. It's also just a bad user experience IMO.

@technosophos yes, that is the correct behavior as of now.
@freeformz there is an easy work around. Instead of using ./... you can do:
go install $(go list ./... | grep -v /vendor/)

or use go test if you want to run tests.

On the testing front, I think it is valuable to know that your dependencies' tests pass.

Yeah, I don't find any of this particularly disconcerting. I just wanted to verify that it was the intended behavior.

@adg Heh, thanks. I should have thought of that. :-(
Are there any plan to special case vendor/ at all, say in the 1.6 timeframe?

Possibly. That's what this issue is now tracking. We're going to gather people's experience with the 1.5 vendoring experiment before making such plans.

@freeformz After looking at this issue carefully, I would say the current behavior would be my desired behavior. While I understand Godep copies over the entire repository to vendor, I don't think that is what I would encourage. I think copying over each package as needed by the vendor tool is where the analysis work needs to be. At least that is what vendor will do.

@kardianos The issue really has nothing to do with Godep, even though Godep's current behavior triggers it.

A couple reasons I would prefer go test ./... not include things in vendor:
When I choose to take on a dependency, I've already verified it serves my need. That includes verifying automated tests are run by that dependency's overall project, running them myself, or similar. With things in vendor likely changing infrequently relative to my project it doesn't feel right to run the same tests for unchanged things every time. This is the same logic I use for feeling comfortable using, say, net/http in my project without running its tests every time.
Second, It's possible a dependency's tests will never build, work, or be relevant in my particular environment. This could be due to the way the tests are written, differences in developing vs deploying platform, etc. I develop projects on my OS X laptop that use libcontainer that would probably be good examples for this.

@dpiddy Tests that are specific to an environment shouldn't run on a different environment anyway. If they try to run file a bug against the package. I would also argue when copying packages over the vendor tool user should decide if they want to bring over the test files or not
The case where my reasoning falls down is where people choose to use a vcs features like sub-modules.
(Not directly trying to argue here, just state other sides.)

@kardianos good point, tools could do that.
Re the original issue of go install ./... installing things from vendor, that sounds really confusing and surprising. "Why did $GOPATH/bin/x change unexpectedly? Oh, I ran go install ./... in some completely unrelated project that vendors an old version of x three dependencies deep."
Vendoring tools could also skip main packages I suppose.

@dpiddy Of note, I've had at least one person explicitly want to vendor "main" packages. What the kardianos/vendor tool does it to categorize packages by a "status":
Missing (packages referenced but not found)
Standard (in standard library)
Local (package is under "project" and not rewritten or in vendor dir)
External (package is outside of "project" but referenced)
Internal (package is rewritten under "internal" but still a vendor package)
Unused (package is rewritten under "internal" or in vendor folder but unused)
Program (package is a "main" package but rewritten or in vendor folder)
Vendor (package is in vendor folder)
These are two overlapping issues I hope to address soon (location and status), but that's the general idea. Then my tool can work on package status rather then just paths. For instance, if vendor added an install command, you could run "vendor install -status local".
I'm not convinced that adding a "status" concept to the go tool would ever be a desirable thing. "go" already supports "std". The syntax "go install ./...!local" could be interesting I think.

This has come up repeatedly, but it seems like a bad idea.
Wildcards are wildcards.
Vendoring is vendoring.
They're completely separate things.
One should not influence the other.
If there is junk in a vendor/ subtree,
I think that's an argument for deleting, not for ignoring.

I missed Andrew's comment earlier about using this issue to track feedback. That's fine, but if so it should not be marked as milestone Go 1.5. Reopening and reclassifying.

@rsc Thanks for re-opening. I hope other's provide additional feedback ... My general thoughts on your last comments are: I "technically" agree with you, but it makes for a poor user experience.

Having go test ./... run dependencies' tests seems attractive to me in theory, but I just tried it in practice and it was a pain in the ass. Mostly because it means you need to pull in the transitive closure of their dependencies, which is large and which godep doesn't seem to support well.

@jacobsa Sure, but don't confuse the issue of "go test ./..." with a tool, godep. There (will be) many tools.  There will be some that analyze dependencies well.

You're right, sorry. In theory the current tool support isn't so important, but nevertheless it still is a pragmatic issue today (since as far as I can tell godep is the only tool that comes close to supporting the expected format today).

My expected behavior from other languages would be that all wildcard matching would by default apply, but the user should have the ability to easily exclude a folder in a CLI flag or dot file, like gitignore.
git add . takes into account what I've ignored even though it essentially wildcards the whole subtree.
go test ./... --ignore ./vendor wouldn't include packages in vendor.
If not done in the Go tool itself, try to make the "right" values part of the public API so it can be easily extended.
My 2c, Thanks!

I'd like to reopen the discussion on naming the vendor folder, _vendor as
many tools already ignore this pattern.
On 28 Jul 2015 8:45 pm, "Jamie Stackhouse" notifications@github.com wrote:

My expected behavior from other languages would be that all wildcard
matching would by default apply, but the user should have the ability to
easily exclude a folder in a CLI flag or dot file, like gitignore.
git add . takes into account what I've ignored even though it essentially
wildcards the whole subtree.
go test ./... --ignore ./vendor wouldn't include packages in vendor.
If not done in the Go tool itself, try to make the "right" values part of
the public API so it can be easily extended.
My 2c, Thanks!
—
Reply to this email directly or view it on GitHub
https://github.com/golang/go/issues/11659#issuecomment-125552999.

@jacobsa Please look into github.com/kardianos/govendor . It now supports the vendor folder as well. It also flattens any nested dependencies and only picks packages that you use.

@kardianos: Thanks, I quite like the tool. It also imports tests that I don't want; I'll file a bug.

github.com/kardianos/govendor can now ignore test files and arbitrary build tags. If you don't want to run tests, then we don't vendor them.

btw.. for those impatient, this helps run tests and excluding vendor/..
GO15VENDOREXPERIMENT=1 go test $(GO15VENDOREXPERIMENT=1 go list ./... | grep -v '/vendor/')
go list does adhere to the env and excludes the vendor/ directory.. pretty inconsistent / busted implementation :/

@pkieltyka go list ./... actually does include the vendor directories. It is grep that is removing them:

-v, --invert-match
Selected lines are those not matching any of the specified patterns.

Thanks for teaching me about grep, but the point is that it shouldn't include the vendor directories on go test ./...

On Aug 23, 2015, at 6:41 PM, Nathan Youngman notifications@github.com wrote:
@pkieltyka go list ./... actually does include the vendor directories. It is grep that is removing them:
-v, --invert-match
Selected lines are those not matching any of the specified patterns.
—
Reply to this email directly or view it on GitHub.

The documentation for go list says:

Usage:
go list [-e] [-f format] [-json] [build flags] [packages]

List lists the packages named by the import paths, one per line.
The default output shows the package import path

However, github.com/foo/bar/vendor/foo/baz is not a valid import path. You can't import it, you can't build it, you can't install it.
Similarly, running go build github.com/foo/bar/... or go generate github.com/foo/bar/... used to be completely normal and produced expected results in Go 1.4, but as of Go 1.5 it will do unexpected things and fail for repos that have a /vendor/ directory. Setting or not setting GO15VENDOREXPERIMENT has no effect here.
Import path matching and general importing of Go packages uses the following rule for ignoring certain paths:

Directory and file names that begin with "." or "_" are ignored by the go tool, as are directories named "testdata".

Source: http://golang.org/cmd/go/#hdr-Description_of_package_lists.
So, in order to resolve this problem for Go packages inside /vendor/ folder (if that decision is made), I see  two approaches:

Modify the rule to ignore directories and file names that begin with ".", "_" and directories named "testdata" as well as "vendor".

This would have to be done by the go tool as well as every tool in the wild that implements this rule. Tools would need to be updated for Go 1.5 and for GO15VENDOREXPERIMENT even if those people are not interested in using GO15VENDOREXPERIMENT=1 themselves.

Use the existing rules to hide contents of /vendor/ folder. For example, by renaming it to /_vendor/.

I think the real problem is over-using the ./... pattern.
Normally when you're working on one package, you don't need to test
everything, only the actually affected packages. Using ./... is pretty
overkill. Aftet fixing this problem, this issue will be solved as well.
I don't want to add more special cases to cmd/go.

I don't want to add more special cases to cmd/go.

I agree. I would prefer changing the newly added (and still experimental) GO15VENDOREXPERIMENT to use the existing rules of go tool, for example by using the folder name /_vendor/.
I think between the two, it's only acceptable option, given that GO15VENDOREXPERIMENT is still an experiment and it's meant to be opt-in. Using a folder /vendor/ means it's not opt-in for tools that are unaware it's actually a special folder when GO15VENDOREXPERIMENT=1 is set.

Normally when you're working on one package, you don't need to test
everything, only the actually affected packages. Using ./... is pretty
overkill.

Most of my personal repositories contain 1-6~ Go packages. Depending on the exact project, they may be direct subfolders in the git repo root, or they may be a few levels deep. So, when I'm in the repo root of a random project github.com/shurcooL/randomfoo, I really appreciate the ability to do things that affect all Go packages inside that folder (since they're all related to the project I'm working on), rather than having to remember individual Go packages and run commands on them individually (waste of my time, plus risk of forgetting some Go package).
In fact, the ... pattern matching is one of my favorite aspects of the go command and I use/rely on it daily, upwards of hundreds of times. I don't think I abuse it, I think I use it to a great benefit. Without it, I would feel like I'm back in C++ land where I have to write Makefiles that explicitly list subfolders in a given project.

Well, I think this is a pretty obvious one if developer experience matters

On Aug 23, 2015, at 9:09 PM, Dmitri Shuralyov notifications@github.com wrote:
I don't want to add more special cases to cmd/go.
I agree. I would prefer changing the newly added (and still experimental) GO15VENDOREXPERIMENT to use the existing rules of go tool, for example by using the folder name /_vendor/.
I think between the two, it's only acceptable option, given that GO15VENDOREXPERIMENT is still an experiment and it's meant to be opt-in. Using a folder /vendor/ means it's not opt-in for tools that are unaware it's actually a special folder when GO15VENDOREXPERIMENT=1 is set.
Normally when you're working on one package, you don't need to test
everything, only the actually affected packages. Using ./... is pretty
overkill.
Most of my personal repositories contain 1-6~ Go packages. Depending on the exact project, they may be direct subfolders in the git repo root, or they may be a few levels deep. So, when I'm in the repo root of a random project github.com/shurcooL/randomfoo, I really appreciate the ability to do things that affect all Go packages inside that folder (since they're all related to the project I'm working on), rather than having to remember individual Go packages and run commands on them individually (waste of my time, plus risk of forgetting some Go package).
In fact, the ... pattern matching is one of my favorite aspects of the go command and I use/rely on it daily, upwards of hundreds of times. I don't think I abuse it, I think I use it to a great benefit. Without it, I would feel like I'm back in C++ land where I have to write Makefiles that explicitly list subfolders in a given project.
—
Reply to this email directly or view it on GitHub.

+1 to @shurcooL's thoughtful comments. In particular, I use ./... all of the time for testing the implementation details of a command and how they work together.

Have you tried something like this?
go test go install -v ./... 2>&1
It's simplified from what I'm using, but I think you get the idea: only
test what is being affected by the change. Normally your change should not
affect the vendored packages, so the command won't run tests for the
vendored packages.

I would second @davecheney around suggesting bringing the discussion back up around renaming the vendor directory to _vendor which @shurcooL also mentions above.  Like others I believe the  dependency problem should be solved utilizing the conventions already established if possible and _vendor seems like a plausible way to do that.

@jackspirou: Agreed, that's the most elegant solution I've heard so far.

Testing

On the testing front, I think it is valuable to know that your dependencies' tests pass. @adg

Agreed. Since I began using GO15VENDOREXPERIMENT=1, I began noticing failing tests in dependencies I hadn't checked before. It's something I should either look into or reevaluate which dependencies I'm using.
Mind you, it could still be possible to test vendor/ dependencies independently of go test ./.... Though in the little testing I did with a hypothetical _vendor folder, I did run into various issues.

I think copying over each package as needed by the vendor tool is where the analysis work needs to be. @kardianos

This makes a lot of sense to me. If you want to vendor _test.go files, it may be necessary to vendor their assertion and test packages as well (like go get -t). If not, just delete the _test files or have the vendor tool provide that option.
Installation
The original issue is with regards to installation, not testing. Using ./... with go install may be asking for trouble, but I do find it useful:
cd x/tools
go install cmd/...

I guess the @kardianos approach is to have the vendor tool or person vendoring a library be aware of main packages and not copy them if not needed.
Deciding to use vendored tests and main packages
So which is better:

Making the decision once when vendoring files as to which files (tests/cmds) are included?
Deferring the decision to every test/install command (whatever the default may be)?

@shurcooL I see this as opt-in per project, not per user. If a project opts-in, then all the users that use it need to opt-in when developing it.
@nathany Good summary. One additional point I'd like to make is how vendor code is handled. Many people like to think of vendor code per repository, tools are easier to build using this assumption. I like to think of vendor code per package. The difference is extremely important. When thought of as per package it becomes natural to exclude main packages unless they are requested. It is also a natural extension to exclude files based on if they are test files or other build tags. But if you think of vendor code versions and copying as per repository choosing the packages you want out of the mix becomes much more problematic.

Renaming the vendor directory to _vendor will mean that 'go test
_vendor/...' won't work anymore.
I still think the problem arose from overusing the ./... pattern.

Renaming the vendor directory to _vendor will mean that 'go test _vendor/...' won't work anymore.

I believe that is the goal, people using vendoring do not want to test their vendored dependencies

I imagine the goal is to not test vendored packages when using go test ./...
what if I want to test all vendored packages? go test _vendor/... won't
work.
I'd still want to propose negative patterns, which is more generally
useful. e.g. go test ./... -vendor/...

I believe that is the goal, people using vendoring do not want to test their vendored dependencies

What about a situation when there is a bugfix applied to a vendored package that cannot be taken from upstream (for whatever reason). It seems to be useful to run tests for vendored dependencies in this case.

I do not want tests / main packages in vendor/ compiled or executed when ./... is specified, but any packages contained inside of vendor searched and used as necessary. My main motivation for this is so that building / testing can be done simply at the repo level (go install -v ./... & go test -v ./...) w/o having to specify the local packages to build / test via Makefiles or similar means.
I do not want to have to rely on a specific vendoring tool to accomplish this either. I don't think go should require a specific vendoring tool. (PS:  I applaud govendor / @kardianos for implementing so much.)
If I want to run the tests for my vendored dependencies (which I sometimes do), I do that by checking out a version, reading their docs and running their tests in whatever manner is necessary. Sometimes it's not as simple as go test -v ./..., but I think we should strive to make it that easy. ALWAYS.
Renaming vendor/ to _vendor/ I believe accomplished this and IMO is the path to least surprise for users.

What if support was provided for both vendor/ and _vendor/?
Since there seem to be use cases for both wanting to test vendored code and not test vendored code could support be built for both?  This way anyone can choose how they would like to treat testing vendored dependencies for a specific project.  This might mean that the current GO15VENDOREXPERIMENT functionality would work with both _vendor/ and vendor/.
If vendor/ is present go test ./... will test vendored packages and if _vendor/ is present go test ./... will skip over third party vendored packages.   If both _vendor/ and vendor/ is present then an error is thrown.  Perhaps a command could also be included that makes a go project testable by renaming vendor/ to _vendor/ (not that renaming it by hand is difficult, just a thought for automation purposes).
I think allowing both vendor/ and _vendor/ would be a simple solution that leverages established go conventions.  Giving options to different projects might be useful if the value gained from testing third party vendored packages varies from project to project.
While I say "simple" above, I do understand giving this kind of flexibility to users may have unintended consequences.  Go, being known as a very opinionated, usually seems to have one way of solving a particular problem.  Maybe including vendor/ and _vendor is too much flexibility, or maybe it is just the right amount?

What if support was provided for both vendor/ and _vendor/?

No thanks. Then I and all other users don't need to read the following wall of text to understand the difference. Having two nearly-identical ways doing things is not in the Go spirit.

FWIW, I prefer just vendor/

Removing the ability to test your dependencies entirely is not the same as having the ability to not test them.
As a CI user I would want to be able run a minimal set of tests for pull requests that won't hit production, to give me a good feeling that this PR doesn't break existing functionality.
However, before doing a build that would hit a prod server I want the ability to do a full regression of all the sub packages. To make sure that we aren't deploying a version of a dependency that has failing tests.
I also would like to be able to this while being able to keeping the command simple, and not having to explicitly list each dependency, as having to enter our CI tool to update the build scripts on every change would not be ideal.
I would be even fine with making it a requirement that if you wanted to skip vendored tests that you must use go list to feed the go test list of packages.

@itsjamie fair points. this would work too if ./vendor was just ignored in ./...
go test ./... && cd ./vendor && go test ./...
solves both cases

this would work too if ./vendor was just ignored in ./...

If ./vendor is ignored why not just rename it to _vendor to work with other tools and maintain current conventions as @davecheney and @shurcooL have mentioned above?
Then your example would be go test ./... && cd ./_vendor && go test ./... which does not break current conventions.

@itsjamie
I'm not sure how that removes the ability to test dependencies. It just makes them 2 explicit operations, which could be chained together when it's appropriate to do so:
$ go test -v ./... && cd _vendor && go test -v ./...

All sounds good to me.

If you guys decide on _vendor, then I'll support it. I just prefer ./vendor aesthetically, I realize it doesn't make sense beyond that

I also think that go test ./... et al. should ignore the vendor folder by default.
Dependencies should not be treated like they are my own project. Installing my project should not install all dependencies. Testing my project should not test all dependencies.
Before, my dependencies were in $GOPATH, and I did not install with go install or test them with go test, golint them, etc.
All of a sudden, instead of using the good old go test ./... etc, I have to use a wrapper script or a clumsy subcommand with go list ./... | grep -v /vendor/, update all my CI scripts, etc.?

Including ./vendor/ in wildcard commands like go list ./... significantly increases the duration of the command when there are a lot of dependencies (obviously).
While I agree with @rsc that wildcards are wildcards and vendoring is vendoring, including dependencies in a wildcard seems like a significant unintended consequence.
I like the aesthetics of using ./vendor/ as well, but think that the functionality of ./_vendor is what people coming from previous vendoring projects, like godep, are expecting.
I guess my suggestion is that if $GO15VENDOREXPERIMENT=1 then ./vendor should be treated specially and excluded from wildcards.

Honest question: Do people plan to vendor packages by hand?
If they do, these questions have merit. If they plan to use a tool, let the tool do the work after the developer decides what they want to test and what they want to not test. Some will want to run dependency tests especially when if multi-platform. Others may decide they don't want the tests. Then the default behavior falls out.
Do people really not want to make their tools do this work?

I think people do plan to use tools to prepare the vendor directory. For ex, I've written a super crappy one to copy packages from $GOPATH to ./vendor so I can get by: https://github.com/pressly/gv ..which I trigger through a Makefile like in https://github.com/pressly/imgry.
.. the challenge is, once a tool prepares the vendor directory, how will that tool fix go test ./... ? Try to design one and offer some suggestions

On Aug 26, 2015, at 6:34 PM, Daniel Theophanes notifications@github.com wrote:
Honest question: Do people plan to vendor packages by hand?
If they do, these questions have merit. If they plan to use a tool, let the tool do the work after the developer decides what they want to test and what they want to not test. Some will want to run dependency tests especially when if multi-platform. Others may decide they don't want the tests. Then the default behavior falls out.
Do people really not want to make their tools do this work?
—
Reply to this email directly or view it on GitHub.

@pkieltyka Maybe I suck at marketing... I've made one that can remove tests here: github.com/kardianos/govendor . Not trying to promote it here, but once you have a tool that works well, adding a test filter isn't too hard. It only copies over needed or declared packages so go install ./... also works because main packages aren't copied over by default either. I'm sure other tools could do similar things.

One other thing to keep in mind. As much as we want everyone to use Go 1.5
immediately, experience has shown that there are many go programs out there
being built with older versions on a if it ain't broke, don't touch it,
basis.
For those older versions, and more importantly library writers, who want to
adopt the versioning experiment in their code, using _vendor would allow
those library writers to try this feature without being overwhelmed with
support requests from users on older versions.
Thanks
Dave
On Thu, 27 Aug 2015 09:21 Daniel Theophanes notifications@github.com
wrote:

@pkieltyka https://github.com/pkieltyka Maybe I suck at marketing...
I've made one that can remove tests here: github.com/kardianos/govendor .
Not trying to promote it here, but once you have a tool that works well,
adding a test filter isn't too hard. It only copies over needed or declared
packages so go install ./... also works because main packages aren't
copied over by default either. I'm sure other tools could do similar things.
—
Reply to this email directly or view it on GitHub
https://github.com/golang/go/issues/11659#issuecomment-135203788.

@davecheney I understand you are saying using "_vendor", which existing go tool will ignore, will result in fewer support requests to package authors. But the same is true for the go 1.5 tool as well in the context of this issue because go test ./... and go install ./... will also do the same thing as older versions. So while I think I understand what you are saying, I don't follow your logic.
It would appear you think that tools shouldn't exclude test files should the author request it. Is that true? Could you explain why?

@kardianos

But the same is true for the go 1.5 tool as well in the context of this issue because go test ./... and go install ./... will also do the same thing as older versions. So while I think I understand what you are saying, I don't follow your logic.

I don't believe this is true, Go 1.4 will see .../vendor/... as a valid path and attempt to build, test and install code it find inside it. If the path were .../_vendor/... it would be ignored by Go 1.4.2.  Am I misunderstanding how Go 1.4 works ?

It would appear you think that tools shouldn't exclude test files should the author request it. Is that true? Could you explain why?

I am sorry that I gave that impression. I am saying the opposite.

@kardianos I don't want to limit Go users into having to use a specific tool in order to properly vendor things, unless that tool also ships as part of the go toolchain. FWIW: I'd be perfectly happy if govendor was that tool, although it doesn't provide for all of my needs.

@freeformz I'm not suggesting using a single tool. I'm suggesting other tools work a little bit harder.

Any chance we could see a change to _vendor in 1.5.1? Would a CL for that be helpful?

@dpiddy, we have an extremely high bar for changes in point releases (1.n.m): security bugs, or bugs with no possible workaround, and only if they're a new regression (not something we've had for the past few major releases). Tweaking an experimental feature meets none of the criteria.

I don't think we should exclude vendored packages from ./... pattern (i.e.
by
renaming vendor to _vendor)
For example, go get ./... should consider vendored packages because it's
possible for vendored package to still references other packages not
vendored.
I really don't know why everyone seems to insist on doing go test ./...
why not adopt a dependency based testing solution? Always running all
the tests is not going to scale when the project goes bigger. And If you
just
want to run all the tests, I think knowing that the vendored packages also
works correctly is also desirable.

@kardianos I am saying that a single tool should be provided with the go toolchain if this behavior is default.

@minux It also doesn't scale well to have to list every package by hand in a Makefile (for example) so that make test will test all non-vendored packages. That introduces a human element that will certainly be forgotten. Obviously this can be scripted, but it is very ugly. Here are some lines of a Makefile I have for a go1.5 vendored project:
ALL_DIRS=$(shell find . \( -path ./vendor -o -path ./.git \) -prune -o -type d -print)
GO_PKGS=$(foreach pkg, $(shell go list ./...), $(if $(findstring /vendor/, $(pkg)), , $(pkg)))

lint:
	@golint ./... | grep -v '^vendor\/' || true
	@go vet ./... 2>&1 | grep -v '^vendor\/' | grep -v '^exit\ status\ 1'

test:
	@for pkg in $(GO_PKGS); do \
		cmd="go test -v $$pkg"; \
		eval $$cmd; \
		if test $$? -ne 0; then \
			exit 1; \
		fi; \
	done

This isn't just about testing. Here are my thoughts to date....
When vendor/ is the name of the vendoring directory and a package spec of ./... matches everything in that directory as well...
The issues I have with this are:

Most people I've talked to don't expect the package spec ./... to match stuff in vendor/. This indicates to me that it's counter intuitive. I will likely have to do a lot more education about packages / paths / etc. This isn't really a bad thing, but most people I talk to are already confused and/or frustrated by the way $GOPATH works. I don't want to add to that confusion.
It's impossible to test only the packages that I'm writing with a repo level ./.... I care about testing the packages I'm writing more regularly than testing the stuff I've vendored. I'll test the stuff I vendor when I originally vendor it and/or when I upgrade the vendored deps. IMO/E more people follow this workflow than the test everything all the time workflow. I'm worried that this will end up leading people to encode test Makefile or similar targets to avoid having to remember / type the go test $(go list ./... | grep -v vendor/) stuff. I thought we wanted to avoid that?
Additionally some packages test's require additional setup which I may not have on my local machine / environment.
I will require additional meta data about what to install when someone pushes code to me. Right now I can assume that go install ./... will install all of the packages in the pushed repo. Godeps handles this case just fine by recording the package spec(s) used during godep save, but I can tell you most people just use ./...
Similarly for projects with multiple main packages for commands, it will require additional documentation where it's now possible to simply say go get github.com/my/proj/.... This is probably pretty minor though.
As per @davecheney's comment: Not everyone is using Go 1.5. vendor/ will cause issues for users of Go versions < 1.5.

Some, but not all of the above stuff can be handled by vendoring tools. IMO if we're going to cause this much disruption then Go itself should provide a vendoring tool such as govendor/godeps/something new/whatever as part of the standard tooling (i.e. go vendor).

I second @freeformz's points FWIW..

@minux

And If you just want to run all the tests, I think knowing that the vendored packages also works correctly is also desirable.

If I add dependency X that is only a sub-package of a larger package Y, the whole thing ends up in the vendored folder, even if I only need the subpackage and its dependencies. If I test the whole project with go test ./..., it tests the whole package Y. I don't need to test all of Y, and worse, it will fail if I did not add its recursive dependencies, which I don't need for package X.
You may ask why I add the all of Y if I only need X. The reason is that I use git submodules to add the repo, and go get behaves the same way.

I share all of @freeformz's concerns. I think it is telling that at least godep has always used an ignored _workspace path, even when using godep save -r which made saved dependencies valid import paths as the vendoring experiment does.
@bradfitz that is what I figured. I fear if it takes a major release to change the experiment's behavior enough momentum (whether because it's good or not) will be built with the current behavior that it will be unlikely to ever change.

My main concern, before I can start to think about others, is that of the import paths. As I understand example.com/foo/bar/vendor/example.com/foo/baz is not a valid import path. example.com/foo/baz is a valid import path of a Go package that happens to be vendored inside example.com/foo/bar.
Are other people okay with the idea that go list all will print out a bunch of invalid import paths (in addition to valid ones)? How should other tools deal with these invalid import paths?
I thought the point of vendoring was to put a copy of the original package (e.g., example.com/foo/baz) inside your VCS tree, but not change it in any way. No import path rewriting required - the vendored package still has the same original import path. You should be able to vendor/not vendor any package as you wish. It seems to do that but also change import paths of the packages being vendored.

I've been dealing with the repercussions of the vendoring experiment for a few weeks now.  I like having fine-grained control over my dependencies, and it’s awesome to be able to commit the vendor source along with my project source without having to change all the vendored import paths.
I think I voice the same concern as most of the non-googlers here, which is that I didn’t expect to have golint, go vet, and go test blew up in my face when I moved everything to the vendor directory.  It’s frustrating to have to contort my workflow to accommodate external code that didn’t previously require it.
However, I wouldn’t have known about many of these problems with the third-party libraries had I not moved them into the vendor directory.  It’s been a pain to get everything to play nice, and I understand that not everyone has the luxury of being able to fix other-people’s-code, but the new methodology incentivizes quality code… which is one reason I believe Go is as successful as it is today.
If I compare the two solutions from a long-term perspective, I see two very different outcomes.  Allowing people to ignore bad code results in a complex solution whereby some code is whitelisted, some is blacklisted, the lists require regular maintenance, and you have no guarantee of a minimum level of quality.  The other solution is to develop good code, fork bad code, and take the penalty of due diligence up front.
I’m not saying either solution is right or wrong, but since this issue is still open for feedback, I’m of the opinion that the short-term pain of broken libraries will eventually subside as the authors feel the pain and/or new authors emerge.  I believe that the long-term result will be a better foundation from which to build future solutions.

I was asked to provide my opinion here, so here it is.
I agree with @rsc's comment in https://github.com/golang/go/issues/11659#issuecomment-123126664.
Further, it seems renaming the directory to _vendor would make it harder to operate on the vendored packages. For example, it's useful (at least to me) to be able to run go list ./vendor/... and see everything that's been vendored.
Personally, I think the current behavior is ok. It hasn't really been a problem for me (though I understand why some find it inconvenient). For example, when I run interactive commands to test or vet packages, I type go test or go test ./foo ./bar or maybe go test ./foo/... since I generally want to test only one or two packages at a time. Our automated testing system tests all our packages, using a command like go test $(go list ./...|grep -v vendor). This would be inconvenient to type by hand, but there is more than one way to make it convenient. Complicating the behavior of the go command probably isn't the best way.

Thanks for all of your input. I think everything has been covered here that could be covered.
I'm satisfied with the outcome, although I still think it ignores usability a bit, so I'm going to close this issue, (since I opened it).

FYI, see https://godoc.org/rsc.io/gt for a go test wrapper
that could help with this issue.

That doesn't help with the issue where go list all outputs invalid import paths (sorry, I'm repeating myself).

it's always possible for go list to output invalid import paths.
try run go list outside of GOPATH or GOROOT.

But I said go list all specifically, not just go list.
I have multiple tools that use the output of go list all to navigate and open Go packages. For example, if I want to go to github.com/russross/blackfriday, I expect there to be just one. With vendoring as is, there may be 10, 20 or 30+ matches for blackfriday in various vendor folders.
All tools that prefer to preserve previous behavior of not including vendored package copies (e.g., like they did not include the vendored packages in Godeps/_workspace subfolders) need to be updated to perform go list all and then try to filter out vendored copies. Even if GO15VENDOREXPERIMENT is not enabled.

@shurcooL In hopes of being helpful, you might try to migrate your tools to use something else. For instance you could try http://godoc.org/github.com/kardianos/govendor/context create a new context then call ctx.Status(), then filter all the returned items by their "status" which includes if they are in a vendor directory or not. If that doesn't quite fit your need, but is close, feel free to open an issue.

I have been doing research and setting up a continuous integration & testing pipeline for Go during most of the day, on different tools, and this is an issue that really bothers me.
I love Go for it's minimalist and clean approach. In line with that, I use the clean architecture approach (or something close to it) for most of my projects. This means I will usually have four subpackages, with the main file in the root folder. Sometimes, there are some small additional tools in the cmd folder as well. I was under the impression that this is a configuration that most projects use.
I now have to use a dirty workaround to get CI to build and test properly, all due to the fact that ./.. includes folders in vendor. For testing, and even for building if there are tools included. This is counter-intuitive and not well thought-out at all. Which is contrary to what I've come to expect and appreciate about Go. The default case should not require any workaround.
There should be a coherent approach where all go tools exclude vendor when using ./... if the GO15VENDOREXPERIMENT environment variable is set to 1. That would be consistent. As someone mentioned before, you can still do "cd vendor && go test ./..." if you want to include the subpackages. But you will have to be explicit about it, which is fine since it constitutes a non-default workflow imo.
If the go tool deals with vendoring using the environment flag (and by default in Go 1.6), then it should also deal with the resulting consequences and solve them in a coherent way. Let's keep it simple, folks.

I think there are two cases:

A package in a vendor tree that is actually used (directly or
indirectly) by the code outside the vendor tree. In this case I think you
would want to test the package, just as you test all your other
dependencies. If code you depend on is broken, you want to find out in the
most precise test possible, not wait until a test of the main program fails
mysteriously.

A package in a vendor tree that is not actually used (directly or
indirectly) by the code outside the vendor tree. In this case, obviously no
one cares and the test need not run. But in this case I don't understand
why tools are bringing that package into the vendor tree in the first
place. I would argue that, for many reasons, dead (unused) code should not
be copied into a vendor tree. Code that is in your vendor tree has to be
maintained, updated, and so on. If it's not needed, it should just be
omitted.

I think it's a mistake to skip the test in (1). And I think (2) indicates a
problem in the vendoring process.
The counter-argument to this is that you can't omit individual packages
when you're using git submodules. But when you're using git submodules I
think you're tying yourself to the target repository quite a bit more and
so it may still be appropriate to test the entire thing.

@rsc I disagree even with your first point. If you vendor your dependencies, you will usually pin a specific version, so it is nonsense to run the tests each and every time you push a commit and your CI kicks in. You should test your dependencies once, manually, each time you update them. That's completely besides the point of automatic testing of your own commits and pull requests.

Agree with @awishformore, similar to what I said in https://github.com/golang/go/issues/11659#issuecomment-122539346.
How many projects that use net/http run those tests every time? Would that be reasonable? I don't think so, because presumably you've verified that the Go release you're using is good and you know net/http isn't changing out from under you.
I feel things in vendor/ can be treated similarly: on add/change, verify it works for you, then be sure the vendored code doesn't change.

As others have noted, there is an easy workaround if you just want to test
everything but vendor:
go test $(go list ./... | grep -v /vendor/)

Depending on your configuration, it may be appropriate or worthwhile to use
that in your CI system. My point is that it may also be appropriate or
worthwhile to include those tests, so it is not appropriate for ./... to
categorically exclude certain directories. Different people use vendor
differently. Some people do make local changes, or do want to include it in
"go vet ./..." or other commands.
I also still think that for the specific case of testing, if we could solve
test caching then that would help considerably.

@rsc Please don't qualify it as an "easy" workaround. It's a dirty workaround for something that should work out-of-the-box. Why? Because someone decided that the Go tool should deal with vendoring, and since it does, the default way should correspond to the most intuitive and common approach. The workaround should be required if you do something out of the ordinary, like testing dependencies on each build or, like you mention, customizing dependencies. In that case, a workaround (i.e. cd vendar && go test ./...) seems perfectly fine. I don't understand why you argue to make the default more annoying in favour of some edge cases. One of the most wonderful things about Go is that defaults usually work in intuitive ways and hardly ever annoy me.

You should test your dependencies once, manually, each time you update them.

It's more convenient and simpler (for both the user and for the go tool's implementation) to test them every time you run your own tests. I'd love to know, concretely, why some folks are so resistant to this idea. Is it because running more tests is slower? (If so, see @rsc's last comment above.)

It's a dirty workaround for something that should work out-of-the-box.

From my point of view, it does work out of the box. That's because I like to run the tests for my dependencies. Of course, anyone is free to prefer otherwise, but that preference doesn't necessarily imply that the go tool's design is wrong.

That's because I like to run the tests for my dependencies.

In case it's helpful, here's an easy workaround if you just want to test dependencies:
go test $(go list -f '{{join .Deps "\n"}}' ./...)

@kr build time is certainly a factor with current Go versions. I also gave my reasons in https://github.com/golang/go/issues/11659#issuecomment-122539346 and other comments.
Perhaps my mental model of what vendor/ is is just wrong. I consider it to be an extension of the stdlib for my project. If that is true, should I be running tests for everything I've used from the stdlib every time I test my project?

Of course I'm forgetting that an easy way to exclude vendored tests is to do just that: exclude them when vendoring things. At least godep and govendor already support this. Same for unwanted main packages.
Curious to see how things like go install github.com/foo/bar/... play out once 1.6 is released.

The fact that everyone is proposing workarounds just points to the fact that this should be fixed.  Renaming it to _vendor just works.  If you want to test or install stuff from your vendor directory, you can by just cd'ing into that directory.... but if you don't want to, then you can go whatever ./... just like you do today, and it'll work just like it does today.  Making it "vendor" breaks things.  I can't imagine anyone wanting that to be the default.
Keep go simple.  Make the vendor directory _vendor so that tools skip it.  I shouldn't have to use some annoyingly arcane command to just build and test my code.

"vendor" is not being renamed. That ship has long sailed.
I think everything that can be said on this issue has been said in the many comments above. I think it's time we move this to other forums (for general bugs) or new bugs (for specific new bugs that aren't just a dup of this one). Ultimately this bug is a policy/behavior question and not actually a bug report.

authors: adg awishformore benma bradfitz davecheney dpiddy freeformz itsjamie jackspirou jacobsa joshuarubin kardianos kostya-sh kr minux natefinch nathany pkieltyka rsc shurcooL technosophos wardn

repo: golang/go


text:
cmd/cover: Allow specifying uncovered lines

In this reddit discussion, /u/jerf has a nice suggestion: label lines that should be impossible to reach to improve coverage. Something like follows:
res, err := prudentFunction("constant")
if err != nil {
  panic(fmt.Sprintf("The impossible has happened: %v", err)) // gocover: Ignore
}

The coverage tool could then instrument the line and ignore it if there are 0 passes, and issue an error if there were more than zero.

CC @robpike

No, thanks. I dislike tools forcing people to annotate their code to silence them.

authors: brunokim ianlancetaylor robpike


text:
testing: b.Fatal does not terminate test

lucky(~/src/issue) % cat issue_test.go
package issue

import "testing"

func BenchmarkFail(b *testing.B) {
        for n := 0; n < b.N; n++ {
                b.Fatal("non zero exit")
        }
}
lucky(~/src/issue) % go test issue_test.go -test.bench=.
testing: warning: no tests to run
PASS
BenchmarkFail-4 --- FAIL: BenchmarkFail-4
        issue_test.go:7: non zero exit
ok      command-line-arguments  0.003s
lucky(~/src/issue) % echo $?
0

The subject of the issue does not match the behavior of the program, and there is no explanatory text. Please explain.

b.Fatal should terminate the test program like its counterpart t.Fatal

t.Fatal does not terminate the program.

I am sorry, you are correct. But calling t.Fatal in a test will cause the
test binary to eventually exit with a non zero error code. I was surprised
that calling the counterpart on testing.B did not cause the same behaviour.
On Sat, 13 Feb 2016, 09:32 Rob Pike notifications@github.com wrote:

t.Fatal does not terminate the program.
—
Reply to this email directly or view it on GitHub
https://github.com/golang/go/issues/14307#issuecomment-183512337.

@davecheney want to change the issue title? Maybe "testing: (*B).Fatal should cause non-zero exit status".
I took a brief look at this. The code does this right now (irrelevant lines elided):
testOk := RunTests(m.matchString, m.tests)
exampleOk := RunExamples(m.matchString, m.examples)
if !testOk || !exampleOk {
	fmt.Println("FAIL")
	return 1
}
fmt.Println("PASS")
RunBenchmarks(m.matchString, m.benchmarks)
return 0

Unfortunately the exported function RunBenchmarks doesn't return failure status. Perhaps we could modify InternalBenchmark to smuggle failure status through.
If we fix this, should we also only print "PASS" if benchmarks pass, as well as tests and examples?

Would adding a return value to RunBenchmarks break the compatibility promise? It seems to me it keeps the source compatibility since ignoring the single return value doesn't require any change to calling code.

Would adding a return value to RunBenchmarks break the compatibility promise?

Yes.

@cmarcelo, see http://blog.merovius.de/2015/07/29/backwards-compatibility-in-go.html

@bradfitz Thanks for the link.
I wrote a CL that adds an extra field to the struct InternalBenchmark to pass the information (a "failed bool" field). It does pass the API check in all.bash, and seems "acceptable" from reading https://golang.org/doc/go1compat document, and the Internal in the name makes me thing is not widely used. On the other hand it doesn't pass the criteria from the post you've linked. So I'm a bit unsure whether is OK or not.
Pushed the CL in https://go-review.googlesource.com/#/c/19889/.

Ian had a better suggestion without having to add a field to the exported struct, it became patch set 3 in the CL.
I'm still curious about the compatibility: because of the embedding explanation mentioned in the link, is adding fields a no-go for exported structs?

In the compatibility doc (https://golang.org/doc/go1compat) we explicitly permit adding fields to exported structs.

authors: bradfitz cespare cmarcelo davecheney ianlancetaylor robpike


text:
cmd/vet: validate that time.Format strings are sane

vet will let the user know if fmt.Printf strings are incorrect and if the types passed to them are incorrect. It'd be very handy if the also rather specific format for time.Format strings were similarly checked for validity.

@cellofellow could you provide some examples of wrong inputs and errors you'll expect?

fmt.Printf would still print something, albeit not what was intended. But time.Format would simply throw an error. So it's not like a mistake in the time.Format would go unnoticed so easily.
@alandonovan @mvdan

FYI: a timeformat checker now exists.

authors: adonovan agnivade cellofellow rugginoso


text:
crypto/x509: Wrong error text when failing to unmarshal key-id

See: https://github.com/golang/go/blob/master/src/crypto/x509/x509.go#L1136
The error returned when unmarshmaling a key-id reads x509: trailing data after X.509 authority key-id. However, this is not the authority-key id, it is the key-id.
Instead it should read x509: trailing data after X.509 key-id
It looks like someone copied and pasted the error from here (https://github.com/golang/go/blob/master/src/crypto/x509/x509.go#L1102) without making the needed change to the text.

https://go-review.googlesource.com/22240

authors: bradfitz phayes


text:
x/cmd/coordinator: hang if writing source tarball fails midway

I noticed an openbsd build fail and hang:
  builder: openbsd-amd64-gce58
      rev: 26124695adcf9440d38dbd2f117221019243d4fa
 buildlet: http://10.240.0.25 GCE VM: buildlet-openbsd-amd64-gce58-rn1ced503
  started: 2016-05-06 18:53:34.631002774 +0000 UTC
   status: still running

Events:
  2016-05-06T18:53:34Z get_buildlet
  2016-05-06T18:53:34Z awaiting_gce_quota
  2016-05-06T18:53:34Z finish_awaiting_gce_quota after 3.07µs
  2016-05-06T18:53:34Z create_gce_buildlet buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:53:34Z create_gce_instance buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:54:02Z finish_create_gce_instance after 27.855829702s; buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:54:02Z wait_buildlet_start buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:54:04Z got_instance_info waiting_for_buildlet...
  2016-05-06T18:54:04Z finish_wait_buildlet_start after 1.617098383s; buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:54:04Z finish_create_gce_buildlet after 29.473051761s; buildlet-openbsd-amd64-gce58-rn1ced503
  2016-05-06T18:54:04Z finish_get_buildlet after 29.473098819s
  2016-05-06T18:54:04Z using_buildlet 10.240.0.25:80
  2016-05-06T18:54:04Z write_go_bootstrap_tar
  2016-05-06T18:54:04Z write_version_tar
  2016-05-06T18:54:04Z get_source
  2016-05-06T18:54:04Z finish_get_source after 31.995µs
  2016-05-06T18:54:04Z write_go_src_tar
  2016-05-06T19:03:50Z finish_write_go_src_tar after 9m46.615158891s; err=writing tarball from Gerrit: Put http://10.240.0.25/writetgz?dir=go: write tcp 10.240.0.3:33048->10.240.0.25:80: write: connection reset by peer
 +1047.1s (now)

Build log:

(buildlet still starting; no live streaming. reload manually to see status)

One goroutine is stuck at:
goroutine 3391 [select, 28 minutes]:
net/http.(*persistConn).roundTrip(0xc82039c2d0, 0xc820845b80, 0x0, 0x0, 0x0)
	/home/bradfitz/go/src/net/http/transport.go:1701 +0x95f
net/http.(*Transport).RoundTrip(0xc820fd1b30, 0xc8205905a0, 0xc820fd1b30, 0x0, 0xc800000000)
	/home/bradfitz/go/src/net/http/transport.go:385 +0x42a
net/http.send(0xc8205905a0, 0xd7b880, 0xc820fd1b30, 0x0, 0x0, 0x0, 0x8, 0x2a, 0xc820021238)
	/home/bradfitz/go/src/net/http/client.go:253 +0x14f
net/http.(*Client).send(0xc8206c8ea0, 0xc8205905a0, 0x0, 0x0, 0x0, 0xc820021238, 0x0, 0x1)
	/home/bradfitz/go/src/net/http/client.go:143 +0xef
net/http.(*Client).doFollowingRedirects(0xc8206c8ea0, 0xc8205905a0, 0xb0dae0, 0x4, 0x5f9901, 0xc8202c85dc)
	/home/bradfitz/go/src/net/http/client.go:498 +0x4d4
net/http.(*Client).Do(0xc8206c8ea0, 0xc8205905a0, 0xc8218518c0, 0xc8218518c0, 0x45defe)
	/home/bradfitz/go/src/net/http/client.go:184 +0x10b
golang.org/x/build/buildlet.(*Client).do(0xc8202c8540, 0xc8205905a0, 0xa36c80, 0xc8206e10b0, 0xc821636d80)
	/home/bradfitz/src/golang.org/x/build/buildlet/buildletclient.go:210 +0xf5
golang.org/x/build/buildlet.(*Client).doOK(0xc8202c8540, 0xc8205905a0, 0x0, 0x0)
	/home/bradfitz/src/golang.org/x/build/buildlet/buildletclient.go:306 +0x55
golang.org/x/build/buildlet.(*Client).PutTarFromURL(0xc8202c8540, 0xc8208ac6e0, 0x4f, 0xabb798, 0x5, 0x0, 0xd7b400)
	/home/bradfitz/src/golang.org/x/build/buildlet/buildletclient.go:345 +0x2cb
main.(*buildStatus).writeBootstrapToolchain(0xc820e05680, 0xc828288740, 0xc828288750)
	/home/bradfitz/src/golang.org/x/build/cmd/coordinator/coordinator.go:1749 +0xe4
main.(*buildStatus).(main.writeBootstrapToolchain)-fm(0x7573655200000008, 0xb0e120)
	/home/bradfitz/src/golang.org/x/build/cmd/coordinator/coordinator.go:1414 +0x20
go4.org/syncutil.(*Group).Go.func1(0xc8206e1020, 0xc821852fa0)
	/home/bradfitz/src/go4.org/syncutil/group.go:35 +0x4d
created by go4.org/syncutil.(*Group).Go
	/home/bradfitz/src/go4.org/syncutil/group.go:41 +0x58

And the parent one is stuck at:
goroutine 247 [semacquire, 28 minutes]:
sync.runtime_Semacquire(0xc8206e102c)
	/home/bradfitz/go/src/runtime/sema.go:47 +0x26
sync.(*WaitGroup).Wait(0xc8206e1020)
	/home/bradfitz/go/src/sync/waitgroup.go:131 +0x8d
go4.org/syncutil.(*Group).Err(0xc8206e1020, 0xc821852fa0, 0xe)
	/home/bradfitz/src/go4.org/syncutil/group.go:52 +0x23
main.(*buildStatus).build(0xc820e05680, 0x0, 0x0)
	/home/bradfitz/src/golang.org/x/build/cmd/coordinator/coordinator.go:1415 +0xfa8
main.(*buildStatus).start.func1(0xc820e05680)
	/home/bradfitz/src/golang.org/x/build/cmd/coordinator/coordinator.go:1267 +0x3d
created by main.(*buildStatus).start
	/home/bradfitz/src/golang.org/x/build/cmd/coordinator/coordinator.go:1275 +0x53

And from the coordinator logs:
2016/05/06 18:55:04 Buildlet http://10.240.0.25 GCE VM: buildlet-openbsd-amd64-gce58-rn1ced503 failed three heartbeats; final error: timeout waiting for headers
2016/05/06 18:55:04 Sent request to delete instance "buildlet-openbsd-amd64-gce58-rn1ced503" in zone "us-central1-f". Operation ID, Name: 9118484307818665575, operation-1462560904200-53230ff283f41-44626dc1-b9470cc4

So the VM was deleted but RoundTrips are still in-flight, hung forever.


text:
package testing fail for go1.6+

What version of Go are you using (go version)?
go1.4

What operating system and processor architecture are you using (go env)?
redhat el6/amd64

What did you do?
tried to compile go1.6.2 from source using go1.4 (also compiled from source) using the instructions on https://golang.org/doc/install/source

What did you expect to see?
ALL TESTS PASSED

What did you see instead?
panicking during package testing. Full error linked below.
go1.6.2 package testing panic.txt

The package testing errors only occurred when trying to compile for go1.6 and above. Compiling versions go1.5.4 and below is successful and passes package testing.

The system you were building on does not have enough free memory (either physical, or swap) to complete the build. We don't track the memory usage required to run ./all.bash but it is at least 512mb per core.
I'm going to close this as I think there is no issue here.

Increasing thread count per user solved the issue. Same bug here with solution. https://github.com/golang/go/issues/13968

authors: davecheney paeynivek


text:
Proposal: x/net/ethernet: new package for ethernet sockets

At this time, no official x/net package is available to create and use ethernet sockets in Go.  I have written a couple of packages which can be used for this purpose, but I'd like to try to create an  x/net/ethernet package as an official Go x/net subproject.
My packages can be found at:

https://github.com/mdlayher/raw
https://github.com/mdlayher/ethernet

Ethernet sockets are useful for low-level networking applications like ARP, LLDP, ATA over Ethernet, and a wide variety of other networking protocols.  This package would make use of Linux's raw sockets and the BSD family's BPF to enable this functionality, and I already have working examples for these operating systems in my raw package.  Adding support for other operating systems should be possible over time, but I have no personal experience working with ethernet sockets outside of Linux and BSD.
The proposed API could look something like:
package ethernet

// An Ethernet frame, which the Conn and RawConn types marshal
// and unmarshal to transport data
type Frame struct {
    // Contains usual Ethernet fields
}

// net.Addr implementation which indicates source or destination
// hardware address of Ethernet frame
type Addr struct {
    HardwareAddr net.HardwareAddr
}
func (a *Addr) Network() string
func (a *Addr) String() string

// Protocol identifier used to filter traffic by EtherType
type EtherType uint16

// A net.PacketConn which wraps and unwraps bytes from ethernet
// frames seamlessly.
type Conn struct {}
func Listen(ifi *net.Interface, et EtherType) (*Conn, error)
func (c *Conn) Close() error
func (c *Conn) LocalAddr() net.Addr
func (c *Conn) ReadFrom(b []byte) (int, net.Addr, error)
func (c *Conn) SetBPF(filter []bpf.RawInstruction) error
func (c *Conn) SetDeadline(t time.Time) error
func (c *Conn) SetReadDeadline(t time.Time) error
func (c *Conn) SetWriteDeadline(t time.Time) error
func (c *Conn) WriteTo(b []byte, addr net.Addr) (int, error)

// A modified Conn which returns all Ethernet frame data on incoming
// frames and enables writing arbitrary outgoing frames.
type RawConn struct{}
func ListenRaw(ifi *net.Interface, et EtherType) (*RawConn, error)
func (c *RawConn) Close() error
func (c *RawConn) ReadFrom(b []byte) (f *Frame, p []byte, error)
func (c *RawConn) SetBPF(filter []bpf.RawInstruction) error
func (c *RawConn) SetDeadline(t time.Time) error
func (c *RawConn) SetReadDeadline(t time.Time) error
func (c *RawConn) SetWriteDeadline(t time.Time) error
func (c *RawConn) WriteTo(f *Frame, b []byte) error

I am curious if 802.11Q VLAN tags should be included in this package.  In general, the operating system seamlessly handles adding and removing VLAN tags as needed, so it may not be necessary, but could also be nice to have.
In addition, since the runtime network poller is not accessible outside of package net at the moment, the SetDeadline methods may just return an error until implemented later.  See my other proposal at #15984 .
If implemented, this package would fix #8432.
Thanks for your time, and please feel free to ask for clarification if needed.

/cc @mikioh

Thank you for your proposal. A few random thoughts.

Perhaps datalink might be a better package name
I'm not keen on providing Ethernet marshaler/unmarshaler
It's okay that this package focuses on a bit slow BPF/LSF and AF_PACKET socket IO
Having a separate package that implements Netmap/DPDK-like faster stuff sounds reasonable
s/Conn/PacketConn/
It's fine to have an internal kqueue/epoll code fragments
Probably we need a type that can be used to configure BPF/LSF files and AF_PACKET sockets; maybe 'type Listener struct { }`?

A simple question: What's the reason why we need to provide RawConn on Linux?

Perhaps datalink might be a better package name

I'm not keen on providing Ethernet marshaler/unmarshaler

I'd be okay with making this package called datalink, but since I assume most folks would use it for ethernet sockets anyway, I figured I would go for the more common use case.  If this package were to be a raw datalink package, should another package be created to leverage it with ethernet sockets (x/net/ethernet or maybe even x/net/datalink/ethernet)?

It's okay that this package focuses on a bit slow BPF/LSF and AF_PACKET socket IO
Having a separate package that implements Netmap/DPDK-like faster stuff sounds reasonable

I'm not familiar with Netmap or DPDK, but perhaps they could be cleanly integrated into this package at some point too.

Probably we need a type that can be used to configure BPF/LSF files and AF_PACKET sockets; maybe 'type Listener struct { }`?

Sure, but what type of options would need to be configured?  Would this be things like the ability to set the socket to nonblocking, or change the direction of traffic to capture using BPF?

A simple question: What's the reason why we need to provide RawConn on Linux?

Maybe we don't.  My intention for the RawConn type was to enable writing arbitrary ethernet frames, but perhaps it isn't needed after all.  If this package only focuses on "datalink" sockets, then this functionality is totally unneeded.

I think that providing separated datalink IO and datalink wire-format/coding packages makes sense. Because the former needs to work on abstraction of various kernel system calls and data types, and the latter should take a burden of various wire-formats/codings come from IEEE wildcats. For example,
package datalink

type PacketConn struct { /* TBD */ } // implements net.PacketConn interface and a few fancy datalink IO specific methods
type Listener struct {} // or Datalink or Config or something else
func (ln Listener) Listen(ifi *net.Inteface) (PacketConn, error)

--

package ether // or ethernet

type Frame interface { Tag; Stack }
type Tag interface { Len() int; Marshal() ([]byte, error) }
type Stack interface { Lower() Tag; Upper() Tag }

type Dot3 struct {}
  :
type Dot1ad struct {}

If this package were to be a raw datalink package, should another package be created to leverage it with ethernet sockets (x/net/ethernet or maybe even x/net/datalink/ethernet)?

For the package that provides Ethernet encoding/decoding functionality, all of us know a variety of MAC/LLC sublayers from DIX/dot3 through dot22, and dot1 higher layers including MAC bridging through congestion notification. Ah, I have no strong opinion on it. Probably we can use github.com/google/gopacket package.

Sure, but what type of options would need to be configured?

There are a few options that should be configured before calling bind on AF_PACKET sockets, I guess.

Assigning to @mikioh to make a decision.

A simple question: What's the reason why we need to provide RawConn on Linux?
I just found this proposal because I was searching for a way to read and emit raw ethernet frames in Go. My intended project is a "universal auto-join" tool which is used in security testing settings to join any network, regardless of configuration (presence of DHCP, MAC filtering, etc). I need to have direct access to raw frames for this project to do things like packet sniffing, ARP spoofing, manually speaking DHCP, etc.

@joshlf as an interim solution, you can use my packages:

https://github.com/mdlayher/raw
https://github.com/mdlayher/ethernet

@mdlayher Thanks, I will :)
While we're on the subject - if this proposal ends up getting adopted, I'd suggest two additions from your raw package:

Be able to create a connection which returns frames of any protocol/ethertype
Be able to listen for ethernet frames not destined to the current host or to the broadcast MAC address (ie, promiscuous mode)

Hey @mikioh :
package datalink

type PacketConn struct { /* TBD */ } // implements net.PacketConn interface and a few fancy datalink IO specific methods
type Listener struct {} // or Datalink or Config or something else
func (ln Listener) Listen(ifi *net.Inteface) (PacketConn, error)

^ I'd be happy to look into implementing your proposed API here for a x/net/datalink.  Does that work for you?

func (ln Listener) Listen(ifi *net.Inteface) (PacketConn, error)

Why have a Listener type? Why not just a Listen function at the top level?

Why have a Listener type? Why not just a Listen function at the top level?

The Listener type could hold socket options to be set during the Listen call. For example, Listener could have a Promiscuous bool, where the a call to Listen would only succeed on a successful call to setsockopt(sockfd, SOL_PACKET, PACKET_ADD_MEMBERSHIP, ...) with mr_type=PACKET_MR_PROMISC (for the linux implementation).

@freddierice That's a good point; I suppose if you had a top-level Listen function, it'd still have to create a listening socket under the hood (at least on Unix).

@mdlayher,
Sorry for being late. I'd like to postpone decision making to follow the consensus of #17244. Also we need to work on #15984 first.

On hold for #17244.

At this point in time, I don't think it's necessary for this functionality to live under x/net; especially since it isn't needed in the standard library or similar. I'd be okay with closing this out, unless there are strong objections to doing so.

Punch it, please!

authors: bradfitz freddierice joshlf mdlayher mikioh rsc