proposal: build: define standard way to recognize machine-generated files

Abstract
I propose Go creates a standardized format, which would enable code-generating tools to reliably communicate to humans and other machine tools that the output is in fact a generated file. Additionally, Go should add a recommended style for a simple code generated disclaimer (which satisfies the above criteria).
Proposed Definition
A file is considered to be "generated" if and only if the maintainer(s) of the project consider it a non-canonical source. In order to make long-term changes to such files, another source must be modified, and the file in question is then fully (re)generated by a reproducible machine tool.
A distinguishable property of generated files is that they can be deleted and re-generated with a zero diff.
Background
One of the strong values that Go brings are conventions and best practices that reduce bikeshedding, increase consistency and readability across diverse teams of Go programmers. Having a well defined convention, format, or standard for things that are unimportant to the key task, but need to have some value saves time.
During Gopherfest SV 2015, Rob Pike gave a talk Go Proverbs (video, bullet-point summary) that mentioned:

Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.

To expand on that, there are many examples for things that have have a recommend format/style in Go that let you simply reuse that and not force you (and other people) to invent your own style:

gofmt for Go code formatting. https://golang.org/cmd/gofmt/
Variable naming. https://github.com/golang/go/wiki/CodeReviewComments#mixed-caps
Package comments. https://github.com/golang/go/wiki/CodeReviewComments#package-comments
Package names. https://github.com/golang/go/wiki/CodeReviewComments#package-names
Example func naming. https://godoc.org/testing#hdr-Examples
Build constraints. https://godoc.org/go/build#hdr-Build_Constraints
Struct tag strings. https://godoc.org/reflect#StructTag

Description
There is one type of comment which is commonly used, but has no existing well-defined officially suggested style recommended by Go.
It is a comment that most tools that generate Go code tend to write somewhere at the top of the code.
There are currently many variations of such disclaimer headers in the wild, and they often vary insignificantly (in spacing, punctuation, etc.). New variations come to be when authors look at how other tools do this, see a large variance, end up picking their favorite and tweaking it.
Consider the following examples in the wild:
// generated by stringer -type Pill pill.go; DO NOT EDIT

// Code generated by "stringer -type Pill pill.go"; DO NOT EDIT

// Code generated by vfsgen; DO NOT EDIT

// Created by cgo -godefs - DO NOT EDIT

/* Created by cgo - DO NOT EDIT. */

// Generated by stringer -i a.out.go -o anames.go -p ppc64
// Do not edit.

// DO NOT EDIT
// generated by: x86map -fmt=decoder ../x86.csv

// DO NOT EDIT.
 DO NOT EDIT.

// DO NOT EDIT. This file is generated by mksyntaxgo from the RE2 distribution.

// GENERATED BY make_perl_groups.pl; DO NOT EDIT.

// generated by mknacl.sh - do not edit

// DO NOT EDIT ** This file was generated with the bake tool ** DO NOT EDIT //

// Generated by running
//  maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt
// DO NOT EDIT

/*
* CODE GENERATED AUTOMATICALLY WITH github.com/ernesto-jimenez/gogen/unmarshalmap
* THIS FILE SHOULD NOT BE EDITED BY HAND
*/

This creates 2 problems.

It's a problem for authors of code generator tools. Such authors need to spend time figuring out what format of a disclaimer header they want to use; there is no canonical standardized format.
It's a problem for authors of tools that want to be able to use the information (in a helpful way) whether certain files are generated or not. There is no simple implementation that will catch all of the variations above, and there is no standardized machine-readable format that they can detect.

This leads to circular arguments and PRs/CLs. For example, see the discussion and the change itself at https://go-review.googlesource.com/#/c/15073/. It started from https://github.com/github/linguist/blob/473282d/lib/linguist/generated.rb#L241, which led to CL 15073. That lead to https://github.com/shurcooL/vfsgen/commit/b2aab1c20b69a97790ee44d826c7f44baa852382 and https://github.com/shurcooL/go/commit/43b216689b91e7e2d2c6b7dd0971ad4625384a81. But the initial GitHub behavior came from protobuf disclaimers.
I've created the following func to try to answer the question if a file is generated. At this time, it uses heuristics and best-effort to tell if a file is generated. https://github.com/shurcooL/go/blob/master/analysis/generated_detection.go If there was a well defined requirement for tools to follow, this code can be made simpler and more reliable. Ideally, that helper should be moved into external library for people to reuse, and for generator tools that wish to be compliant to be able to use it for verification.
Goals
The goals of this proposal are twofold.
Primarily, to resolve the current impossibility of reliable communication between code generator tool output, and tools that try to determine if a file is code generated.
There should be a way for code generator tool authors to be able to express in their generated output that the file is generated, such that it's possible to reliably detect if a file is generated by other tools.
Secondarily, for code generator authors that simply don't care about what their disclaimer header looks like, provide a recommended style (that satisfies the first condition) template to use.
The implementation details should be defined in a design doc.
Non-goal
It is a non-goal to figure out how existing tools should choose to use or not use the fact whether a given file is generated.
There is some fear that if it's possible to determine if a file is generated reliably, then tools that display code differences will hide generated code differences. That is absolutely the choice of the tool, and in my opinion it should not enforce any behavior that users are unhappy with.
Having additional information (whether a file is generated or not) should enable tools to offer better user experiences - it should not cause tools offer worse experiences than currently.
This proposal focuses solely on enabling code generator tool authors that wish to use a standard disclaimer header to do without forcing them to invent their own format, and for tool authors that wish to make use of information whether a file is generated or not to be able to use that information as they wish. Details of how they do that is outside the scope.
Conclusion
By not standardizing a way for those two types of tools to communicate, it leads to ad-hoc solutions that are sub-optimal emerging, as can be seen above. Go has an opportunity to de-fragment this space and create a recommended standard format that will resolve the needs above, and allow people to migrate existing tools to use the specified format.
Once there's a standard, it's easy to begin updating existing tools towards it over time, and new generator/other tools can start relying on it.
Meta-disclaimer
I expect coming up with a recommended style may likely cause a lot of bikeshedding. However, I think it's a cost that's worth incurring, to go through this process once, so that we can avoid having to continuously suffer it while there's no standard at all. I personally don't care too much about what the actual format is (as much as I do about resolving the higher level problems described); I'm okay with whatever Go authors come up with. Any standard is better than no standard at all.

This proposal comes from the result of discussion in CL 15073. /cc @bradfitz @dominikh

How about // DO NOT EDIT. Generated by $GENERATOR_NAME_OR_COMMANDLINE on the first line of the file?

@zellyn, I think we need to agree that this is a good idea to standardize before we pick paint colors. :)

@bradfitz Agreed. Just curious why the proposal didn't include a strawman proposal for format. :-)

I think a good and simple way to approach it would be to add somewhere (Go style guide, or a more internal document) a section that says something like this:

If a code generator tool wishes to mark a file as "generated" in a way so it can be reliably understood by other tools to be generated, then it must satisfy these criteria:

{{Simple rule to follow.}}
{{Another simple rule that must be followed.}}

As an example, the following disclaimer will satisfy both the criteria above:
{{Example disclaimer template.}}

That way, people who don't want to be creative can just copy/paste that example disclaimer template and use it.
However, if someone wants to add more details or make their disclaimer different, they can still do that. They just need to follow rules 1 and 2 so that it can still be recognized by tools as a generated disclaimer header.
I think putting it that way makes this workable. I don't think it's possible to enforce a single header format that will satisfy everyone's needs/wants and have people be okay with it. But providing simple rules that the header will follow and giving an example would work splendidly.

/cc @hyangah for gobind generated files.

This proposal sounds like a good idea. We should use the same string that is recognized by GitHub.

Seems like a reasonable way to address the problem that tools that IDEs depend on tools that are too fussy about generated code.

And what about people writing in languages that are not, er, English? :P
There could be syntax for "this is machine readable". Off the top of my head, a special form of comment; Rust does this for embedding tests within doc-blocks.
/// ... the triple slash
//! ... double-slash-bang

What actual machine-readable text should be used to denote a generated file is wider issue. This quickly explodes in scope and potential bloat in the future -- e.g. Rusts function decorations.

And what about people writing in languages that are not, er, English? :P

That's a valid point, and I think it should be considered.

We should use the same string that is recognized by GitHub.

The outcome may happen to be a string that is recognized by GitHub with their current code in linguist, but I respectfully disagree with this being a high priority criteria (if that's what was implied, I'm not sure).
As I mentioned in the proposal, what GitHub came to recognize as a Go generated file had a high degree of luck and variance in it (and circular arguments):

This leads to circular arguments and PRs/CLs. For example, see the discussion and the change itself at https://go-review.googlesource.com/#/c/15073/. It started from https://github.com/github/linguist/blob/473282d/lib/linguist/generated.rb#L241, which led to CL 15073. That lead to https://github.com/shurcooL/vfsgen/commit/b2aab1c20b69a97790ee44d826c7f44baa852382 and https://github.com/shurcooL/go/commit/43b216689b91e7e2d2c6b7dd0971ad4625384a81. But the initial GitHub behavior came from protobuf disclaimers.

Specifically, see these two PRs:

https://github.com/github/linguist/pull/2152/files
https://github.com/github/linguist/pull/2426/files

Notice the motivation of those PRs (first one was to detect protobuf generated files, and the second was "modeled after the first"). Also notice how easy it was to get them merged in.
Now, maybe we got lucky with the above sequence of events and what github recognizes today is a great format. But if not, I think it's a relatively low effort followup to submit a PR to GitHub's linguist – similar to those 2 PRs above – to make any necessary corrections after this proposal is resolved and Go has an officially recommended and recognized way to indicate that a file is generated.

Seems like a reasonable way to address the problem that IDEs that depend on tools are too fussy about generated code.
robpike self-assigned this 23 days ago

I am very happy with that outcome and I trust whatever Rob Pike comes up with is going to be a great resolution for this issue.

And what about people writing in languages that are not, er, English? :P

I don't see this as a big issue. Autogenerated files are usually not meant for being read by people, and I don't know how someone would write Go code without being able to read English anyway.

Not happening in 1.8

How about using the file extension ".gen.go" (or suffix _gen.go) for generated files, instead of standardising comments? This had two advantages:

The format of the comment is irrelevant (and as such language independent)
There is even no need to open the file to check if the file is generated, both by humans and by tools. This will also make tools faster (such as golint) which can skip the file immediately based on filename, instead of forcing them to open the file and inspect the source code for comments.

The benefit of standardizing on a comment is that fewer tools would need to be changed, as most tools already generate comments that can be matched by a common phrase. Virtually no tool, however, uses a gen suffix.

@stanim - That's a non-starter. Many generator tools, including multilingual ones, already make decisions about how to name their files. One I know personally is that the protocol buffer compiler uses .pb.go.
So a comment it will be.

Having standard suffixes makes attaching build tags (GOOS and GOARCH)
to file names impossible.
Also, it will make generating test files impossible.
If we must use file names to label generated files, I prefer the standard
library's
way of prefix the filename with "z", e.g. used in syscall and runtime
packages.

Actually, I was just thinking today of suggesting that generated files could be zipped; i.e. ".gogz"; go doc could still read these, but they wouldn't be readable in a normal text-editor, a good way of hinting that you shouldn't be touching the file and that the contents are not important outside of the API (go doc, if used)

No, please no. Generated code is just as valid as code written by humans.
Please don't hide it, punish it, or otherwise denigrate it to a lower
status just because it wasn't typed directly by a human.
On Wed, 26 Oct 2016, 05:23 Kroc Camen<EMAIL_ADDRESS>wrote:

Actually, I was just thinking today of suggesting that generated files
could be zipped; i.e. ".gogz"; go doc could still read these, but they
wouldn't be readable in a normal text-editor, a good way of hinting that
you shouldn't be touching the file and that the contents are not important
outside of the API (go doc, if used)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/golang/go/issues/13560#issuecomment-256123320, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAcA280spISObDFUpOGMki-jDR6hhiPks5q3kkAgaJpZM4GyXqy
.

@robpike, why can't this happen during Go 1.8? Is there more to do here than pick a phrase?

It can happen in Go 1.8 if someone else wants to take it on, but I don't have the bandwidth in this cycle.

Just wondering, why is the choice of a magic phrase even tied to a given Go release? Sure, generated source-code in the standard library won't be updated yet, but at least the rest of tooling can get started on this.

@dsnet, so we could update our headers to conform to the decision?

@dsnet Because releases are how we plan timelines.

Here is my suggestion for how to do this.
The text must be a // comment, and that comment must be either the first or the second comment in the file. (The first because that's best, the second because some organizations insist on copyright notices first.)
The comment must match the regular expression (in Go syntax):
`^// Code generated .* DO NOT EDIT$'
The .* means the tool can put whatever folderol it wants in there, but the comment must start with Code generated and end with DO NOT EDIT. Perhaps the regexp match should be case insensitive.
This seems simple and adequate to me, and not coincidentally would cover stringer.
Bikeshed away.

Works for me.

I think we should require "🚲🏠" in the text somewhere...

I think 🏚 might be better ducks

Is it okay that the text DO NOT EDIT might appear offscreen if word-wrap is off and the tool decides to have a long message?
E.g., I'm trying to imagine how this would be rewritten to match that guideline:
// Generated by running
//  maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt
// DO NOT EDIT

Would it be?
// Code generated by running maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt DO NOT EDIT

Notice that DO NOT EDIT is not visible.
Some tools try to include all flags so it's easier to reproduce the generated file (including stringer itself. However, I imagine they can work around that by putting the rest of the details on another line. Something like https://github.com/golang/go/blob/b443babad49f90e9507d91819736c97a7495e308/src/net/http/h2_bundle.go#L1-L2.
So, it can instead be written as:
// Code generated by maketables; DO NOT EDIT
// maketables --tables=all --data=http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt DO NOT EDIT

I'm okay with that if @robpike is, just wanted to ask the first question that comes to mind.

// Code generated by the following command. DO NOT EDIT
// maketables --tables=all --data=
http://www.unicode.org/Public/8.0.0/ucd/UnicodeData.txt --casefolding=
http://www.unicode.org/Public/8.0.0/ucd/CaseFolding.txt

@minux, that wouldn't work (if I understand it right) because of:

The comment must be a single line and not part of another comment.

On Thu, Feb 2, 2017 at 12:51 AM, Dmitri Shuralyov<EMAIL_ADDRESS>wrote:

@minux https://github.com/minux, that wouldn't work (if I understand it
right) because of:
The comment must be a single line and not part of another comment.
Did Rob actually imply that?

I think it's pretty common to add more notices under this warning.

It's a little unclear to me what exactly "not part of another comment" means. Aren two // comments on two lines next to each other considered one comment? E.g.:
// Foo reports
// whether something is true.
func Foo() bool {

Is that one or two line comments? godoc parses it as one "logical" comment...
Perhaps he meant that it shouldn't be a part of a general comment (aka block comment)? I.e.:
/* start of a general comment

// Code generated ... DO NOT EDIT

end of a general comment */

I think some clarification would be helpful.

It should probably also support /* Code generated by go run gen.go DO NOT EDIT */ just for completeness.

Regexp should end
DO NOT EDIT\.?$

since some people believe that DO NOT EDIT is a sentence and should be treated as such.

@carlmjohnson Why? We are defining a format, not writing a pattern to match every whim of the code generator.
@jba: Yes, you're right.

@shurcooL By this, I mean the comment should have blank lines before and after.  It should stand alone.

Updated proposal:
The text must appear as the first line of a properly formatted Go // comment, and that comment must appear before but not be attached to the package clause. /* */ comments are not OK. We are defining a format and we choose to define it simply.
The comment line must match the case-sensitive regular expression (in Go syntax):
`^// Code generated .* DO NOT EDIT.?$'
The .* means the tool can put whatever folderol it wants in there, but the comment must start with Code generated and end with DO NOT EDIT, with an optional period.

One more tweak: To be like the rules for build tags, and for easier processing, I've added an extra detail: There must be no /* comment before it.

The updated proposal SGTM.
Just for clarification, does it mean that copyright notices must appear
after the magic comment or appear before in /**/ block comments?

@minux: Yes, a condition that also applies to build tags.

The clarification confuses because it's different wrt build tags:

A build constraint, also known as a build tag, is a line comment that begins
// +build
that lists the conditions under which a file should be included in the package. Constraints may appear in any kind of source file (not just Go), but they must appear near the top of the file, preceded only by blank lines and other line comments. These rules mean that in Go files a build constraint must appear before the package clause.

src
Ie, "copyright notices must appear
after the magic comment or appear before in /**/ block comments" seems to not be a requirement for build tags. Particularly, the copyright header may precede build tags in '//'-style comments. Example.

@cznic Sorry, the response to @minux was confusing. You notice I did not edit the updated proposal.
There can be no /* comment before the generator comment, even a Copyright notice.

Thanks for clarification.

Given the updated proposal seems to be well received so far, it's probably a good time to try an implementation that can parse that specification and see how well it works out. It's possible that when implementing a prototype, we might learn something previously unforeseen [0], and now's the best time to iterate on the specification.
This original scope of the proposal was primarily about establishing a specification; a parser implementation can be easily followed. But given there's only one specification, this is a good case where a canonical parser used by many people makes the most sense.
[0] dotGo 2016 - Robert Griesemer - Prototype your design! https://www.youtube.com/watch?v=vLxX3yZmw5Q

a parser implementation can be easily followed.

So far I thought this proposal is not connected to the parser in the sense that go/parser should recognize/is going to recognize the "generated by robots" signature.

By parser, I mean something that can return a bool stating whether the specified Go file has a signature matching a generated file.
I don't know if that'll be a part of go/parser or not. I don't think anyone has discussed implementation details yet, or where the code for it will live. My point was to try it soon and see what happens once some working code is written.

I'm not sure what you're asking for. This proposal is as much for tools (such as IDEs) written in other languages as it is for tools written in Go. The "parser" is a few lines of line-oriented code. If the parser assumes gofmt input, it's trivial.

It completely slipped my mind that there will be tools written in something other than Go that'll want to parse this. In that case, you're right, there will be more than 1 canonical implementation.
Still, it might be insightful to try and implement this at least for Go. I might give it a shot, because I will need to update https://github.com/shurcooL/go/blob/16c2a662/analysis/generated_detection.go#L17 anyway.
I agree it's easy to detect the header, but I'm not sure how easy it is to avoid false positives, and how to handle tricky/ambiguous edge cases. Perhaps it's not very important because real world occurrences should be pretty unambiguous.

My 3 cents, what about having // +autogenerated with the same placement rules as // +build?

Definitely a 👍  on @robpike's comment-based proposal here (thanks for raising in the first instance @shurcooL).
To touch on @stanim's comment from earlier. Presumably the main reason against doing anything filename-based is not that existing tools (like the protoc compiler) have already established "conventions" of their own, i.e. the .pb.go suffix, but that other tools/systems already depend on these conventions. The assumption is that far fewer tools are reliant on the structure of generated files (particularly comments), hence the comment-based approach is the most practical way forward? I say this only to highlight that changing one's tool/generator to be compatible in terms of an output comment is surely just as easy as changing the format of the generated filename? All that nitpicking said, I repeat my support for the comment-based proposal because it does indeed seem like the most practical way forward for the reasons outlined.
@stanim, there is nothing to preclude you naming files however you like, whether they be the output from your own generators or indeed the renamed output from tools like protoc. We prefer to have a common prefix gen_* for all generated files (Go or otherwise) to:

visually distinguish when browsing a file system
marking such files as binary in tools like git so that they are ignored from default diff comparisons
allowing editors/other tools to present a listing of generated files differently, for example a slightly greyed out italicised text
...

There are plenty of good reasons to my mind to do things at a filename level (not least because such an approach is language/format agnostic), but I'd tend to agree with @robpike, that ship has probably sailed so as to make a generally accepted solution for Go impossible (assuming that's what you meant, @robpike)

Ok, I've gone ahead and attempted to create a Go implementation for the latest proposed specification from https://github.com/golang/go/issues/13560#issuecomment-277804473:

The text must appear as the first line of a properly formatted Go // comment, and that comment must appear before but not be attached to the package clause and before any /* */ comment. This is similar to the rules for build tags.
The comment line must match the case-sensitive regular expression (in Go syntax):
`^// Code generated .* DO NOT EDIT\.$`

The .* means the tool can put whatever folderol it wants in there, but the comment must be a single line and must start with Code generated and end with DO NOT EDIT., with a period.

I had two priorities when implementing it:
// The first priority is correctness (no false negatives, no false positives).
// It must return accurate results even if the input Go source code is not gofmted.
// The second priority is performance. The current version is implemented
// via go/parser, but it may be possible to improve performance via an
// alternative implementation. That can be explored later.

As the comment says, I started out by implementing this by using go/parser package, since @robpike mentioned that in https://github.com/golang/go/issues/13560#issuecomment-278184985.
I wouldn't say it was "trivial", but it was relatively easy (current implementation is ~35 lines).
I found the specification relatively easy to read and understand, and it was mostly clear. There were two things that I was initially confused/unsure about, but later on I think I figured them out.

"The text must appear as the first line of a properly formatted Go // comment". What does "properly formatted Go // comment" mean? What would be an example of an improperly formatted Go // comment?
At first I had no idea, but then I realized that it's probably referring to gofmt formatting. So, the only example of an improperly formatted Go // comment I can think of is something that contains whitespace before the //:
            // This is an improperly formatted Go // comment.

I was a little uncertain if there were any restrictions on what follows the line comment. Specifically, can there be a /* */ general comment right away? What about improperly formatted ones (with whitespace at the beginning of lines)? Based on the discussion above, I think that it's fine, as long as it's not a part of the package comment. So, I consider this to be a positive match:
// Code generated by tool; DO NOT EDIT.
// More information can go here.
/* Or here. */

// Package comment.
package p

Finally, the current go/parser-based implementation is okay, but I suspect a better approach is to implement this from scratch. Since we only need to parse up to the package clause, there can only be line comments, general comments, and whitespace, so it's relatively easy. I expect such an implementation would be faster, and I think parts of the specification would actually be even easier to implement this way. But, I haven't started thinking about that yet, and I haven't even profiled current implementation.
The implementation is here:
https://github.com/shurcooL/go/pull/28
I'd really welcome feedback and review.

The implementation is here:
shurcooL/go#28
I'd really welcome feedback and review.

Using go/parser for this is an overkill. We need to inspect first few lines of the source, parsing it all and building the AST is a [sometimes huge] waste of resources. I would avoid even go/scanner, FWIW. Just read line by line and detect the ones we're interested in. All that's needed is a trivial state machine with like 3-4 states, I guess, to decide if the source conforms to the simple specification provided earlier in this thread.

Just read line by line and detect the ones we're interested in.

The code used in go generate may be a useful reference point here (it avoids parsing and scanning)

So ... still absolutely no consideration for the several billion people not speaking English, am I right?

@Kroc With all due respect - programming was, is and will be English-centric. And I say that as a person from non-English world. English is lingua-franca of computer science since its birth and there's no point in arguing with that.
Besides - how many of those "several billion people not speaking English"

Writes a code in Go (with, to the grand extend, English-only documentation)
Is advanced enough to use or write a code generator
Does not understand big, fat, DO NOT EDIT?

I agree, that this solution is not perfect, bo laguage used in the massage is not an issue.

Using go/parser for this is an overkill. We need to inspect first few lines of the source, parsing it all and building the AST is a [sometimes huge] waste of resources.

I agree, see I mentioned this in my comment and new package documentation. But I just want to check, did you see that I used parser.PackageClauseOnly mode? It's documented as:
const (
	PackageClauseOnly Mode = 1 << iota // stop parsing after package clause

But I just want to check, did you @cznic see that I used parser.PackageClauseOnly mode?

No, I did not see that, sorry. That solves the fear of parsing the source when it is a monster-sized one, which generated code sometimes is. My apologies.
My choice would still be to not use the parser, not the scanner, not even regexp (strings.{HasPrefix,HasSuffix} is enough and you're already doing that :+1: )

Could just have // Code generated by followed by whatever the code generator wants to throw in there, I'm not sure you necessarily need the do not edit part.
People should be able to recognise that it's a generated file and ponder over whether they really want to be messing with the code in a generated file.

I've merged shurcooL/go#28, and so the github.com/shurcooL/go/generated package is available for early testing now, if anyone wants to.
There's still a known TODO to decide on how to deal with .go files that contain syntax errors.
Aside from that, there are no known issues, so please let me know if you find any. I'll consider working on a faster low-level version (that doesn't go through go/parser) next, but it's not high priority for me.

I think it's a mistake for this proposal to require knowing anything about Go syntax.
It should be confined to a pattern being matched on the first N lines only, ignoring everything about packages or package comments, etc.

While I understand your position, I'm not sure I agree. I like the fact that the current design matches the way build tags work, so there is a single consistent way of describing how to find them. Also, if you make it the first N lines, what is N? It's tricky because some companies require large copyright boilerplate on every file that must come first. I do not want to bikeshed N; the package delimiter works well in preventing that.
But it could be redefined easily to say "in the first 100 lines, if one matches this pattern (same as above) it is treated as machine-generated."

I think it's a mistake for this proposal to require knowing anything about Go syntax.

Furthermore, a syntactically incorrect  .go file (i.e. that cannot be go/parser-ed) that has been generated is still a generated file. Just because it's syntactically incorrect doesn't make it any less generated.

But it could be redefined easily to say "in the first 100 lines, if one matches this pattern (same as above) it is treated as machine-generated."

@robpike is there any reason to even specify N? We could bring this proposal in line with the go generate spec such that any line matching the pattern constitutes a match, anywhere in the file.

All right, fine. I like that and there is a precedent. New proposal, having almost no affect on existing practice:
Updated proposal:
Generated files are marked by a line of text that matches the regular expression, in Go syntax:
^// Code generated .* DO NOT EDIT\.$
The .* means the tool can put whatever folderol it wants in there, but the comment must be a single line and must start with Code generated and end with DO NOT EDIT., with a period.
The text may appear anywhere in the file.

SGTM, thanks.

One small suggestion: moving DO NOT EDIT somewhere before the tool-specific string makes the whole construction parseable without regular expressions (the final dot should be dropped to, though).
Also, dropping the word "code" might make sense, because tools can generate arbitrary files, such as configs or data tables.
Here is the sample (without Unicode and long string handling):
func GeneratorString(r *bufio.Reader) ([]byte, bool) {
	prefix := []byte("// DO NOT EDIT: generated by ")
	for {
		line, _, err := r.ReadLine()
		if err != nil {
			return nil, false
		}
		if bytes.HasPrefix(line, prefix) {
			return line[len(prefix):], true
		}
	}
}

You don't need regular expressions to find a string at the end of a line.
The bike shed is painted a lovely color now.

is there any reason to even specify N? We could bring this proposal in line with the go generate spec such that any line matching the pattern constitutes a match, anywhere in the file.

The go generate spec says to look anywhere in the file, and I think it's reasonable to expect to read through the entire Go package source code when running go generate on it.
But I think it's a shame if one would have to read the entire .go file to be able to say if it's machine generated or not. Imagine a directory with 10 .go files, each one is 5 MB. To be able to answer "how many of these 10 .go files are machine generated", with the new proposal, one would have to read and scan through 50 MB of bytes.
If there was a guarantee that the "// Code generated by ... " header was before package main, it would be able to stop earlier on each file.
Worse yet, think about the humans. A human can open a 5 MB (note, I'm using 5 MB as an example, it's an arbitrary number) .go file and it's no longer sufficient for them to look at the comments above package clause to be sure whether the file is generated or not. The "// Code generated by ... " clause can now be anywhere in the file, even the very last line. Is that really friendly?
There's a lot of precedent in putting information that should be "quick to access" at the top of the file and limiting where it can be. The import declarations are all before top level declarations. The package clause is always before all import declarations. The // +build constraints are always above package clause. go generate has different goals and does not apply.

But I think it's a shame if one would have to read the entire .go file to be able to say if it's machine generated or not. Imagine a directory with 10 .go files, each one is 5 MB. To be able to answer "how many of these 10 .go files are machine generated", with the new proposal, one would have to read and scan through 50 MB of bytes.

Only if the comment is at the end of the files. If it's at the top, the scanner should bail out as soon as it finds the comment.

I guess if the files were hand generated and so didn't have a comment, it would require a full scan.

Yes, the worst case is when all files are hand written.

It should be confined to a pattern being matched on the first N lines only, ignoring everything about packages or package comments, etc.

What if N = 1, i.e., the "// Code generated by ..." comment must be the first line. That's quick for computers and humans to detect. It's simple to implement and doesn't require knowing Go syntax.
It's only a problem if there's something else that must be placed on top of "// Code generated by ..." comment. Can anyone think of such a thing? The "// Copyright ..." comments can be placed below the generated comment, right?
If something new comes along in the future that must be first line, then the scope can be increased. It's really hard to go back from "The text may appear anywhere in the file."

@shurcooL, 50 MB is nothing. And that 50 MB will need to be read by the compiler anyway, so it'll already be in cache. Let's stop making up hypotheticals.
I've created https://golang.org/s/generatedcode to link to (for now) Rob's comment above.
If somebody wants to document it elsewhere I can update the link, but I can't think of where. The wiki seems like it would just be edited by somebody without discussion.

I'm going to close this bug for now. I consider it decided.

@robpike / @bradfitz with all due respect, I think we should have it done before the package declaration. That way we don't digress with the "what's N" discussion, or how big a source is or anything like that. And for some tools that just need to quickly scan the files so that their representation for them can be generate. Tthink of github for example, or your favorite directory browsing tool or your favorite code editing tool which are the first one ever to read the file after the file is removed from cache or wasn't there to begin with. Thank you.

@dlsniper, we want tools not written in Go and without access to go/* parsing packages to recognize these files easily.

we want tools not written in Go and without access to go/* parsing packages to recognize these files easily.

For reference, this is what github/linguist, written in Ruby, currently does (it was done before this issue existed):
https://github.com/github/linguist/blob/c39d7fd6e892f4dd1e86ba215bb425ef679fceb6/lib/linguist/generated.rb#L262-L267

@dlsniper @shurcooL - how do you determine if the special comment is before the package declaration if we agree it's not a requirement that the .go file is syntactically correct?
Indeed how do you define what a package declaration is in a syntactically incorrect .go file?
Placing syntactic requirements on the location of such comments complicates not only the specification but also any implementation as @bradfitz just highlighted. And more than that it just feels like overkill.

When linguist is updated, they can also fix the off-by-one error.

@bradfitz / @myitcv I'm not saying that we should have any validation against it by these tools (and I have the same use-case in mind as you said, tools that are not written by go/). Having the artificial: "Must be before package declaration" doesn't mean it must be enforced by any tool at all nor that non-go/ tools must understand what package means.
For example, this is a tool which searches for //go:generate  and it recently got support for a feature for based on this proposal. It doesn't matter what tool it is.
But if you think that it's at odds with the restriction having it anywhere in the file...

@bradfitz  As far as tools that do have access to the go/* packages, will there be a Generated bool added to *ast.File or something like that so tools can figure out whether something came from a generated file and can be ignored?

@jimmyfrasche, I'd open a separate bug for that. I have no opinion.

@bradfitz I started to, but as I was writing I realized it's a trivial amount of code to check an individual file is generated by looping over Comments and checking them one by one.
There doesn't seem to be an easy way to go from a token.Pos (which almost everything deals in) to an *ast.File to do that check, though. It's not too difficult to keep that map if you're only parsing one package time, but you still have to build a map of *token.File to *ast.File for a given *token.FileSet by matching the file names.
Short of adding a way to get the *token.File from the *ast.File and the *token.FileSet from the *token.File I don't see a way around that, and that seems like a lot of churn for something that most people won't run into, so I don't think there's anything to do.

@jimmyfrasche Adding a bool to an *ast.File requires that comments always be read, even if the parser mode is not set as such (which is a significant cost); or, alternatively, the flag may not be correct if a file was parsed without parsing comments, which seems worse.
I'm not sure how the token.Pos values come into play here: if you actually parse a file and collect comments, it's sufficient to just check those comments for the string and then you know whether that specific file was generated or not (if one of the comments matches). What am I missing?

@griesemer

if you actually parse a file and collect comments, it's sufficient to just check those comments for the string and then you know whether that specific file was generated or not (if one of the comments matches)

I don't think that works in all situations, does it?
Consider:
https://play.golang.org/p/VPe1Gp2zVm
This file is not considered to have been generated according to the proposal but would be by simply checking the *ast.File's comments (because the *ast.CommentGroup's and *ast.Comment's don't have knowledge of whether they are file-level or not). But maybe I misunderstood?

Generated files are marked by a line of text that matches the regular expression, in Go syntax:
^// Code generated .* DO NOT EDIT.$
The .* means the tool can put whatever folderol it wants in there, but the comment must be a single line and must start with Code generated and end with DO NOT EDIT., with a period.

There's a problem with this - it is quite likely that code-generation code becomes mistaken for generated code.:
package main

import "io/ioutil"

func main() {
	ioutil.WriteFile("generated.go", []byte(`
// Code generated by foo. DO NOT EDIT.
package foo

func main() {
}
`), 0666)
}

@rogpeppe the same problem exists with go generate directives.
In both cases the problem is not limited to people who write code generators... and in both cases it can be easily worked around.
The only way to avoid this side effect is to analyse the syntax of the file to determine whether it's genuinely a file-level comment or not (and even then there could be false positives from the user's perspective). I think we've already concluded that requiring a .go file to be syntactically valid to be considered a generated file is not a constraint we want to impose, hence the more relaxed "spec".
On a related point, is there an existing wiki that lists all of the core "special comments" (build tags, go generate directives etc)? If not, might be a useful thing to pull together, if only to signpost people to the specific documentation?

@myitcv
I'm guessing that it's a much lesser problem with go generate directives
because you almost never want one of those in a generated file.
My own suggestion would be something like:
Generated files are marked by a line of text that matches the regular expression, in Go syntax:
^// Code generated .* DO NOT EDIT.$
as long as that line is before any line of text that matches:
^package .*$

I've updated my generated comment parser Go implementation for the latest specification:
https://github.com/shurcooL/go/pull/29
The implementation is simple, it uses bufio.Reader and ReadBytes and scans through the go file one line at a time, checking each for a match. I see some opportunity to improve speed slightly by using even lower level IO primitives (such as bufio.ReadSlice maybe) and allocating less.
Review or feedback is very welcome. I care about correctness first, speed second.

@rogpeppe

I'm guessing that it's a much lesser problem with go generate directives
because you almost never want one of those in a generated file.

We happen to chain go generate directives, but I agree with the gist of your point, that it's much less likely.

My own suggestion would be something like...

I like it 👍  - simple enough to understand and implement and, as you suggest, would deal with the vast majority of false positives one might find in code generator source files. Quite how prevalent such false positives are/could be, I don't know...

@griesemer the false negative is another excellent point (unless the parser special cased checking for matching comments before it discarded them—which seems hacky and a pain to get right).
My point about token.Pos is that essentially everything using or based on go/* packages, even the super-cool high-level ones in golang.org/x let you work your way back to the AST by returning a token.Pos.
If you're doing something fancy, you probably need to analyze the package as a whole, generated files included. If you want to filter out noise resulting from machine generated files before reporting results, then you need some way to go from token.Pos to the *ast.File, which you need to determine whether the file was generated. There's no way to do that without building a map of *token.File to *ast.File, which is slightly annoying to do.
That's not a big deal and the SLOC to do it are probably a rounding error compared to whatever else you're doing in that situation.
It would be lovely if this were easier to do, but making it easier seems like it would be too complex for something that would be nice for some people sometimes, theoretically.

@myitcv At least according to the proposal your suggesting, even the example you gave (https://play.golang.org/p/VPe1Gp2zVm) does work. Note that the proposal says "anywhere in the file", and that it's easy to look at the comment's position to see if the comment was at the start of a line.
@jimmyfrasche If you have all *token.File's which it sounds like you probably have, you have the token.Pos of all *token.File's, and thus the respective filenames (from the token.Position). Just look for the token.File associated with the same filename as the one you got from your comment position. Usually there'a not that many files (< 100 per package in a worst-case scenario). Linear search is probably just fine if you do it once.

@griesemer

... and that it's easy to look at the comment's position to see if the comment was at the start of a line.

That's the obvious bit I was missing, the use of the comment's position to get the column value 👍 . Thanks

Quite how prevalent such false positives are/could be, I don't know...

One data point: both the go generators that I use regularly would have
been susceptible to the issue had they followed the standard.
@griesemer indirectly makes a good point though - if you gofmt your
output (and who doesn't?) then it's very easy to indent the comment
in the template source, avoiding the diagnosis, and have gofmt take
the indentation away on output. Still a bit subtle though - I think that
having it somewhere before the package decl is more robust and readable - I
suspect everyone will do that anyway.

I've merged shurcooL/go#29, and so the github.com/shurcooL/go/generated package is updated to latest version of the spec, and is available for use by Go programs.
All TODOs have been addressed, and there are no known issues. Please let me know if you find any.

How about if the generator left a hash digest of the contents in the generated code, and then have some tool in the toolchain complain if the hash is not valid.

@ziffusion, that seems too much effort and visual noise for the expected benefits. Also, you're commenting on a closed issue. Our dashboards don't track closed issues. If you want to float this wider, I'd recommend a new issue.

As a random data point, semicolon is by far the most common punctuation before DO NOT EDIT in the Go source as of commit 32409a2dfca955b9a28bb7f581512da8a5beaedc.
git grep 'DO NOT EDIT' | grep -Eo '..DO NOT' | awk '{print $1}' | sort | uniq -c
  66 -
  91 .
  20 /
3236 ;

@mark-rushakoff The period is used to emphasize that the user should not edit the file.  No need to undermine good grammar.


docs: fix search engine optimization (SEO) to only index latest

We need a robots.txt, map, etc. to make sure that Google only indexes our latest docs. Opening this issue in the hope that someone knows how to do this.

I'm not an SEO expert by any means but having a quick look at https://www.robotstxt.org/robotstxt.html I think we need a robots.txt file at root with something like
User-agent: *
Disallow: /docs/envoy/v1.11.2/
Disallow: /docs/envoy/v1.11.1/
Disallow: /docs/envoy/v1.11.0/
Disallow: /docs/envoy/v1.10.0/
Disallow: /docs/envoy/v1.9.1/
Disallow: /docs/envoy/v1.9.0/
Disallow: /docs/envoy/v1.8.0/
Disallow: /docs/envoy/v1.7.1/
Disallow: /docs/envoy/v1.7.0/
Disallow: /docs/envoy/v1.6.0/
Disallow: /docs/envoy/v1.5.0/

And to make it worse we'll need to update it each time there is a new release.

I would think there is a way to disallow based on wilcard/regex but I have no idea.

I opened a CNCF service desk ticket to have someone help us fix this. I think it should be easy for someone that knows what they are doing.


When using -r, the hashed env name is never printed

nox > Running session unit_tests(python_version='3.6')
nox > The virtualenv name was hashed to avoid being too long.
nox > Re-using existing virtualenv.

vs.
nox > Running session unit_tests(python_version='3.6')
nox > The virtualenv name was hashed to avoid being too long.
nox > virtualenv /path/to/.nox/775478c5 -p python3.6

Ah fair enough. Should be an easy fix, we just need to modify that to say "The virtualenv name was hashed to {hash_name} avoid being too long."

@jonparrott Seems it'd be better in the next line:
nox > Re-using existing virtualenv {hash_name}.

It actually does print it later, because it prints the command that makes the virtualenv.

But it doesn't create the virtualenv when re-using.
On Thu, Apr 13, 2017 at 3:01 PM Luke Sneeringer<EMAIL_ADDRESS>wrote:

It actually does print it later, because it prints the command that
makes the virtualenv.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/jonparrott/nox/issues/23#issuecomment-294033190, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAPUc4bJVss_OFd47BcfLZi8oPeb2zenks5rvps2gaJpZM4M9Mki
.


Fix history link in Django 1.9+

In Django 1.9 history link is at the same level of the changeview

@czpython please review

@yakky is there a way we can just reverse the url by name?

@czpython yes, we still need to inject the history view name in the context, though

That's ok for me 👍

@czpython done, have a look at it


telegraf: Allow passing ENV variables to pod

resolves #56

This would be super useful for injecting secrets. https://github.com/helm/charts/tree/master/stable/telegraf supports .Values.env, but that creates a Deployment, not a Daemonset.


MAINT: Apply lint fixes to Vue3 codebase (next)

Runs the lint fix on the Vue3 codebase in next.
This may not be required, depending upon discussion in #799.

rejecting b/c we're using the REACT codebase in next packages/react/


libofx: 0.9.12 -> 0.9.13

Semi-automatic update generated by https://github.com/ryantm/nixpkgs-update tools.
This update was made based on information from https://repology.org/metapackage/libofx/versions.
These checks were done:

built on NixOS
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofx2qif -h’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofx2qif --help’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofx2qif help’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump -h’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump --help’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump help’ got 0 exit code
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump -V’ and found version 0.9.13
ran ‘/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump --version’ and found version 0.9.13
found 0.9.13 with grep in /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13
directory tree listing: https://gist.github.com/408d21f6a77f49bceb283d4730ff7884

Success on x86_64-linux (full log)
Attempted: libofx
Partial log (click to expand)
shrinking RPATHs of ELF executables and libraries in /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13
shrinking /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofx2qif
shrinking /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin/ofxdump
shrinking /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/lib/libofx.so.7.0.2
gzipping man pages under /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/share/man/
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/lib  /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13/bin
patching script interpreter paths in /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13
checking for references to /build in /nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13...
/nix/store/3yhk4sd6brh5z7ksmhw9sznflcc3bl8s-libofx-0.9.13

Success on aarch64-linux (full log)
Attempted: libofx
Partial log (click to expand)
shrinking RPATHs of ELF executables and libraries in /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13
shrinking /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/bin/ofxdump
shrinking /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/bin/ofx2qif
shrinking /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/lib/libofx.so.7.0.2
gzipping man pages under /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/share/man/
strip is /nix/store/gp7fylxwn18b7pl2c18ks89hsiaxyfvf-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/lib  /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13/bin
patching script interpreter paths in /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13
checking for references to /build in /nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13...
/nix/store/yyx5953fw8wrpkj5bi86kic0yws1srbf-libofx-0.9.13

No attempt on x86_64-darwin (full log)
The following builds were skipped because they don't evaluate on x86_64-darwin: libofx
Partial log (click to expand)

a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


Failing test for flow.hello-world-optimization.

The tests for flow.hello-world-optimization are failing. I believe this is due to some change in signac or signac-flow. This example needs to be fixed.
root:INFO:Testing 'flow.hello-world-optimization/'...
root:INFO:Executing code found in README.md file:
root:DEBUG:Copy project into temporary directory '/tmp/tmpvzepl7f_'.
root:DEBUG:Start test.
root:INFO: - python init.py
root:INFO: - python project.py run --num-passes=100
Using environment configuration: StandardEnvironment
ERROR: An exception was raised during operation spawn_new_simulations for job or aggregate with id db46ada2c91cf8fc760a92eba1a8f898.

Traceback (most recent call last):
  File "/opt/conda/envs/signac-examples/lib/python3.8/site-packages/flow/project.py", line 3117, in _execute_operation
    self._operations[operation.name](*operation._jobs)
  File "/opt/conda/envs/signac-examples/lib/python3.8/site-packages/flow/project.py", line 619, in __call__
    return self._op_func(*jobs)
  File "project.py", line 114, in spawn_new_simulations
    random.setstate(_convert_to_tuple(job.doc.rng))
  File "/opt/conda/envs/signac-examples/lib/python3.8/random.py", line 164, in setstate
    super().setstate(internalstate)
TypeError: state vector must be a tuple
Execute with '--show-traceback' or '--debug' to show the full traceback.
root:ERROR:Error at line: python project.py run --num-passes=100
Test failed.

Resolved by #42.


Update 300-pipelinerun.yaml

Need to be consistent with 300-taskrun.yaml

Changes
Change “additionalPrinterColumns” to be consistent with config/300-taskrun.yaml

Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:

[ ] Includes tests (if functionality changed/added)
[ ] Includes docs (if user facing)
[x] Commit messages follow commit message best practices

See the contribution guide
for more details.
Release Notes

release-note

/ok-to-test

/test pull-tekton-pipeline-integration-tests


Add document metadata extraction

Context

Add custom file upload handler to extract metadata from uploaded documents

Codecov Report
Patch coverage: 85.71% and project coverage change: +0.61 :tada:

Comparison is base (deb2231) 41.69% compared to head (eb282f8) 42.30%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #308      +/-   ##
===========================================
+ Coverage    41.69%   42.30%   +0.61%
===========================================
  Files           95       97       +2
  Lines         5634     5704      +70
  Branches       669      674       +5
===========================================
+ Hits          2349     2413      +64
- Misses        3285     3291       +6

Impacted Files
Coverage Δ

trade_remedies_public/documents/views.py
38.88% <40.00%> (+2.04%)
:arrow_up:

trade_remedies_public/documents/upload_handler.py
92.85% <92.85%> (ø)

trade_remedies_public/config/settings/base.py
96.93% <100.00%> (+0.03%)
:arrow_up:

trade_remedies_public/documents/tests.py
100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.


Cannot download files from Baidu outside of China

Hi, it appears to be quite difficult to create a Baidu account and download files from Baidu from outside of China. Would it be possible to make the preprocessed image features and the model checkpoints available via some other cloud storage service, such as Google Drive?

The google drive link is https://drive.google.com/drive/folders/1lElkMAoNxM47oD9-qhAM88S1AnArC-rL?usp=sharing
I am sorry that it is available only for one month due to the storage fee. Please download it as soon as possible.


Added lights for coasts near Tuzmer

Maps:

Latarniane Wybrzeże
Wioska Rybacka
Port Tuzmer

Damned rebases never going as expected


Unable to log event: analytics library is missing

Hello everyone, in FCM I can not get notification onMessage in Android device but in IOS everything works good. Every time I send notification on foreground I get this message "Unable to log event: analytics library is missing" but if I send the notification and the app is in background it works. Do anyone knows have to fix this issue ?

Hi @NSO-Elturki
Running the official example on Android
I/flutter (31533): onMessage: {notification: {title: test, body: thanos}, data: {}}

Make sure you've the following dependencies and firebase_messaging: ^6.0.16
dependencies {
    def multidex_version = "2.0.1"
    implementation "androidx.multidex:multidex:$multidex_version"
    implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
    implementation 'com.google.firebase:firebase-analytics:17.2.2'
    implementation 'com.google.firebase:firebase-messaging:20.1.7'
}
apply plugin: 'com.google.gms.google-services'

Thank you

Shouldn't this be mentioned in the firebase_messaging README?
https://pub.dev/packages/firebase_analytics#-readme-tab-

Hi @NSO-Elturki
Running the official example on Android
I/flutter (31533): onMessage: {notification: {title: test, body: thanos}, data: {}}

Make sure you've the following dependencies and firebase_messaging: ^6.0.16
dependencies {
    def multidex_version = "2.0.1"
    implementation "androidx.multidex:multidex:$multidex_version"
    implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
    implementation 'com.google.firebase:firebase-analytics:17.2.2'
    implementation 'com.google.firebase:firebase-messaging:20.1.7'
}
apply plugin: 'com.google.gms.google-services'

Thank you

for me I'm facing the same problem now and your solution is not meant to solve the problem, I don't want to include firebase analytics. And it is working in another app I have perfectly without need of firebase analytics to be included. Anyway I will update this post when I recognise my mistake.


Very strange error

Hi, I'm new to Flutter and I just upgraded to 2.5 but I'm having an error that I can't understand.
Launching lib\main.dart on SM G611MT in debug mode...
√  Built build\app\outputs\flutter-apk\app-debug.apk.
Connecting to VM Service at ws://<IP_ADDRESS>:50240/OF-IBcc23Ms=/ws
D/vndksupport(31118): Loading<EMAIL_ADDRESS>from current namespace instead of sphal namespace.
E/flutter (31118): [ERROR:flutter/fml/platform/android/jni_util.cc(182)] java.lang.SecurityException: Permission Denial: broadcast from android asks to run as user -1 but is calling from user 0; this requires android.permission.INTERACT_ACROSS_USERS_FULL or android.permission.INTERACT_ACROSS_USERS
E/flutter (31118): 	at android.os.Parcel.createException(Parcel.java:1966)
E/flutter (31118): 	at android.os.Parcel.readException(Parcel.java:1934)
E/flutter (31118): 	at android.os.Parcel.readException(Parcel.java:1884)
E/flutter (31118): 	at android.app.IActivityManager$Stub$Proxy.reportActivityFullyDrawn(IActivityManager.java:7309)
E/flutter (31118): 	at android.app.Activity.reportFullyDrawn(Activity.java:2092)
E/flutter (31118): 	at io.flutter.embedding.android.FlutterActivity.onFlutterUiDisplayed(FlutterActivity.java:1061)
E/flutter (31118): 	at io.flutter.embedding.android.FlutterActivityAndFragmentDelegate$1.onFlutterUiDisplayed(FlutterActivityAndFragmentDelegate.java:88)
E/flutter (31118): 	at android.os.MessageQueue.nativePollOnce(Native Method)
E/flutter (31118): 	at android.os.MessageQueue.next(MessageQueue.java:326)
E/flutter (31118): 	at android.os.Looper.loop(Looper.java:181)
E/flutter (31118): 	at android.app.ActivityThread.main(ActivityThread.java:7124)
E/flutter (31118): 	at java.lang.reflect.Method.invoke(Native Method)
E/flutter (31118): 	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494)
E/flutter (31118): 	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:975)
E/flutter (31118): Caused by: android.os.RemoteException: Remote stack trace:
E/flutter (31118): 	at com.android.server.am.UserController.handleIncomingUser(UserController.java:1827)
E/flutter (31118): 	at com.android.server.am.ActivityManagerService.broadcastIntentLocked(ActivityManagerService.java:25923)
E/flutter (31118): 	at com.android.server.am.ActivityManagerService.broadcastIntentLocked(ActivityManagerService.java:25854)
E/flutter (31118): 	at com.android.server.am.ActivityManagerService.broadcastIntent(ActivityManagerService.java:26724)
E/flutter (31118): 	at android.app.ContextImpl.sendBroadcastAsUser(ContextImpl.java:1264)
2
E/flutter (31118):
F/flutter (31118): [FATAL:flutter/shell/platform/android/platform_view_android_jni_impl.cc(1201)] Check failed: fml::jni::CheckException(env).
F/libc    (31118): Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 31118 (breaklabs.onpay), pid 31118 (breaklabs.onpay)
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'samsung/on7xrefltedtvvj/on7xreflte:9/PPR1.180610.011/G611MTVJS8CUG1:user/release-keys'
Revision: '8'
ABI: 'arm'
pid: 31118, tid: 31118, name: breaklabs.onpay  >>> br.com.breaklabs.onpay <<<
signal 6 (SIGABRT), code -6 (SI_TKILL), fault addr --------
Abort message: '[FATAL:flutter/shell/platform/android/platform_view_android_jni_impl.cc(1201)] Check failed: fml::jni::CheckException(env).
'
    r0  00000000  r1  0000798e  r2  00000006  r3  00000008
    r4  0000798e  r5  0000798e  r6  ff7ee0e4  r7  0000010c
    r8  00000001  r9  ff7ee1f8  r10 ecb347ac  r11 00000034
    ip  00000041  sp  ff7ee0d0  lr  efaed0c5  pc  efae3f76
backtrace:
    #00 pc 0001cf76  /system/lib/libc.so (abort+58)
    #01 pc 011cc85d  /data/app/br.com.breaklabs.onpay-kXm_dBDvbRigQSTNQufOng==/lib/arm/libflutter.so (offset 0x11cb000)
    #02 pc 000035fe  [anon:libc_malloc:c9680000]
Lost connection to device.
Exited (sigterm)

If anyone can help me, please. Thanks.

Permission Denial: broadcast from android asks to run as user -1 but is calling from user 0; this requires android.permission.INTERACT_ACROSS_USERS_FULL or android.permission.INTERACT_ACROSS_USERS

run flutter clean and then flutter analyze
If you are using a device instead of the emulator then try to remove the app from the phone and reinstall
Or check the phone permission.

Duplicate of https://github.com/flutter/flutter/issues/88767

The error happened because I simply removed the snippet below the code from the AndroidManifest.xml file:
<meta-data
    android:name="io.flutter.embedding.android.SplashScreenDrawable"
    android:resource="@drawable/launch_background"
    />

I use the flutter native splash package so I returned the removed code and the application worked.

Hi @jesselpereira, is it safe to assume that your issue has been solved?

Hi @jesselpereira, is it safe to assume that your issue has been solved?

Yep, has been solved.


⚠ Text Filter API Endpoint has degraded performance

In 56cd402, Text Filter API Endpoint (http://textfilter.roblox.com/) experienced degraded performance:

HTTP code: 200
Response time: 74 ms

Resolved: Text Filter API Endpoint performance has improved in 622a6eb.


PSEL assertion not synchronous to clock

The following property is asynchronous:
property amiq_apb_sel_validity_during_transfer_phases_p;
  @(posedge enable) disable iff(!reset_n || !en_protocol_checks) (|sel);
endproperty

It's sensitive to glitches on enable. It should be rewritten as:
property amiq_apb_sel_validity_during_transfer_phases_p;
  @(posedge clk) disable iff(!reset_n || !en_protocol_checks) (enable |-> |sel);
endproperty

I would use:
property amiq_apb_sel_validity_during_transfer_phases_p;
@(posedge clk) disable iff(!reset_n || !en_protocol_checks) (!enable || |sel);
endproperty


Change in comparison logic behaviour in 0.38.0

Describe the bug

We recently upgraded from 0.36.2 to 0.38.0 and noticed some of our custom rules were not evaluating the same as previously. I narrowed it down to one specific macro which seems to have changed behaviour between versions.
The comparison is:
- macro: system_users
  condition: (user.name in (bin, daemon, games, lp, mail, nobody, sshd, sync, uucp, www-data))

- macro: non_privileged_users
  condition: ((not system_users) and (user.loginuid >= 1000))

The comparison user.loginuid >= 1000 does not seem to evaluate to False when its value is -1. This was the behaviour in 0.36.2 but not in 0.38.0
This can be worked around by adding another explicit condition: (user.loginuid != -1 and user.loginuid >= 1000)
How to reproduce it

- macro: recreate_issue
  condition: (user.loginuid >= 1000)

- rule: Tracking Files Deletion or Renaming
  condition: >
    ( evt.type in (unlink, unlinkat, rename, renameat)
      and evt.dir = <
      and recreate_issue )
  output: A user has successfully deleted or renamed a file. file=%fd.name name=%user.name uid=%user.uid loginuser=%user.loginname loginuid=%user.loginuid command=%proc.cmdline
  priority: INFO

Expected behaviour

We expect that this rule will trigger on file renames for logged in users which are not root.
More specifically, we expect using the greater than or equal to operator in the following condition: user.loginuid >= 1000 will evaluate to False when user.loginuid is -1
Screenshots
Example output from falco on a server running docker:
{"hostname":"********","output":"21:47:51.530700201: Informational A user has successfully deleted or renamed a file. file=<NA> loginuser=<NA> loginuid=-1 user=root command=dockerd -H fd:// --containerd=/run/containerd/containerd.sock --default-ulimit nofile=32768:65536","priority":"Informational","rule":"Tracking Files Deletion or Renaming","source":"syscall","tags":["cis"],"time":"2024-06-11T21:47:51.530700201Z", "output_fields": {"evt.time":1718142471530700201,"fd.name":null,"proc.cmdline":"dockerd -H fd:// --containerd=/run/containerd/containerd.sock --default-ulimit nofile=32768:65536","user.loginname":"<NA>","user.loginuid":-1,"user.name":"root"}}

Environment

Falco version: 0.38.0 (x86_64)

System info: Linux version 4.14.344-262.563.amzn2.x86_64 (mockbuild@ip-10-0-35-189) (gcc version 7.3.1 20180712 (Red Hat 7.3.1-17) (GCC)) #1 SMP Fri May 1718:07:48 UTC 2024

Cloud provider or hardware configuration: AWS
OS: Amazon Linux 2

Kernel: Linux 4.14.344-262.563.amzn2.x86_64

Installation method: RPM

Additional context

Thank you. We fixed the data type for the loginuid in this PR https://github.com/falcosecurity/libs/pull/1192/files#diff-70b8039f1cd1e5664df22d0b231919b7b042dfdf223134fb8356a4b7f4d0da02, @jasondellaluce would you have ideas how this could have affected the filtering logic?

I believe this might be more impacting than we thought. The data is right, but a signed comparison is being evaluated as unsigned. I suspect this line which sets a template but does not use template params:
https://github.com/falcosecurity/libs/blob/5c5edf2f58b6df59b206181af45116bd8962b9a4/userspace/libsinsp/filter_compare.cpp#L418
Let me check

Thanks for the quick turnaround on this! Much appreciated.
Happy for this to be closed now as completed.

Falco 0.38.1 is now available 🎉


Snap to roads

Is it possible use snapping on the roads in Google Maps plugin?
Google Maps have this useful API for it:
https://developers.google.com/maps/documentation/roads/snap

The Snap to Roads API is not implemented in this plugin.
If you want to use it, you have to implement it by yourself.


Rename from aries-bbs-rs to aries-bbssignatures-rs

Rename from aries-bbs-rs to aries-bbssignatures-rs. The goal is to match the naming convention of the other 2 repos created for indy-blssignatures-rs and anoncreds-clsignatures-rs.

Looks like I don't have the ability either :)


Update attributes to suppress warnings during compilation

Currently, when using this package with the latest version of V there are many undesired compilation messages such as:
/home/<user>/.vmodules/irishgreencitrus/raylibv/raylib.v:744:1: notice: `[attr]` has been deprecated, use `@[attr]` instead
  742 | // start functions
  743 | fn C.InitWindow(width int, height int, title &i8)
  744 | [inline]
      | ^
  745 | pub fn init_window(width int, height int, title &i8) {
  746 |     C.InitWindow(width, height, title)

This PR updates the attributes to start with @[.
see: https://github.com/vlang/v/blob/master/doc/docs.md#attributes

Fixed in https://github.com/irishgreencitrus/raylib.v/pull/25 too, which also runs v fmt -w .

Thanks for your contribution, I have merged this change in #25. Thanks again for the effort, though


Question: How to pass custom variables via the API?

Question
Hi,
is there a possibility to pass a set (list, dict, etc.) of custom variables via the API to use them in Ansible Playbooks?
For example:
Trigger a playbook run with {{ ip_address }} somewhere inside, where it will be filled with data from the API call wich triggered it.
Thanks!
Related to
Web-Backend (APIs), Ansible (task execution)

Just found it here: https://github.com/semaphoreui/semaphore/issues/1043#issuecomment-1269835356


Portfolio account switch

close #1236

Not too sure how it happened but I got this error when switching between accounts

Can't reproduce it

I think we should still support account sharing if there is a connected wallet. For example if I click the link on the market page I get redirected to the connected wallet isn't of the one in the link I clicked

You're right this should work, but what happens if i change the account now?
e.g. my wallet is connected and, open Malte's portfolio and now I switch account in my wallet. Think I should still be on Malte's portfolio page

update portfolio account switching

yeah that sounds right to me

Not too sure how it happened but I got this error when switching between accounts

Can't reproduce it

I've pushed a fix


finish ul with nl2br in markdown

Sorry for mi bad english.
When i press enter after a defined ul in the markdown editor, the output will not make a new "p" tag.

I'm not sure if this is a question or a statement :)
Markdown generally requires an empty line between paragraphs.
So something like this:
intro pargraph...

1. my list item
2. another list item

another paragraph

which will output:

intro pargraph...

my list item

another list item

another paragraph

You can test markdown parsing here: http://parsedown.org/demo
As we use the Parsedown library to do all our markdown parsing.

Thx for your answer.
But when i use the nl2br with the markdown filter then i have this problem.
How i can use nl2br with markdown?
I will make a new line for every "enter" in the editor.


Access fields using attributes, in transactions and blocks

First draft was at #201 - see there for previous commentary. Things changed enough that I wanted a fresh branch, instead of backing out the commits or using a force push.
Additional benefits:

Objects become hashable for use in sets
Immutability by default

Currently applied to these output objects:

Transactions
Transaction Receipts
Blocks
eth.syncing

Also, I added gitignore for vim swap files and the virtualenv folder name used in the setup instructions.
What was wrong?
These were some key benefits I was looking for:

can be used in a set, to prevent duplicate transactions with the same hash
can access the transaction values as attributes (It just feels more natural to use tx.hash than tx['hash'])
transaction object should be immutable, to eliminate a whole class of bugs at very low cost

In short, I want to use transactions as a first class object. At request, I extended this concept to several other objects.
How was it fixed?
A new AttributeDict data structure is applied to the relevant output formatters. Note that AttributeDict(None) will raise an exception, so @apply_if_not_null must precede the AttributeDict wrapper.
There is also a MutableAttributeDict that is currently unused, and a base class to share implementation details.
What was implemented contrary to feedback?

I used the shorter name for the Immutable version, because
** it was a bit long/unwieldy
** it was the only one in current use in the project
** it mirrors the python collections naming of: Mapping / MutableMapping
I didn't create a separate hashable variation
** A hashable, mutable class is disallowed by python spec
** I didn't see a need for a non-hashable, immutable class

Cute Animal Picture

This looks good to me.  There's the one thing about repr formatting that's your call.  Let me know when you're good with this being merged and I'll get it in and released.

Leaving it as-is wasn't sitting quite right with me, either. My favorite solution now is to have a classic repr, plus a custom ipython output.
I looked at hooking into pprint with a custom dispatch, but it was getting ugly (and wasn't working for ipython).
I don't love tying the solution to one package, but ipython is the de-facto standard, and the implementation only affects aesthetics. We should steer people away from the under-featured default python REPL, anyway.
I'm inclined to say this is good enough, and if someone finds a better solution, we can fix forward.


Specify library dependencies in library.properties

Specifying the library dependencies in the depends field of library.properties causes the Arduino Library Manager (Arduino IDE 1.8.10 and newer) to offer to install any missing dependencies during installation of this library.
arduino-cli lib install will automatically install the dependencies (arduino-cli 0.7.0 and newer).
Reference:
https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification#libraryproperties-file-format

Thank you @per1234 for the notification & PR relating to the depends flag!


[redo] remap non-element entities in the transformer in order to handle entities referencing each other out of order

retry of #4137
This is intended to be backported as a patch to 3.4
Differences

native code was updated/fixed, there is a new native PR. This caused most of the (mac only) test failures.
reuse the db or use different names for some dbs in the new ECReferenceTypesCache tests.
some doc fixes

Original Description:
What & Why
Before this PR, the transformer when copying an aspect's props will put the invalid id 0 in a navigation property if that aspect entity hasn't been mapped to the target yet. This causes crashes when processing ExternalSourceAspects in some models because their scope is expected to be valid. Worse, it has caused data loss in the past. We have a way of handling that for elements, which is putting it in the partiallyCommittedElements and pendingReferences collections and resolving those references, updating the corresponding element with the mapped id once we have it. This PR generalizes partiallyCommittedElements to all entities and provides some ways to refer to, insert, and update entities generically.
Changes

depends on a ~native PR~ (this PR was bad, above is the correct one)
change deepEqualWithFpTolerance to advanceEqual with the ability to compare denormalized class names in ec props correctly, fix up small parts of older test code to use this where it makes the intent more readable
add the EntityReference type, which is an Id64 but with the a root bis class tag (ConcreteEntityTypes) prepended, to make it possible to use them in sets of potentially overlapping id spaces (elements and non-element entities have overlapping Id64 spaces)
insertAspect and corresponding aspect import functions in the transformer now returns the id (this necessitated the native change)
introduce the future of getReferenceIds as the temporary getReferenceConcreteIds which gets an EntityReference. In a future breaking change, @internal getReferenceConcreteIds will be removed but its implementation will replace that of the @beta getReferenceIds.
add an ECReferenceTypesCache to store the ConcreteEntityTypes of relationships and navigation properties, for fast retrieval where necessary, such as in the transformer
add a small utility makeCategorizedLogger to make categorized logging less verbose
add a TupleKeyedMap type for hash-less value-compare mapping of tuple types, which is used in ECReferenceTypesCache
add a skipped test that demonstrates an in-code approach to profiling a block of code, and a utility runWithCpuProfiler(func), which was used to analyze performance of changes
minor clean up to some transformer code
correct an out of date argument in the imodel-transformer test-app README
add an EntityUnifier class to make it easier to perform CRUD (like insert) operations on non-abstract entities without knowing their type, as necessary for finalizing partially committed entities in the transformer
move cloning aspects out of the transformer into the IModelCloneContext to make dealing with aspect entities consistent with element entities
deprecate calling initFromExternalSourceAspects before any of the process* functions, by doing it for you
save some milliseconds per test in a few tests by not calling ExtensiveTestScenario.populateDb in those tests, instead do it once lazily in ReusedSnapshots, and create new snapshots from it instead of creating an empty db and reinserting everything
add a test for out of order aspect references which is what prompted this design change to handle non-element entity references. Although it doesn't happen yet afaik, this will now properly also handle navprops on models and link table relationships (although there is a bug in navprops on link table relationships as of writing)
some changes here race https://github.com/iTwin/itwinjs-core/pull/4082, and I will have to move some of the utilities out of the backend and into the transformer to use the ecschema-metadata classes

changes since merging with master:

add a dev dependency on ecschmea-metadata in core-backend to make a type-import to deal with raw metadata in ClassRegistry
move ECReferenceTypesCache and EntityUnifier to transformer where I can use the ecschema-metadata package
split deprecate IModelCloneContext, rename it to IModelElementCloneContext, and create a new IModelCloneContext in the transformer deriving from it that handles non-element entities.
fix a type error in ElementDrivesElements.create where it used the wrong props type. Not sure why the tests didn't fail before aspect changes.
remove detecting of relationship end reference types, since only 3 base linktable relationship classes can exist, and their constraint classes are known. I added ModelSelectorRefersToModels for completeness, but it isn't used and the class is sealed so can't be extended

this is currently being held up by me not having a working mac environment to test on (my VM just crashed installing xcode)

thanks to @aruniverse got it working on ARM mac. Now just need to confirm it works on x86 since as @khanaffan noticed with me, the error appears to be different there. Hopefully it's the same root cause so it's fixed.

CI confirms it now works on MacOS x64

Waiting for builds to succeed before reviewing.

Waiting for builds to succeed before reviewing.

looks like I still had to kick them off manually even after undrafting, maybe because of the failing extract-api step...

MacOS_Node_16 failed at step Build iOS display-test-app swift with no space left on device

MacOS_Node_16 failed at step Build iOS display-test-app swift with no space left on device

@iTwin/mobilesdkadmins

same test failed again with same message

xcodebuild: error: Could not resolve package dependencies:
artifact of binary target 'IModelJsNative' failed download: The operation couldn’t be completed. No space left on device

@MichaelBelousov are you trying to merge your native PR, publish new addon, integrate that into master, then merge this PR into master?

@MichaelBelousov are you trying to merge your native PR, publish new addon, integrate that into master, then merge this PR into master?

Yes that is the intent I attempted to describe in the unresolved comment I posted earlier.

Yes that is the intent I attempted to describe in the unresolved comment I posted earlier.

I thought so, but that seems like a very bad idea. What are other developers supposed to do while the imodel02 branch is broken?

I thought so, but that seems like a very bad idea. What are other developers supposed to do while the imodel02 branch is broken?

I see what you mean. I will rebase this on imodel02, the primary reason to do it this way was so that I could make sure the CI passes on all platforms in this PR before it merged.

I cherry-picked only the "don't reuse empty bim changes" onto the original branch, and redid extract-api and rush lint. All other fixes are in the native branch referenced inthe description.

should now be ready for review since CI doesn't apply anymore.

same test failed again with same message

xcodebuild: error: Could not resolve package dependencies:
artifact of binary target 'IModelJsNative' failed download: The operation couldn’t be completed. No space left on device

The internal disk drive for the CI machine was indeed full, Chuck cleaned it out. I think he's the only one that can do that.

I intend to perform the new addon process tomorrow morning. Can someone please comment if that means I can't merge after approvals? I believe the regular procedure is not to merge either native or js PR until you're ready to produce the addon

same test failed again with same message

xcodebuild: error: Could not resolve package dependencies:

artifact of binary target 'IModelJsNative' failed download: The operation couldn’t be completed. No space left on device

The internal disk drive for the CI machine was indeed full, Chuck cleaned it out. I think he's the only one that can do that.

Why is this happening? Is the mobile sdk not getting cleaned up? Happening for other reasons unrelated to mobile build?

same test failed again with same message

xcodebuild: error: Could not resolve package dependencies:

artifact of binary target 'IModelJsNative' failed download: The operation couldn’t be completed. No space left on device

The internal disk drive for the CI machine was indeed full, Chuck cleaned it out. I think he's the only one that can do that.

Why is this happening? Is the mobile sdk not getting cleaned up? Happening for other reasons unrelated to mobile build?

In this case a whopping 350GB was taken up in the .npm folder. I don't know what's causing that, but it certainly isn't related to mobile builds, that was just the one that failed.

@calebmshafer @paulius-valiunas could you please review this since you reviewed the previous PR https://github.com/iTwin/itwinjs-core/pull/4137?
The differences, summarized in the description, can be seen with:

the corresponding native PR was fixed
sharing a filename during tests was fixed. diff
some doc fixes over the initial PR. diff

Unfortunately a diff over the entire section of new commits will include the merge and therefore all changes in the imodel02 branch since the initial PR, so it is pretty big, hence the two separate diffs above.

I overrode the policy for @ColinKerr to approve as he is out of office currently.


fix ci

PR title
Status
READY/IN DEVELOPMENT/HOLD
Description
A few sentences describing the overall goals of the pull request's commits.
Related PRs
List related PRs against other branches:

branch
PR

other_pr_production
link

other_pr_master
link

Todos

[ ] Tests
[ ] Documentation

Steps to Test or Reproduce
Outline the steps to test or reproduce the PR here.
git pull --prune
git checkout <feature_branch>
pytest --showlocals

Impacted Areas in Application
List general components of the application that this PR will affect:

:white_check_mark: Build ebook_homebrew 1.0.443 completed (commit https://github.com/tubone24/ebook_homebrew/commit/763db8ba63 by @)


SqliteForeignKeyIndexConvention not working for Composite Primary Keys

Define a class with a composite primary key
Add 2 many-to-many relationships to the class
During creation, the indices for the navigation table are not properly changed to include the table name.

Two Intermediate Tables are created:  FooCompositeKeyFooRelationshipA and FooCompositeKeyFooRelationshipB  both tables contain a IX_FooCompositeKey_Id_FooCompositeKey_Version index for the composite primary key.
     public class FooCompositeKey
    {
        [Key, Column(Order = 1)]
        public int Id { get; set; }

        [Key, Column(Order = 2), StringLength(20)]
        public string Version { get; set; }

        [StringLength(255)]
        public string Name { get; set; }

        public virtual ICollection<FooRelationshipA> FooeyACollection { get; set; }

        public virtual ICollection<FooRelationshipB> FooeyBCollection { get; set; }
    }

    public class FooRelationshipA
    {
        public int Id { get; set; }

        [StringLength(255)]
        public string Name { get; set; }

        public virtual ICollection<FooCompositeKey> Fooey { get; set; }
    }

    public class FooRelationshipB
    {
        public int Id { get; set; }

        [StringLength(255)]
        public string Name { get; set; }

        public virtual ICollection<FooCompositeKey> Fooey { get; set; }
    }

Fix: Update line 49 in the SqliteForeignKeyIndexConvention.Apply method from
string propertyName = edmProperty.Name;

to
//Check for Composite Primary Keys
string propertyName = item.Constraint.FromProperties.Count > 1 ? String.Join("_", item.Constraint.ToProperties) : edmProperty.Name;

Thank you for creating the issue, it is fixed with the version 1.5.
To support this project you can

star the repository;
report bugs/request features by creating new issues;
write code and create PRs;
donate.

https://github.com/msallin/SQLiteCodeFirst/tree/v<IP_ADDRESS>#support-the-project-


Error when submitting narwhal transactions through SubmitTransactionStream

Steps to Reproduce Issue
I'm playing around with narwhal and try to submit some transactions to it. The code is similar to the benchmark client, but I always run into the following error without an helpful message:
Failed to send transaction: status: Unimplemented, message: "", details: [], metadata: MetadataMap { headers: {"date": "Tue, 15 Nov 2022 15:24:59 GMT", "content-type": "application/grpc", "content-length": "0", "grpc-path-req": "/narwhal.Transactions/SubmitTransactionStream"} }

let mut client = TransactionsClient::connect(dsts[client].clone())
    .await
    .expect("Could not create TransactionsClient");
let stream = tokio_stream::iter(txs.into_iter())
    .map(move |tx| {
        println!("Resending tx {:?}", &tx);
        TransactionProto {
            transaction: tx.serialize(),
        }
});

if let Err(e) = client.submit_transaction_stream(stream).await {
    println!("Failed to send transaction: {e}");
    // FIXME: Not sure why this keeps happening, ignore for now
    // return Ok(());
}

Expected Result
Submitting should either work or provide a more helpful error message.
Actual Result
Error without useful message.
System Information

OS: macOS 13.0.1
Compiler: rustc 1.64.0

@palango can you confirm which revision of the code base you're using as this has changed recently?
/cc @Andrew Schran that has recently looked at this

@huitseeker This is on  0bc7d1984 - (tag: devnet-0.14.1) from 3 weeks ago.

Are there any debug logs on worker side we could also look at?

@aschran Here are logs of the above happening:
logs.zip
In demo_client-1.log and demo_client-2.log you see the failures when sending transactions to the narwhal node. I didn't find any errors in the worker logs with a quick grep.

I don't see any changes in the gRPC server code recently. Did it used to work and now it broke, or have you never gotten this working before?
Based on lack of log errors I'd guess that the endpoint your client is connecting to is not actually providing the Transactions gRPC service and this is some default error gRPC generates for that case. Which server are you connecting to? If you manually run grpc_cli ls on it while it's running, what services does it show as available?

Hi @aschran
to give you a bit more context: I wanted to learn about more about the provided API and decided to build a little end-to-end prototype including narwhal. I found the existing benchmark and demo infrastructure quite useful and changed it to my requirements. This means:

the demo client was changed to run a simple blockchain that creates blocks from the transactions received from narwhal
the benchmark client was changed to send transactions of my simple blockchain

Creating the transactions in the benchmark client works, so the SubmitTransactionStream endpoint seems to work. Also, in the demo client, usually a few transactions can be sent before the mentioned error appears. From that I'm assuming that the Transactions gRPC service is available.
The code used to generate the logs is available at https://github.com/palango/sui/tree/for-mysten . To run it, just do
cd narwhal/benchmark/
fab demo

Hope this makes the problem clearer for you, I'm happy to answer any additional questions.


Chains should be able to specify what consensus proof gates their ability to update state on AggLayer

I believe the current assumption is that there is a signature from the submitter that is used to gate these updates. We would also like to be able to support Proof of Consensus (for e.g. tendermint), or maybe a threshold signature scheme.
I think this effectively requires the Protocol to support this as a part of its consensus implementation for pessimistic proof, but the agglayer interface also needs to support accepting this information during a chain update.
cc: @hadjiszs @Freyskeyd @krlosMata

I think the idea here is in https://github.com/AggLayer/agglayer-rs/blob/main/crates/agglayer-node/src/rpc/mod.rs#L155 instead of calling verify_signature we can add a function verify_finality where based on RollupTypeId we decide to verify the signature OR based on https://github.com/AggLayer/specs/blob/main/src/protocol/consensus_proof.md to verify the block header for tendermint based chains.
graph TD
    A[Chain] -->|Submit Tx| B[AggLayer]
    B -->|Forward Consensus Proofs| C[Proof Verifier]
    B -->|Relay Auth Methods| D[Verify finality]
    B -->|Determine Actions Based on| H[Rollup Type ID]
    C -->|verify_execution_proofs| E[Execution Proof Verifier]
    C -->|verify_pessimistic_proofs| F[Pessimistic Proof Verifier]
    D -->|verify_signature| I[Signature Verification]
    D -->|verify_threshold_signature| K[Threshold Signature Verification]
    G[Polygon Rollup Manager Contract] -->|Get Rollup Type ID| H

    subgraph Proof Verifiers
        E
        F
    end

    subgraph Finality Verification
       I
       K
    end

    subgraph Tx Structure
        M[SignedTx]
        M --> N[tx: ProofManifest]
        M --> O[signature: Option<Signature>]
        M --> P[threshold_signature: Option<ThresholdSignatureData>]
    end

@vladimir-trifonov I think all of this will not apply to the send_tx RPC, but to the next RPC that isn't properly design yet. As we do not have a decision on the format of the inputs.

@vladimir-trifonov I think all of this will not apply to the send_tx RPC, but to the next RPC that isn't properly design yet. As we do not have a decision on the format of the inputs.

makes sense

Generic consensus identifier has been implemented:

https://github.com/0xPolygonHermez/zkevm-contracts/blob/feature/ongoingPP/contracts/v2/consensus/pessimistic/PolygonPessimisticConsensus.sol#L36

From the prover perspective, it needs to re-build the hash to match the public input consensusHash.
The specification is as follows:
uint32 CONSENSUS_TYPE;
consensusHash = keccak256(abi.encodePacked(CONSENSUS_TYPE, CONSENSUS_PAYLOAD));

First iteration only supports ecdsa signature:

CONSENSUS_TYPE = 0;
CONSENSUS_PAYLOAD = uint160 (address)

Adding new consensus will not change pp-proof but only support them in the smart contracts


Error in Log

I'm not sure if this is pointing to a larger problem or not, but the individual file log has incorrect info.  The identified input file and the original file name are two different files (see the attached example).  What is going on here Fernando?
FSCAN.SCX.LOG

HI Rob!
Can you tell me, please, this info?:

What version of FoxBin2Prg are you using
The LOG attached is incomplete, header lines have important info missing
Are you using EXE syntax, PRG syntax or object syntax or how are you running the program
Are you using the program alone or integrated with some Source Control tool or editor (name/version)?
Can you attach the SCX/SCT file?

Thanks!

I am using the latest code from this repository with the vbscript named "Convert_VFP9_BIN_2_PRG.vbs".  This has nothing to do with the source control.  I simply ran your script from the "SendTo" folder on a top level folder of my project, which has many forms within, as instructed.  The form is attached.
forms.zip

Ok, I think that I can --more or less-- replicate part of your LOG. Yes, it's not ok that bad LOG info appear, but you have nothing to care, generation is OK.
But anyway, I'm going to look at this to try to fix this.
Thanks Rob!

No, thank you Fernando for all this free work.  I just wanted to ensure this wasn't the sign of a bigger problem in the converted files.

Hi Rob,
There is a confirmed bug with latest version v1.19.48 and 49 of FoxBin2Prg.
Please, use v1.19.47 until I fix it, and always have a backup if you don't use a Versioning Control System
You can check at issue#9 to be notified when fixed.
Regards.-

Hi Rob:
Last bug (issue#9) I'd comment to you is now fixed. Please update to latest FoxBin2Prg from GitHub, and do not forget to copy all the files and not just FoxBin2Prg.exe/prg, because based on your attached LOG I can see that you don't have all the programs in place.
Once you have downloaded the latest version, and erased old FXPs (very important!), please, repeat the operation that did not make the correct LOG, to check if still happens
Thanks!

I've updated my local foxbin2prg repo, deleted all *.exe and *.fxp files, recompiled all projects, copied filename_caps.exe to the repo root folder, and rerun the Convert_VFP9_BIN_2_PRG.vbs script on my project folder.  The latest version of the example log file I provided initially is attached.
FSCAN.SC2.LOG

It's really annoying why "freprint" keep showing in your log.
In order to be sure that there isn't a configuration or installation problem, I give you an example of a FoxBin2Prg typical installation, and tell me what differences with what you see with your installation:

It's necessary to have a directory for FoxBin2Prg project only (in example: "c:\vfp\foxbin2prg", but you can use another) ==> Do not use the same directory of your application
In this directory, it's necessary to have at minumum, this files: [foxbin2prg.exe, filename_caps.exe, fb2p_diff.exe, VFP9_FoxBin2Prg.vbs, Normalize_FileNames.vbs, Convert_VFP9_BIN_2_PRG.vbs, Convert_VFP9_PRG_2_BIN.vbs].
Initially I suggest having ONLY this files in the FoxBin2Prg directory, and no sources (no PRG, no FXP, no subdirectories) so we make sure that there is no other interferences
Scan your disk just in case you have another copy of foxbin2prg.* file in another directory, because this can interfere with the normal execution (mainly when an old copy is found in the PATH of execution)
Make sure that in the "Send To" folder you have a clean shortcuts of Convert_VFP9_BIN_2_PRG.vbs, Convert_VFP9_PRG_2_BIN.vbs and FoxBin2Prg.exe (Warning: just the shortcuts, not a copy of the original programs! Please check this point)
I suggest using the foxbin2prg.exe shortcut only (with the "INTERACTIVE-SHOWMSG" added in the shortcut, as explained in the README.md example table), because it can handle all convertion cases: TX2 single file, VFP binary fine and directory selection

Does occurs the same problem if you use the FoxBin2Prg.exe shortcut directly instead of the VBS scripts?
Thanks Rob!

Closed because can't reproduce and there is a lack of data to continue looking at this.