text:
Implement Node.clear_children

Allows to quickly clear out all children of a node.

Production edit: This closes https://github.com/godotengine/godot-proposals/issues/764.

This is a new feature. I would suggest opening a proposal to gauge interest and give good reasons for this being in core (For example, why this would be better than queue_free in a loop).

Existing proposal: https://github.com/godotengine/godot-proposals/issues/764 (although a bit of the conversation in the comments if mostly sidetracking/offtopic in hindsight; the performance debate of .queue_free() is IMO orthagonal to having a single method to call for ease of use)

My concern is how configurable the method should be. Do you want to call queue_free(), free() or even remove_child(). Maybe we could add a parameter?
Also, there is a propagate_call() method, but it includes the node on which you are calling it. We could add an optional parameter or a separate method to exclude the node.

I can attest that propagate_call() has been useful for me countless times, but I've always had to exclude the parent.
But it walks through all descendants, not only children.

For an engine that's centered around nodes and children, I've always been amazed this doesn't exist.
I use my Utils.clear_children() quite often, and propagate_call() is not an appropriate replacement, it would just be long and confusing with extra parameters, plus it's not a well known function in general.
And I agree this method should have an option to remove the children, I have needed that.

As I noted in https://github.com/godotengine/godot/pull/94163#discussion_r1899695477, I'd expect that if deferred is false, the children are deleted immediately with memdelete() (aka free()). Currently the clear_children() method will not really delete children (by default); they will stay after calling the method and only get deleted at the end of the frame. Might be confusing.
Though if memdelete() were to be used, it needs a warning, because you can't just use it at any time.

I'd expect that if deferred is false, the children are deleted immediately with memdelete() (aka free()).

Is this safe to do?

Is this safe to do?

Not always. Hence using queue_free() is usually recommended.

Not always. Hence using queue_free() is usually recommended.

Will there be a huge performance benefit from using free directly instead of queue_free? I was thinking of it since there are no guarantees you need to make about its internal nodes, and only very rarely do you actually use free on a node directly.

It can be beneficial if you create lots of nodes after freeing previous ones. If they are freed immediately, the old and new nodes won't be in memory at the same time. It's rarely useful though.

authors: KoBeWi MewPurPur Mickeon RadiantUwU dalexeev djrain mhilbrunner


text:
Avoid saving EditorSettings on startup

Similar to #89248
EditorSettings were saved when launching, because of dialog update.

Thanks!

This seems to make the Editor Settings take a while to close for me, is it normal ?

Are you sure this PR is what caused it?
It's not expected, but slow was already a problem.

Are you sure this PR is what caused it? It's not expected, but slow was already a problem before.

I tried to do a checkout of the commit just before this one got merged, and the slowdown disappeared. Then checkout the merge of this one or everything after and the slowdown is back.
Before, closing the window by pressing the X button is instant, after it takes ~7seconds (on debug build). The editor also freeze for a bit after the editor settings window is not visible anymore.
The slowdown disappeared and got fixed for good by pressing the Close button instead of the X button of the window itself. Removing the editor_settings-4.4.tres will make the issue come back.

Open a new issue with a minimal project.

source: gharchive/pull-request

authors: AeioMuch KoBeWi akien-mga

repo: godotengine/godot

license: MIT

license_type: permissive

license_source: github-api


text:
Fix missing project date showing the unix epoch

Before:

After:

Bugsquad edit, fixes: https://github.com/godotengine/godot/issues/96274

Thanks!

Cherry-picked for 4.3.1.

authors: akien-mga passivestar


text:
[3.x] Cache results for TranslationServer.compare_locales()

Resolves #98228
The score for the match between two locales does not change with external circumstances. We can cache the result to avoid doing heavy string operations like split() repeatedly.
FPS is about seven times what it was before the change using the MRP here:

Build
Before
After

dev
3
23

release
30
210

This looks great! Do we know which PR caused this regression in performance out of interest?

compare_locales() was introduced by #61878.

Thanks!

authors: lawnjelly timothyqiu


text:
Installation doesn't check if the port specified is in use

Took me quite some time to figure out that the port I had specified (9000) was being used by an fcgi server.
It would be nice to have gogs try and see if the port is available and cry if it's not.

Thanks your feedback!
Gogs does print an error message if it fails to listen on given address and port. I think you just ignored the logs.

Yes, it does. But at boot time only, not when we have started it and are going through initial installation.

I see, I got your point now!

Hmm... I'm losing idea how to fix this. Any thoughts?

@Unknwon When we specify a port during installation, it should try to listen on that port, if it can't it should not let the user proceed

:D Glad you're still here.
Please see https://github.com/gogits/gogs/blob/master/cmd/web.go#L59
At some point I've implemented this.

That's awesome! Thanks!

:tada:

source: gharchive/issue

authors: Unknwon steelbrain

repo: gogits/gogs


text:
View issues assigned to nobody

is there currently a way to filter issues assigned to nobody?

Nope...

cheers, this would be quite useful

authors: Unknwon kellpossible


text:
Do not include line numbers in diff selection

Marking text in a diff will include the line numbers, resulting in copy pastes like this:
43	37
     /**
44
-     * @return array
38
+     * @inheritDoc
45	39
      */

This pull request will tell browser to not include the line numbers when selecting text. Copy paste will then behave like the standard code-view. This is not a problem in code-view, since the structure of the table rendering the code is a bit different.
This css property is widely supported. IE9 and lower is the only (most used) browsers not supporting it.
I could not find a relevant issue.

Thank you!

Hmm, just tested on demo site, it does not seem to work and found https://css-tricks.com/almanac/properties/u/user-select/.

However it's a bit buggy. ... WebKit still allows the text to be copied if you select elements around it.

noo :(
Github puts the line number in an attribute on . And uses css with the attr property.
HTML

CSS
.blob-num::before {
    content: attr(data-line-number);
}

I not familiar with the template syntax. But I guess one could modify the section_unified.tmpl and test this..

Thanks for the tip! Problem solved.  :+1:
Patch has been pushed to develop branch.

authors: Unknwon eigan


text:
Cannot use .Site.RegularPages.Related with index called "categories"; all other taxonomies work fine

What version of Hugo are you using (hugo version)?

$ hugo version
hugo v0.104.0+extended darwin/arm64 BuildDate=unknown

Does this issue reproduce with the latest release?
yes
The Issue
error calling Related: indexing currently not supported for index "categories" and type []interface {}

It corresponds to Line 295 of inverted_index.go; it seems categories is reserved keyword, defined as an interface?
Here is my YAML:
taxonomies:
  category: "categories"
  series: series
  tag: tags
  term: terms
  # authors: authors

related:
  includeNewer: false
  indices:
  - name: "categories"
    weight: 100
  - name: date
    weight: 10
  threshold: 80
  toLower: false

Note that if I replace "categories" with tags, or series, it works; only "categories" doesn't work.

I am unable to reproduce the problem. Try it:
git clone --single-branch -b hugo-github-issue-10346 https://github.com/jmooring/hugo-testing hugo-github-issue-10346
cd hugo-github-issue-10346
hugo server

Your example works, thank you!
I suspect I am using Hugo Modules and multilingual theme imported as module, something goes wrong...

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

I am using this theme, perhaps is redefines "categories" type somewhere but I can't find,
github.com/razonyang/hugo-theme-bootstrap

Thank you!

I was able to isolate the issue, I had one single malformed document (in about 10k):
categories:

  - ABC: xyz
  - Hello
  - World

It caused this error:
error calling Related: indexing currently not supported for index "categories" and type []interface {}

After fixing this, issue disappeared.
Perhaps we can add some pre-validation feature to catch "pre-compilation" errors? It was tricky to find single malformed document in 10,000.

authors: FuadEfendi jmooring

repo: gohugoio/hugo


text:
Pipe through markdownify

This has been shot down once but I'd like to try once more:
I have i18n content and in some places want to be able to do URLs on the front-page. We do this as follows:
reason4Text: "Blabla.
[Read more here](./why-pace/)"

{{ .Params.reason3Text | markdownify }}

It seems to me that content should be separated from logic. But if

[Read more here](./why-pace/)
is removed from the markdown the builds start to fail:
ERROR 2018/02/20 20:47:53 Error while rendering "home" in "": template: /Users/jensmunch/Documents/www.paceup.com/layouts/index.html:5:5: executing "main" at <partial "frontpage.h...>: error calling partial: template: partials/frontpage.html:37:55: executing "partials/frontpage.html" at <markdownify>: wrong number of args for markdownify: want 1 got 0

What's the best practice here...it doesn't make sense that one should vet content for purity.
J

This has been shot down once but I'd like to try once more:

Not sure what "shot it down" before, but I will post my standard reply to these:
Please use https://discourse.gohugo.io/ for questions/troubleshooting. Also see  Hugo Documentation.

Ok trying here..
https://discourse.gohugo.io/t/markdownify/10658
I'm super confused.

authors: bep jensamunch


text:
Javascript Minification Bug

What version of Hugo are you using (hugo version)?

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

Does this issue reproduce with the latest release?
Yes
Bug Report
With Hugo 0.86.0 (I think this might go back to at least 0.84.2), minification breaks some javascript.
Without minification hugo produces:
<script>
    $(document).ready(function() {
      if ($("#player").length) {
        const player = new Plyr('#player', {settings: ['speed']});
      }
    });
  </script>

But with hugo --minify it is reduced to
<script>$(document).ready(function(){if($("#player").length)})</script>

(Code is produced from this partial.)
(The variable declaration of the object is removed.)
As suggested in the above discourse thread, setting
[minify]
minifyOutput = true
disableJS = true

in the config gets around this issue, as does not passing --minify.

I am pretty sure this is due to player not being used anywhere else in the code. maybe console.log the player variable to see if it stays then. That should be an issue in the minifier library then, or maybe they even thought about unused-variables there and have some form of keepit-config-parameter for that.

@davidsneighbour you might be on to something there and I can see why it would do that. Though I don't see that feature listed in the minify library's README.
I'm pretty sure this behaviour is new(ish). But if the solution is my theme only works without minification, I can live with that.

https://github.com/tdewolff/minify/issues/428

@funkydan2 With tdewolff/minify v2.9.20 your code is minified to:
$(document).ready(function(){})

Closed with https://github.com/gohugoio/hugo/pull/8819.

authors: davidsneighbour funkydan2 jmooring


text:
Adding PR ettiquette for Contributions.

Checklist

[ ] New Extension
[ ] New Test
[ ] Changed more than one extension, but all changes are related
[x] Trivial change (doesn't require changelog)

          3 Messages

:book:
Executed 202 tests, with 0 failures (0 unexpected) in 6.190 (6.655) seconds

:book:
Executed 187 tests, with 0 failures (0 unexpected) in 4.419 (4.615) seconds

:book:
Executed 124 tests, with 0 failures (0 unexpected) in 4.295 (4.387) seconds

  Generated by :no_entry_sign: Danger

authors: EZSwiftExtensionsBot Khalian

repo: goktugyil/EZSwiftExtensions

license: mit

license_source: bigquery


text:
Swift 3 version

There are multiple ones, which one would you guys like to have implemented here?
https://github.com/goktugyil/EZSwipeController/pull/36
https://github.com/goktugyil/EZSwipeController/pull/37
https://github.com/goktugyil/EZSwipeController/tree/swift-3

Also I'm no longer working on a project that is using this library, so its hard for me to maintain it. Would you like to maintain this @JFKingsley ?

I'd be very happy to - I've found the project to be very useful in several of my recent projects.

I've added you as collaborator, good luck 😁

Swift 3 support merged in!

authors: JFKingsley goktugyil

repo: goktugyil/EZSwipeController


text:
[BUG] - devslog try to read unexported field which may cause panic.

Describe the bug
when formatting struct attributes, devslog try to read the unexported fields using reflect,
this behaviour cause panic.
To Reproduce
Given the codes below:
// play/ref/main.go
package main

import (
    "fmt"
    "reflect"

    "play/ref/subpack"
)

func main() {
    var s subpack.S

    fmt.Println(`hello world`)
    fmt.Println(reflect.ValueOf(s).Field(0).Interface())
}

// play/ref/subpack/subpack.go
package subpack

type S struct {
    unexported string
}

build and run it, program will panic with:
$ go run .
hello world
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 1 [running]:
reflect.valueInterface({0x100d175c0?, 0x100dd26a0?, 0x1400009cf18?}, 0x1?)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1501 +0xc8
reflect.Value.Interface(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1490
main.main()
        /Users/violin/Workspace/golang/play/ref/main.go:14 +0xc4
exit status 2

As for devlog, given the codes below:
package main

import (
	"context"
	"log/slog"
	"net"
	"os"

	"github.com/golang-cz/devslog"
	grpclog "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging"
	"google.golang.org/grpc"
	"google.golang.org/grpc/reflection"
)

func main() {
	slog.SetDefault(slog.New(devslog.NewHandler(os.Stderr, nil)))

	logger, opts := grpclog.LoggerFunc(func(
		ctx context.Context,
		lvl grpclog.Level,
		msg string, fields ...any,
	) {
		slog.Default().Log(ctx, slog.Level(lvl), msg, fields...)
	}), []grpclog.Option{grpclog.WithLogOnEvents(grpclog.PayloadReceived, grpclog.PayloadSent)}

	svr := grpc.NewServer(
		grpc.ChainUnaryInterceptor(
			grpclog.UnaryServerInterceptor(logger, opts...)),
		grpc.ChainStreamInterceptor(
			grpclog.StreamServerInterceptor(logger, opts...)))
	reflection.Register(svr)
	l, _ := net.Listen(`tcp`, `:8000`)
	svr.Serve(l)
}

when calling grpc, server will panic in devslog:
$ grpcurl -plaintext localhost:8000 list
panic: reflect.Value.Interface: cannot return value obtained from unexported field or method

goroutine 10 [running]:
reflect.valueInterface({0x104ef3ca0?, 0x14000030a50?, 0x19?}, 0x99?)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1501 +0xc8
reflect.Value.Interface(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/reflect/value.go:1490
github.com/golang-cz/devslog.(*developHandler).formatStruct(0x104ef3ca0?, {0x104f3c148, 0x104ef3ca0}, {0x104ef3ca0?, 0x14000030a50?, 0x104a35e14?}, 0x1)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:503 +0x80
github.com/golang-cz/devslog.(*developHandler).elementType(0x140001ad4d0, {0x104f3c148, 0x104ef3ca0}, {0x104ef3ca0, 0x14000030a50, 0x1b9}, 0x0, 0x10)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:541 +0xff0
github.com/golang-cz/devslog.(*developHandler).formatStruct(0x1051f491c?, {0x104f3c148, 0x104f07fc0}, {0x104f07fc0?, 0x14000030a50?, 0x0?}, 0x0)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:520 +0x714
github.com/golang-cz/devslog.(*developHandler).colorize(0x140001ad4d0, {0x1400011a800, 0x47, 0x400}, {0x14000294000, 0x9, 0x104d84be5?}, 0x0, {0x10527cea0, 0x0, ...})
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:336 +0x2174
github.com/golang-cz/devslog.(*developHandler).processAttributes(0x140001ad4d0, {0x1400011a800, 0x47, 0x400}, 0x140001c3048)
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:241 +0x290
github.com/golang-cz/devslog.(*developHandler).Handle(_, {_, _}, {{0xc1669326378f4058, 0x373936, 0x105247ec0}, {0x104d8d46a, 0x10}, 0x0, 0x104d84be5, ...})
        /Users/violin/Workspace/golang/play/ref/devslog/devslog.go:157 +0x2b0
log/slog.(*Logger).log(0x140000214a0, {0x104f35138, 0x140002881b0}, 0x0, {0x104d8d46a, 0x10}, {0x1400014b340, 0x12, 0x1c})
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/log/slog/logger.go:225 +0x15c
log/slog.(*Logger).Log(...)
        /opt/homebrew/Cellar/go/1.21.6/libexec/src/log/slog/logger.go:158
main.main.func1({0x104f35138?, 0x140002881b0?}, 0x1c?, {0x104d8d46a?, 0x140001c3578?}, {0x1400014b340?, 0x4?, 0x4?})
        /Users/violin/Workspace/golang/play/ref/main.go:23 +0xb0
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.LoggerFunc.Log(0x140001fa1c0?, {0x104f35138?, 0x140002881b0?}, 0x140001c3578?, {0x104d8d46a?, 0x104a27c0c?}, {0x1400014b340?, 0x104a274e4?, 0x104a3be90?})
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/logging/logging.go:202 +0x54
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.(*reporter).PostMsgReceive(0x14000119d60, {0x104f07fc0?, 0x14000030a50}, {0x0, 0x0}, 0x0?)
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/logging/interceptors.go:119 +0x550
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.(*monitoredServerStream).RecvMsg(0x14000288210, {0x104f07fc0, 0x14000030a50})
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/server.go:63 +0x9c
google.golang.org/grpc/reflection/grpc_reflection_v1.(*serverReflectionServerReflectionInfoServer).Recv(0x140000219f0)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/grpc_reflection_v1/reflection_grpc.pb.go:142 +0x58
google.golang.org/grpc/reflection.(*serverReflectionServer).ServerReflectionInfo(0x140001af980, {0x104f38288, 0x140000219f0})
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/serverreflection.go:259 +0x74
google.golang.org/grpc/reflection/grpc_reflection_v1._ServerReflection_ServerReflectionInfo_Handler({0x104ed71e0?, 0x140001af980}, {0x104f35f28?, 0x14000288210})
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/reflection/grpc_reflection_v1/reflection_grpc.pb.go:123 +0x98
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/logging.StreamServerInterceptor.StreamServerInterceptor.func2({0x104ed71e0, 0x140001af980}, {0x104f36000, 0x1400028a000}, 0x104e67c80?, 0x104f2e6e8)
        /Users/violin/.go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.1/interceptors/server.go:35 +0x250
google.golang.org/grpc.(*Server).processStreamingRPC(0x14000181000, {0x104f35138, 0x140002880c0}, {0x104f38510, 0x140001b29c0}, 0x14000001680, 0x140001afaa0, 0x105235cc0, 0x0)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1673 +0xec8
google.golang.org/grpc.(*Server).handleStream(0x14000181000, {0x104f38510, 0x140001b29c0}, 0x14000001680)
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1787 +0xc18
google.golang.org/grpc.(*Server).serveStreams.func2.1()
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1016 +0x5c
created by google.golang.org/grpc.(*Server).serveStreams.func2 in goroutine 9
        /Users/violin/.go/pkg/mod/google.golang.org/grpc@v1.60.1/server.go:1027 +0x138

This is because devlog is trying to read XXXRequest.state.atomicMessageInfo.GoReflectType.t, which is type of reflect.Type
Expected behavior
formatStruct() method should ignore and skip the unexported fields in struct, for reading an unexported field of other package is dangerous and useless.

Resolved with #31

repo: golang-cz/devslog


text:
Warn on use of abbreviated sha1 commit hash

Update manifest validation to warn when an abbreviated version of a
sha1 commit hash is used.
Add test for warn.
Fixes #567

@zknill Can you also correct this typo I introduced in the test file along with this PR? 😅

Yeah absolutely!

@carolynvs the validation logic we use there is originally inherited from me - @jessfraz is in the blame b/c she discovered that i was totally wrong about the final component of the revid being hex-encoded 😄
No, there's no reason we don't use a regex. Before we go down that path, though...maybe it'd be best to look through the bzr codebase to find where they validate the shape of these identifiers, and mimic that directly?

this is how bzr generate new rev ids
I'm not able to find the code that validate the format of revids yet.

The tests validates revids through regex.

Also, I would recommend storing the result of regexp.MustCompile() calls instead of generating a new one for each check.

OK, my apologies, but I'm just actually thinking this all through now properly, and I've realized that we're kinda going about this the wrong way. 😦
First, we've disregarded svn here in a way that might be problematic. svn revision numbers will eventually be able to go into this field, then the algorithm as-written will mistake those numbers for abbreviated hex-encoded SHA1s. So, that's no good. We need to narrow the scope of our search to a particular, expected form, rather than trying to validate the field more broadly.
To that end, we actually need to be looking for just one specific pattern - a 7-char long (git's abbrev length) or 12-char long (hg's abbrev length, I believe) string containing only [0-9a-f], with at least one character being in [a-f] (so as to disambiguate from actual numbers). My suggestion about using hex decoding to validate won't work, as that necessarily can't work on an odd-length string.
Being that we've already done the work to figure out how to validate bzr revs, though, let's not throw that work away. Let's open up a separate PR in which we apply it to deduceConstraint(), where we actually are trying to pattern-match these inputs.

Ok, I agree this seems wrong and / or problematic
However if the ultimate goal is to ensure that the full revision id from [insert vcs] is given in the manifest, I struggle to see how matching just the most common abbreviations will satisfy that goal?
I am not super familiar with the code base, and I don't know if it's even possible, but it feels to me that it would be better to know which vcs we were using at the point at which we validate the revision, and ensure that we are using the correct validation rules for that vcs. Else you are right, it does feel a little like blindly guessing at what's acceptable.

I am not super familiar with the code base, and I don't know if it's even possible, but it feels to me that it would be better to know which vcs we were using at the point at which we validate the revision, and ensure that we are using the correct validation rules for that vcs.

Yes, this definitely would make it easier. Getting that information, though, isn't zero-cost:

If we allow the user to explicitly state it (e.g. with a vcs field), then it a) could, and will, be wrong, introducing a new failure mode and b) becomes yet one more annoying thing that the user has to specify.
If we want to infer it for these validation purposes, then now we have a network dependency for some of our most basic validation routines. Avoiding network access where we can is a major way that we can avoid making the tool slooooow.

Fortunately, we can make these changes now, then improve them down the line - say, once we have #431 and can reliably avoid network access to get vcs type. (Note that we actually probably could do this all locally with the information we cache now, but because , I think it may be better to not try to tackle this until after #431)

Ok nice! Thanks for the explanation. So plan going forward? Closing this and opening another with changes to match the specific cases you laid out above?
Should that logic live in validateManifest or else where?

@sdboyer  I'm not sure about this part:

To that end, we actually need to be looking for just one specific pattern - a 7-char long (git's abbrev length) or 12-char long (hg's abbrev length, I believe) string containing only [0-9a-f], with at least one character being in [a-f] (so as to disambiguate from actual numbers). My suggestion about using hex decoding to validate won't work, as that necessarily can't work on an odd-length string.

In git it's not uncommon to get commits where the first 7 (or more) chars are all digits. see https://github.com/ibrasho-forks/dep/commit/28792060519c And I assume the same happens in hg.
Unless I misunderstood your point...

Ok nice! Thanks for the explanation. So plan going forward? Closing this and opening another with changes to match the specific cases you laid out above?

Sure 😄 I'd say this PR should implement the character-counting solution I outlined; if it's easier for you to move that to a separate PR, though, we can close this and do that.

Should that logic live in validateManifest or else where?

Yep, that's still the right place.

In git it's not uncommon to get commits where the first 7 (or more) chars are all digits. see ibrasho-forks/dep@2879206 And I assume the same happens in hg.

Unless I misunderstood your point...

Right - assuming a perfectly random distribution, it would happen (10/16)^7 = 3.7% of the time. My thinking here is that it's worse to have false positives than miss a small percentage of cases, especially when this is a relative rare situation we're guarding against, anyway.

@zknill have you decided? would you rather close this and start fresh, or just push up revised commits to this PR?

just making the changes, I will post a new PR close this one and link to the other

Closing in favour of #671

authors: darkowlzz ibrasho sdboyer zknill

repo: golang/dep


text:
cmd/internal/gc: swapping two elements of a slice of structs generates suboptimal code

Swapping two elements of slice is commonly used to implement sort.SortInterface. However, for the same struct size the speed depends heavily on the struct fields. Swapping 8 bytes two by two is 380% slower than swapping 8 bytes all at once.
package issue10137
import "testing"

var s8 [2]struct{a, b, c, d, e, f, g, h int8}
func BenchmarkCopy8(b *testing.B) {
        for i := 0; i < b.N; i++ {
                s8[0], s8[1] = s8[1], s8[0]
        }
}

var s16 [2]struct{a, b, c, d int16}
func BenchmarkCopy16(b *testing.B) {
        for i := 0; i < b.N; i++ {
                s16[0], s16[1] = s16[1], s16[0]
        }
}

var s32 [2]struct{a, b int32}
func BenchmarkCopy32(b *testing.B) {
        for i := 0; i < b.N; i++ {
                s32[0], s32[1] = s32[1], s32[0]
        }
}

var s64 [2]struct{a int64}
func BenchmarkCopy64(b *testing.B) {
        for i := 0; i < b.N; i++ {
                s64[0], s64[1] = s64[1], s64[0]
        }
}

go version devel +7b0c73a Wed Mar 11 12:53:24 2015 +0000 linux/amd64

PASS
BenchmarkCopy8  1000000000               2.14 ns/op
BenchmarkCopy16 300000000                5.14 ns/op
BenchmarkCopy32 300000000                4.59 ns/op
BenchmarkCopy64 2000000000               1.35 ns/op

Note that if I replace swapping with assignment the timing difference disappears.

The fields are copied piece by piece so that they can be registerized. If this appeared in a larger program, it could be a speedup. In this function, it's not. I don't think we're going to revisit this now. When the new SSA-based code generator is in, I'm sure we'll have all new performance issues.

authors: brtzsnr rsc

repo: golang/go


text:
misc/cgo/testcshared: cgo testp2 fails on OS X

With commit 467a2cb73ca867ccdb1dd22be220fa51b6d52491, the ../misc/cgo/testcshared test fails on MacOS 10.10.5 (14F27).
The problem is that testp2 gives random output instead of PASS. It seems like fd 10 is open and used by MacOS for something else at the time we read from it, so we get that garbage instead of the data that libgo2 is trying to send on it. When I change to fd 100, the problem goes away.
Not saying this is a real fix, just that this points to what the problem is.
diff --git a/misc/cgo/testcshared/main2.c b/misc/cgo/testcshared/main2.c
index 4023383..fbdbc48 100644
--- a/misc/cgo/testcshared/main2.c
+++ b/misc/cgo/testcshared/main2.c
@@ -9,7 +9,7 @@
 #include <time.h>
 #include <unistd.h>

-#define fd (10)
+#define fd (100)

 // Tests libgo2.so, which does not export any functions.
 // Read a string from the file descriptor and print it.
diff --git a/misc/cgo/testcshared/src/libgo2/libgo2.go b/misc/cgo/testcshared/src/libgo2/libgo2.go
index 6096860..6e513f0 100644
--- a/misc/cgo/testcshared/src/libgo2/libgo2.go
+++ b/misc/cgo/testcshared/src/libgo2/libgo2.go
@@ -21,7 +21,7 @@ import (
 // that the C code can also use.

 const (
-	fd = 10
+	fd = 100
 )

 func init() {

@jeffallen, commit 467a2cb was a change to register allocation in the arm64 compiler. But you are reporting a problem on OS X 10.10.5, which does not run on arm64. So perhaps you meant a different commit? I would be interested to know when it started if you can git bisect it (perhaps the bug is in 10.10.5 and any version fails, but that would be good to know too). Thanks.

FWIW, I don't understand why fd 10 would already be open (@ianlancetayor or @mwhudson?) but it's certainly plausible that the C library could have fd 10 open, in which case changing to fd 100 does not seem like such a bad thing.

On a GNU/Linux system I would say that it is impossible for file descriptor 10 to be open (unless you are invoking all.bash/run.bash in an unusual way).  However, I have no idea what MacOS may do.  I'm fine with changing the 10 to 100 if that fixes the real problem.

Also, I tried to reproduce this on my OS X machine and I can't even run the test. So something is odd.
$ go test
2015/11/30 14:36:09 executing go install -installsuffix=5577006791947779410 -buildmode=shared runtime sync/atomic failed exit status 1:
-buildmode=shared not supported on darwin/amd64
exit status 1
FAIL	_/Users/rsc/go/misc/cgo/testshared	0.122s
$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.10.5
BuildVersion:	14F1021
$

@rsc this is about testcshared, not testshared

I just tested and can't repro with:
$ pwd
/Users/jra/go/misc/cgo/testcshared
$ ./test.bash
$ go version
go version devel +1dae473 Tue Dec 1 10:39:06 2015 +0000 darwin/amd64
$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.10.5
BuildVersion:	14F1021

authors: ianlancetaylor jeffallen mwhudson rsc


text:
database/sql and net/http deadlock hangs on linux

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.6.2 linux/amd64

What operating system and processor architecture are you using (go env)?

Ubuntu 16.04 64-bit and alpine 3.3
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/user/work"
GORACE=""
GOROOT="/usr/local/go"
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GO15VENDOREXPERIMENT="1"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0"
CXX="g++"
CGO_ENABLED="1"

What did you do?

I am building a database/sql driver. I noticed that on linux machines (tested with Ubuntu and alpine), the go test tool will deadlock and timeout (after 10 minutes).
The code is available in this repo: https://github.com/F21/deadlock
driver.go:
package deadlock

import (
	"database/sql"
	"database/sql/driver"
	"errors"
	"fmt"
	"net/http"
)

type httpClient struct {
	httpClient *http.Client // <- Do not need to wait for timeout if we remove this struct member and do not import net/http
}

type conn struct{}

func (c *conn) Prepare(query string) (driver.Stmt, error) {
	return nil, errors.New("prepare() not supported")
}

func (c *conn) Close() error {
	return nil
}

func (c *conn) Begin() (driver.Tx, error) {
	return nil, errors.New("Begin() not supported")
}

func (c *conn) Exec(query string, args []driver.Value) (driver.Result, error) {

	items := []int{1, 2}
	fmt.Println(items[2]) //Should cause a panic

	return nil, errors.New("error!")
}

type Driver struct{}

func (d *Driver) Open(dsn string) (driver.Conn, error) {

	conn := &conn{}

	return conn, nil
}

func init() {
	sql.Register("deadlock", &Driver{})
}

driver_test.go:
package deadlock

import (
	"database/sql"
	"testing"
)

func TestExecDeadlock(t *testing.T) {

	db, err := sql.Open("deadlock", "someserver:1234")

	if err != nil {
		t.Fatalf("error connecting: %s", err.Error())
	}

	defer db.Close()

	db.Exec("some statement")
}

tool/main.go:
package main

import (
	"database/sql"
	"fmt"
	_ "github.com/F21/deadlock"
)

func main() {
	db, err := sql.Open("deadlock", "someserver:1234")

	if err != nil {
		fmt.Println(err)
	}

	defer db.Close()

	db.Exec("some statement")

	fmt.Println("Done!")
}

driver.go contains a bug on line 32 that will cause a panic.
If we run go test on the root of the repo, it will hang and will only terminate until it reaches the 10 minute timeout.
If we run the command: go run tool/main.go we also see that it hangs forever.
The interesting thing is that if we remove lines 11 to 13 in driver.go and remove the net/http import, it fails immediately and gives us an error: fatal error: all goroutines are asleep - deadlock!
On Windows (tested on Windows 10 64-bit), doing the above does not hang and it fails immediately.

What did you expect to see?
The deadlock should definitely not hang. It should fail fast like on Windows. However, rather than having an error about a deadlock, I was expecting an error about the panic.

What did you see instead?
The deadlock hangs on linux and fails immediately on Windows. After timing out on Linux, we get this output:

SIGQUIT: quit
PC=0x460891 m=0

goroutine 0 [idle]:
runtime.futex(0x9deba8, 0x0, 0x0, 0x0, 0x0, 0x9de150, 0x0, 0x0, 0x40f3c4, 0x9deba8, ...)
	/usr/local/go/src/runtime/sys_linux_amd64.s:306 +0x21
runtime.futexsleep(0x9deba8, 0x0, 0xffffffffffffffff)
	/usr/local/go/src/runtime/os1_linux.go:40 +0x53
runtime.notesleep(0x9deba8)
	/usr/local/go/src/runtime/lock_futex.go:145 +0xa4
runtime.stopm()
	/usr/local/go/src/runtime/proc.go:1538 +0x10b
runtime.findrunnable(0xc82001c000, 0x0)
	/usr/local/go/src/runtime/proc.go:1976 +0x739
runtime.schedule()
	/usr/local/go/src/runtime/proc.go:2075 +0x24f
runtime.goexit0(0xc820001680)
	/usr/local/go/src/runtime/proc.go:2210 +0x1f9
runtime.mcall(0x7ffea7a9f320)
	/usr/local/go/src/runtime/asm_amd64.s:233 +0x5b

goroutine 1 [chan receive]:
testing.RunTests(0x8a7148, 0x9d59a0, 0x1, 0x1, 0xc82008d201)
	/usr/local/go/src/testing/testing.go:583 +0x8d2
testing.(*M).Run(0xc82004bef8, 0x8a7998)
	/usr/local/go/src/testing/testing.go:515 +0x81
main.main()
	github.com/F21/deadlock/_test/_testmain.go:54 +0x117

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:1998 +0x1

goroutine 5 [semacquire]:
sync.runtime_Semacquire(0xc82001a1e4)
	/usr/local/go/src/runtime/sema.go:47 +0x26
sync.(*Mutex).Lock(0xc82001a1e0)
	/usr/local/go/src/sync/mutex.go:83 +0x1c4
database/sql.(*driverConn).closeDBLocked(0xc82001a1c0, 0x0)
	/usr/local/go/src/database/sql/sql.go:326 +0x3e
database/sql.(*DB).Close(0xc8200cc160, 0x0, 0x0)
	/usr/local/go/src/database/sql/sql.go:528 +0x195
panic(0x7894c0, 0xc82000a0d0)
	/usr/local/go/src/runtime/panic.go:443 +0x4e9
github.com/F21/deadlock.(*conn).Exec(0x9fada8, 0x80e2d0, 0xe, 0x9fada8, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/home/user/work/src/github.com/F21/deadlock/driver.go:32 +0x242
database/sql.(*DB).exec(0xc8200cc160, 0x80e2d0, 0xe, 0x0, 0x0, 0x0, 0xc820029e01, 0x0, 0x0, 0x0, ...)
	/usr/local/go/src/database/sql/sql.go:1035 +0x2c2
database/sql.(*DB).Exec(0xc8200cc160, 0x80e2d0, 0xe, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
	/usr/local/go/src/database/sql/sql.go:1009 +0xbe
github.com/F21/deadlock.TestExecDeadlock(0xc820018120)
	/home/user/work/src/github.com/F21/deadlock/driver_test.go:18 +0x1e8
testing.tRunner(0xc820018120, 0x9d59a0)
	/usr/local/go/src/testing/testing.go:473 +0x98
created by testing.RunTests
	/usr/local/go/src/testing/testing.go:582 +0x892

rax    0xca
rbx    0x0
rcx    0x460893
rdx    0x0
rdi    0x9deba8
rsi    0x0
rbp    0x1
rsp    0x7ffea7a9f178
r8     0x0
r9     0x0
r10    0x0
r11    0x286
r12    0x8
r13    0x8a5d3d
r14    0x9
r15    0x8
rip    0x460891
rflags 0x286
cs     0x33
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (10m0s).
FAIL	github.com/F21/deadlock	600.006s

Thank you for proving a detailed repo. I believe there is a shadowing error in sql.exec around line 1030
        if execer, ok := dc.ci.(driver.Execer); ok {
                dargs, err := driverArgs(nil, args) // shadows outer err
                if err != nil {
                        return nil, err
                }
                dc.Lock()
                resi, err := execer.Exec(query, dargs)
                dc.Unlock()
                if err != driver.ErrSkip {
                        if err != nil {
                                return nil, err
                        }
                        return driverResult{dc, resi}, nil
                }
        }

This shadowing may be preventing the defer of putConn to work properly.

#13677 may also be related.

@F21 Thanks for the clear test case.  I agree with @davecheney that the shadowing is problematic, but I don't think that's the problem here.  I think the deadlock/hang is occurring because of the defer db.Close().  That is run when the panic happens.  It tries to close all the connections.  Unfortunately, in doing so, it tries the lock them, but the connection is already locked.  I will send a CL.

The CL has too many changes, and this case was already broken.  Postponing until 1.8.

authors: F21 davecheney ianlancetaylor


text:
autocert should trim trailing dots

What did you do?
When accessing a server that gets certificates using autocert via a url like https://server.domain.com./ (note the trailing dot) you get an error from the ACME server. See letsencrypt/boulder#2367 for why. They propose that clients should resolve this situation by removing the trailing dot in the certificate request.
Thus, autocert should trim trailing dots.

Thanks. Seems easy enough.
/cc @x1ddos

Sent https://go-review.googlesource.com/33711 ... reviews welcome!

Yeah, sounds good. I'll create a change.
Although, hard to imagine someone typing a url with a trailing dot... Out of curiosity, does it happen regularly in your case? Never happened to me or visitors of domains that I own.

Ah, Brad already did.

For the record, absolute FQDN that ends with a dot is not permitted for SNI of TLS. See https://tools.ietf.org/html/rfc6066#section-3 and #14404.

Oh, yeah? So which browser is sending it, @jeffallen, and why are we accepting it, @agl?

When I hacked autocert to nuke the dot, what I saw is that the URL bar had a dot on the end, but I still had a green padlock even though the SNI had no dot on the end. Don't recall if it was ff or chrome.

We shouldn't accept this. Too late for the current cycle, but this should do it in the future.

authors: agl bradfitz jeffallen mikioh x1ddos


text:
strip binaries

Do the Go developers have an opinion on stripping binaries and libraries?
http://nurmi-labs.blogspot.com/2017/08/bootstrapping-go.html
Generally SlackBuild scripts will contain lines like the following after the
executables and dynamic libraries are compiled. The go source however
already contains some executables, so if only the newly generated files
were to be stripped, I might modify the build adding code to strip those
files in bin and libstd.so.
find $PKG -print0 | xargs -0 file | grep -e "executable" -e "shared object" |
grep ELF | cut -f 1 -d : | xargs strip --strip-unneeded 2> /dev/null || true
NB: This question is intended to pertain solely to source downloads.

We don't the issue tracker to ask questions. Please see https://golang.org/wiki/Questions for good places to ask. Thanks.

authors: davecheney forskning


text:
cmd/compile: untyped types reaching backend

I created CL 94027, which validates that the SSA backend never sees untyped types. This should be safe, because the compiler frontend should eliminate all untyped types during type checking.
However, the CL doesn't currently work, suggesting the frontend is doing something (at least technically) wrong.

Fixing this might be tricky, but an easy way to help would be to find some minimal test cases that reproduce failures.

Apply CL 94027. (Maybe change the two f.Fatalf calls to f.Logf, so you still have a working compiler.)

Run "go install cmd/compile" and then "go build -a std cmd".

Look in the output for lines like:
 ../src/strconv/atoi.go:160:15: internal compiler error: allocating value with type untyped bool

Figure out how to reproduce that error with as little code as possible. For example, this is a pretty minimal repro case (maybe it can be further simplified):
 package p

 func syntaxError(... interface{}) error

 func ParseUint(s string) (uint64, error) {
         if len(s) == 0 {
                 return 0, syntaxError("", s)
         }
         return 0, nil
 }

The example above is caused by the generated .eq.[2]interface{} method.

@fraenkel Thanks. So just
package p
type t [2]interface{}

repros that issue.
So further steps for folks interested in helping debug/fix this:

Run "go tool compile -W repro.go" and look for "untyped bool". (Make sure you're sync'd past c3e8da67dd79d84ac04187b5ce577d76d4e0c032, or else this will be printed as just "bool".)

The output is a bit tricky to understand if you're not familiar with the compiler's AST representation (see cmd/compile/internal/gc/syntax.go for hints), but you should be able to spot things like:
.   FOR l(1) colas(true) tc(1) ARRAY-[2]interface {}
.   .   LT l(1) tc(1) untyped bool
.   .   .   NAME-p..autotmp_4 a(true) l(1) x(0) class(PAUTO) esc(N) tc(1) assigned used int
.   .   .   NAME-p..autotmp_5 a(true) l(1) x(0) class(PAUTO) esc(N) tc(1) assigned used int

or
.   .   IF l(1) tc(1)
.   .   .   OROR l(1) tc(1) hascall untyped bool
.   .   .   .   NE l(1) tc(1) untyped bool
.   .   .   .   .   ITAB l(1) tc(1) PTR64-*uintptr

These two cases reveal that during typechecking we're not forcing if or for conditions to a concrete type.

Find the appropriate places to add x = defaultlit(x, types.Types[TBOOL]). This will probably be right after the corresponding x = typecheck(x, Erv). For example, in typecheck1's 'case OIF', I'd try putting it right after the n.Left = typecheck(n.Left, erv).

Fixed the OIF and OFOR. Do you want separate CLs?
The next one(s) identified are:
.   .   .   .   .   ANDAND l(6) tc(1) hascall untyped bool
.   .   .   .   .   .   ANDAND l(6) tc(1) hascall untyped bool
.   .   .   .   .   .   .   NE l(6) tc(1) hascall untyped bool
.   .   .   .   .   .   .   EQ l(6) tc(1) hascall untyped bool
.   .   .   .   .   .   CONVNOP l(6) tc(1) hascall untyped bool
.   .   .   .   .   .   .   LITERAL-true a(true) l(6) tc(1) untyped bool

Separate CLs is great. Thanks for working on this!

With these last 3 CLs, there are no more reported issues.
I had changed the original CL to issue Warnl instead of Fatalf so I could see all the issues.

A bit stumped on how to fix the "last" issue I can see.
They are all showing up as:
:1: allocating value with type untyped bool
But I cannot seem to easily identify what the generated code is.

I know where the last fix needs to go and I have an ugly hack to make it all work but just seems wrong.
After the very last finishcompare in walkcompare, we need to fix up the types. The problem is that its conditional because of the OCONVNOP that is injected. It also needs to use defaultlit2.

But I cannot seem to easily identify what the generated code is.

You can try adding ssa.Func's Name field to the logging output. It's probably code generated by alg.go (which makes me think the issue is the ORANGE->OFOR loop lowering that I mentioned in CL 97856).

I know where the last fix needs to go and I have an ugly hack to make it all work but just seems wrong.

If you're able to identify minimized test cases, I can probably help identify where the missing defaultlits are.

Its the eq func for pipe using the following:
type atomicError struct{ v interface{} }
type pipe struct {
rerr atomicError
werr atomicError
}

Ohh, I think the issue is OCONVNOP doesn't do type propagation like I thought it does.
If you change finishcompare to just:
func finishcompare(n, r *Node, init *Nodes) *Node {
r = typecheck(r, Erv)
r = walkexpr(r, init)
r = conv(r, n.Type)
return r
}

it looks to handle that test case. (As an actual fix, I'd suggest adding a convnop helper function that does the same thing as conv, but makes sure typecheck turns the newly introduced OCONV into OCONVNOP or OLITERAL.)
Does that handle all remaining cases?

Well, that just ends badly....
<autogenerated>:1: cannot convert type..eq."".scase(<node INDREGSP> = .autotmp_3, <node INDREGSP> = .autotmp_4) (type bool) to type untyped bool
<autogenerated>:1: cannot convert !type..eq."".mcentral(<node INDREGSP> = .autotmp_6, <node INDREGSP> = .autotmp_7) (type bool) to type untyped bool
<autogenerated>:1: cannot convert !runtime.memequal(<node INDREGSP> = .autotmp_6, <node INDREGSP> = .autotmp_7, <node INDREGSP> = 40) (type bool) to type untyped bool

Added a defaultlit and all looks better.

My last CL will get rid of untyped bools in all sample codes.

authors: fraenkel josharian mdempsky namusyaka


text:
sync: eliminate global Mutex in Pool operations

In the review for https://golang.org/cl/101715 (“regexp: use sync.Pool to cache regexp.machine objects”), @ianlancetaylor notes:

A similar change was made in https://golang.org/cl/44150043, by [@bradfitz].  It was rolled back in  https://golang.org/cl/48190043. [@dvyukov] said: "The remaining concern is: Are there cases where a program creates lots of local short-lived regexps (one-shot), and it's not possible to replace them with global ones? For this case we've introduced a global contended mutex."

I'm guessing that comment refers to allPoolsMu here:
https://github.com/golang/go/blob/2e84dc2596f5ca655fd5716e1c277a801c868566/src/sync/pool.go#L242

It strikes me as odd that sync.Pool should have a global Mutex at all. After all, sync.Pool explicitly cooperates with the garbage collector (and will likely cooperate even more after #22950), and the garbage collector will trace all of the live sync.Pool instances in the course of a normal garbage collection.
The allPools slice appears to exist so that the garbage collector can clear all of the sync.Pool instances before tracing them. We have to identify Pools before tracing them so that we don't over-retain pooled objects, but it seems like we could do that without acquiring global locks beyond the usual GC safepoints.
For example, we could add newly-allocated pools to a per-G list, and move that list to the global list only when the thread running that G acquires the scheduler lock.
If we happen to miss a new Pool on the current GC cycle (for example, because its goroutine wasn't descheduled until near the end of the cycle), that's ok: we'll just wait until the next GC cycle to clear it.
That could make sync.Pool substantially more efficient for objects that tend to be long-lived but are sometimes short-lived too (such as compiled regexps).

(See also #24411, the motivating example.)

For example, we could add newly-allocated pools to a per-G list

I tried to solve the problem today.
My guess is that per-G list is a P local store, so there are two ways to implement it.
Scheme 1 is to modify the runtime to support, scheme 2 is to get a function that returns pid from the runtime via linkname, and use this function to create P local storage.
I tried plan 2, if you use map to implement P local storage, you still need a lock to synchronize. You can also try to implement P local storage with slicing.
Finally found that perhaps can use similar to the https://go-review.googlesource.com/c/go/+/552515 API, use two sharded to solve the problem.
Note: If GOMAXPOCS change, sharded is not 100% guaranteed that the same P operates use the same memory, and different Ps operate use different memory. This means that using sharded simply makes the lock no longer global, making the competition for the same lock less intense.
These are my attempts.
I shared it to help solve the issue.

authors: bcmills qiulaidongfeng


text:
x/build/cmd/gopherbot: add command to ping owners of the package

It would be good to ping owners of the package in the ticket without remembering who is the owner of it. What if one could say:
@gopherbot please ping owners
@gopherbot please ping owners to critique the solution

And then gopher would produce something like:
@name has asked @firstowner @secondowner to review this issue
@name has asked @firstowner @secondowner to critique the solution

Should this be semiautomatic? If the issue does not mention anybody and nobody has replied after 1 week then bot should trigger this automatically.

I think this was raised earlier (the "gopherbot pinging owners automatically" part), although I can't find it now.

Seems to me that once I've been mentioned in an issue, every comment on the issue sends me an e-mail.  So I'm not sure what an explicit gopherbot ping command would do that isn't already done by simply commenting on the issue.

authors: agnivade ianlancetaylor ysmolsky


text:
x/tools/cmd/goimports: removes necessary imports with certain characters

Using goimports at release-branch.go1.10, I see it dropping necessary imports when they have - and are not aliased,
You can see it in this removal with a hyphen:
https://github.com/knative/eventing/pull/653/files#diff-abdff22f055e921908e64cdb584fa75cL25
... and this removal with a dot:
https://github.com/knative/docs/pull/611/files#diff-06a9a211281c9004b5638fc5207899d8L32
I can't use HEAD or 1.11 because of: https://github.com/golang/go/issues/29041

Hi @mattmoor - Could you provide us with a self-contained code sample which reproduces this behavior ? I tried this myself but wasn't able to reproduce it.

I can't use HEAD or 1.11 because of: #29041

I believe that is a different issue than this, right ? Is there a reason you cannot use the tip goimports to test this bug ?
/cc @bradfitz

I just tried with HEAD (d0ca3933b724e6be513276cc2edb34e10d667438) and I am still unable to reproduce it.
$git clone https://github.com/knative/docs /tmp/docs && cd /tmp/docs
Cloning into '/tmp/docs'...
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (7/7), done.
remote: Total 4597 (delta 0), reused 0 (delta 0), pack-reused 4590
Receiving objects: 100% (4597/4597), 13.36 MiB | 2.34 MiB/s, done.
Resolving deltas: 100% (2401/2401), done.
12:28:36-agniva-/tmp/docs$
12:31:41-agniva-/tmp/docs$go run golang.org/x/tools/cmd/goimports -w eventing/samples/iot-core/generator/main.go
12:31:46-agniva-/tmp/docs$git st
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean
12:31:56-agniva-/tmp/docs$git diff eventing/samples/iot-core/generator/main.go
12:32:53-agniva-/tmp/docs$

Could you please attach the output of go env ?

Weird, so I cannot reproduce this when github.com/knative/docs is on ${GOPATH}, but I can reproduce this if the repo is simply cloned into /workspace.

Yeah. In general, if a package's name doesn't match its import path, goimports is going to have to load it. In GOPATH mode, that means having it on GOPATH. tip goimports now adds a name to the import in these situations, which prevents the problem.
$ GOPATH=/tmp/foo go get -d github.com/knative/docs
package github.com/knative/docs: no Go files in /tmp/foo/src/github.com/knative/docs
$ cd /tmp/foo/src/github.com/knative/docs
$ goimports -w eventing/samples/iot-core/generator/main.go
$ git diff
diff --git a/eventing/samples/iot-core/generator/main.go b/eventing/samples/iot-core/generator/main.go
index 813e2b0..018e9b0 100644
--- a/eventing/samples/iot-core/generator/main.go
+++ b/eventing/samples/iot-core/generator/main.go
@@ -27,9 +27,7 @@ import (
        "math/rand"
        "time"

-       "github.com/dgrijalva/jwt-go"
        MQTT "github.com/eclipse/paho.mqtt.golang"
-       "github.com/satori/go.uuid"
 )

 const (
$ git reset --hard
HEAD is now at 098b76e move eventing to 0.2.1 release (#629)
$ GOPATH=/tmp/foo goimports -w eventing/samples/iot-core/generator/main.go
$ git diff
diff --git a/eventing/samples/iot-core/generator/main.go b/eventing/samples/iot-core/generator/main.go
index 813e2b0..052ad2d 100644
--- a/eventing/samples/iot-core/generator/main.go
+++ b/eventing/samples/iot-core/generator/main.go
@@ -27,9 +27,9 @@ import (
        "math/rand"
        "time"

-       "github.com/dgrijalva/jwt-go"
+       jwt "github.com/dgrijalva/jwt-go"
        MQTT "github.com/eclipse/paho.mqtt.golang"
-       "github.com/satori/go.uuid"
+       uuid "github.com/satori/go.uuid"
 )

 const (
$ git commit -a -m foo
[master f0aefbd] foo
 1 file changed, 2 insertions(+), 2 deletions(-)
$ goimports -w eventing/samples/iot-core/generator/main.go
$ git diff
$

@agnivade: you probably had a copy of the package in your GOPATH already when you ran goimports.
I don't think there's anything we can do to improve the situation further, so I'm closing this. Comment if you disagree.

@heschik I worked around this by symlinking the repo onto GOPATH.  I will try tip when I get a chance.  thanks.

authors: agnivade heschik mattmoor


text:
cmd/go: can't build go1.10 from source using go1.12

What version of Go are you using (go version)?
Go 1.12
Does this issue reproduce with the latest release?
yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/matloob/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/matloob/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/f4/9p58ddnj40x58zchdb36p7lr004_sl/T/go-build724269079=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
I have Go 1.12 installed on my system. I cloned the go repo and checked out the tag go1.10.8.
Then i cd'd to the go repo's src directory and ran make.bash
What did you expect to see?
a successful build
What did you see instead?
matloob-macbookpro2:src matloob$ ./make.bash
Building Go cmd/dist using /usr/local/go.
go: cannot determine module path for source directory /Users/matloob/go110 (outside GOPATH, no import comments)
But make.bash does work when GO111MODULE is unset

@bcmills

We should fix this. We want old go trees to keep building with new ones as bootstrap. cmd/dist is a single-directory main package binary.

I think this ties closely to #32027: we're erroring out because we do not have a module path (and therefore do not have a package path) for package in the working directory.
In theory we could proceed anyway: since the package in the current directory imports only standard-library packages, the only effects of its go.mod file would be to select the go version in use and set accurate debug information, and for most binaries those don't matter much.

authors: bcmills josharian matloob rsc


text:
Go test breaks on flags in go 1.13

What version of Go are you using (go version)?

$ go version
go version go1.13 darwin/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/michaelt/Library/Caches/go-build"
GOENV="/Users/michaelt/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/michaelt/workspace/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/opt/go/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/k5/mznl8tb12q953v9qmgbhw40m0000gn/T/go-build311187106=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?
Run go test from within a package with test files
What did you expect to see?
Expected the tests to run just as in version 1.12
What did you see instead?
➜  core git:(master) go test
flag provided but not defined: -test.timeout
Usage of /var/folders/k5/mznl8tb12q953v9qmgbhw40m0000gn/T/go-build162470207/b001/core.test:
  -acceptance
    	If we should run acceptance tests
  -integration
    	If we should run integration tests
  -unit
    	If we should run unit tests
exit status 2
FAIL	example/pkg/core	0.025s

Duplicate of #31859.


text:
proposal: crypto/tls: expose TLS alert type for more precise error checks

What version of Go are you using (go version)?

$ go version
go version go1.13.3 darwin/amd64

Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (go env)?
go env Output
$ go env
GO111MODULE="auto"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/me/Library/Caches/go-build"
GOENV="/Users/me/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/me/Code/Golang"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/16/7jt3pcs15570rmz95wf9vlrc0000gn/T/go-build533656385=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

I've hooked http.Server.ConnState using a function that checks for handshake errors on the http.StateClosed state, and logs these. For example:
func ConnStateHook(c net.Conn, cstate http.ConnState) {
	if cstate == http.StateClosed {
		if cc, ok := c.(*tls.Conn); ok {
			tlsState := cc.ConnectionState()
			if err := cc.Handshake(); err != nil {

				if strings.Contains(err.Error(), "remote error: tls: unknown certificate authority") || strings.Contains(err.Error(), "remote error: tls: unknown certificate") {
					// Do something
				}
			}
		}
	}
}

What did you expect to see?
This works, but I really don't like using string comparison on error messages. What would be helpful to ensure accurate comparison (especially if these strings change in the future) is to be able to compare the error with the tls.alert types. The error above (err) is actually a tls.alert wrapped in a *net.OpError type, but because the tls.alert type is not exported, I only get to see the public Error() method that outputs a string.
The result of this would be similar to the http status codes found in net/http/status.go.

CC @FiloSottile @agl @kevinburke

Hi guys, are there any plans to implement this in a next release?

Suprised this isnt already exposed. String matching here is cumbersome

cc @golang/security

bump. this would be super helpful 👍 !

authors: TheHackerDev bcmills domdom82 process0 riraccuia seankhliao


text:
cmd/go: Run "go build" in an directory which contains only go.mod (no other go files) returns error: can't load package unknown import path

What version of Go are you using (go version)?

go version go1.12.2 linux/amd64

Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (go env)?
go env Output
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

I want to put several common modules into one repo.
So the repo layout looks like below:
common-modules/
        |-- module1/
                |-- *.go files
        |-- module2/
                |-- *.go files
        go.mod

The go.mod file looks like:
module company1.com/common-modules

go 1.12

Run "go build" under directory common-modules.
What did you expect to see?
No error is returned.
What did you see instead?
If I run "go build" under directory common-modules, error returns:
can't load package: package company1.com/common-modules: unknown import path "company1.com/common-modules": cannot find module providing package company1.com/common-modules

CC @bcmills @jayconrod

go version go1.12.2

I believe we fixed some related bugs in 1.13. What happens if you try this with the current release (Go 1.13.4)?

Hi @bcmills
The result is the same  for 1.13.4:
[myname@mycompany myproject]$ ll
total 4
drwxrwxr-x. 5 myname myname 100 Nov 11 10:40 fe3
-rw-rw-r--. 1 myname myname  47 Nov 11 10:40 go.mod
drwxrwxr-x. 6 myname myname 107 Nov  7 20:04 ll3
[myname@mycompany myproject]$ go build
build .: cannot find module for path .
[myname@mycompany myproject]$ cat go.mod
module git.mycompany.com/cloud/myproject
go 1.13
[myname@mycompany myproject]$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myname/.cache/go-build"
GOENV="/home/myname/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myname/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/gorootpath/go-1.13.4"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/gorootpath/go-1.13.4/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myname/myproject/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build343362267=/tmp/go-build -gno-record-gcc-switches"
[myname@mycompany myproject]$ go build
build .: cannot find module for path .

authors: GingerMoon bcmills ianlancetaylor