text:
Figma Integration and Screenshot testing omnibus

Rob's comment on a dependency update PR led me to want to implement both Figma integration tests and screenshot tests. This ticket will track progress towards an eventual result. Due to slow response to reviews I've had to start chaining PRs and have rebased older PRs on to new ones. The PR's should be reviewed in this order:

#352 : This enhances DesignCompose's testability with a way to set the Figma Token for Android integrated tests and a way to trigger a fetch and check the results
#343 : This adds the same tests to Validation
#359 : This adds Roborazzi screenshot tests. (PR sill in draft at the time of this writing

Note that this PR incorporates #358, though that PR is separate and has no dependencies
A demonstration PR with a test failure is provided via #360

#294: This was the original PR that prompted this, it updates the compileSDK to 34 and Compose dependencies to the latest.  It also serves as a second test PR for #359

As seen here, we got OOM errors for the Grid Layout and Widget examples, and I still don't have Robolectric set to render them at a large enough size to fully show them. Frankly I think they and a few other of the examples need to be broken up and or shrunk. We should have our Validation examples fit in a reasonably sized screen, preferrably one that uses a known virtual device in Android Studio. We could make a separate Validation app target specifically for abnormally large screens.

Additional screenshot tests I have in mind include:
All parts of the Design Switcher
Pushing buttons and changing values in the interaction, dials/guages and similar Validation apps
Iterating through all screens of Tutorial

repo: google/automotive-design-compose


text:
Ensure Tutorial App can be built with Android Studio Electric Eel

We want to make sure that the previous stable release is compatible with it.

@iamralpht I've taken a shot at this and I'd need a day or so to accomplish this.
Problem 1 is that Electric Eel only supports AGP 7.4.2, not 8, which is what everything currently uses. So the assumed easy fix is to pin the Tutorial app's AGP version to 7.4.2.
But problem 2 is that the Tutorial App is relying on the build-logic subproject for some build config and for the setFigmaToken tasks. The build config is easily solved, but in order to have the setFigmaToken tasks, I would have to separate that code from the build-logic subproject into it's own plugin and publish the plugin to gMaven. I had always intended to do this at some point, after we had more than just the setFigmaToken tasks to publish as a "DesignComposeGradlePlugin", and it would only take a day or less to do this. But until that happens, the Tutorial app will only be compatible with Android Studio Flamingo.
I would like your input here.

Moving to the current iteration, since we didn't complete this in the previous.

Update from triage review -- WIP is working, and also creates a new plugin for the setFigmaToken logic. Should be in review later today with some tests on the new work.

source: gharchive/issue

authors: iamralpht timothyfroehlich

license_type: permissive

license_source: github-api


text:
duplicated joda time?

It seems that third_party/joda-time and third_party/joda_time are identical. I have submitted a code review to remove third_party/joda-time.
https://bazel-review.googlesource.com/#/c/1600/

Thanks!

The change was merged. There was a little issue in the merge and the description got redacted unfortunately :(

authors: damienmg huahang kchodorow

repo: google/bazel


text:
Allow tests to mock filesystem

Tests can call Context.MockFileSystem to pass in a map of filenames to
contents.
Change-Id: Idd67c68f7cb43bc2117cc78336347db7e2f21991

LGTM

source: gharchive/pull-request

authors: colincross danw

repo: google/blueprint

license_source: bigquery


text:
are TagKey and TagValue idiomatic?

I can see the desire to solve the problem of length constraints with types, but in java, you can't extend String. Making TagKey and TagValue wrapper types makes it clunky to specify tags, in a way that will be even more obvious when other JVM languages use this. TagKey and TagValue feel clunkier than the Tag object itself.
Is there some other reason besides length we are doing this? If not, I'd highly recommend reverting and moving the length constraint to a precondition. Libraries should be a joy to use.

As I understand it, applications should use a limited number of unique keys and values, for performance reasons.  The keys and values will probably be written as constants that can be reused, and the classes will let us only validate strings once.  The validation code also checks that the strings are printable ASCII characters.  How will the classes affect other JVM languages?
Tag isn't used much, so I think we can remove it.

There's nothing about performance that makes a string wrapper a better
constant or better for reuse than a string. It is just making the api
more difficult to use. Validation of ascii is also not a good enough
reason, as this can be done without requiring those declaring
constants to use special types.
If I'm using clojure, I'd want to not think about arbitrarily
different types when thinking of key/values.
ex. (hello :name "John" :salutation "Hi")
I'd also prefer this in Java. the more unnecessary types we define,
the more bloated and burdensome the code is to use.

For a tangible next step, even if wrappers are used internally (which
so far, I really don't see a need for), I'd recommend not leaking
these into the public api, or at least not requiring them.
I can punt a pull request on this to show what I mean.. if it doesn't
make the thing more idiomatic or less crufty, I'd be happy to have
given it a shot.

Our experience has been that, if we don't sanitize tag keys and values when they are created then we have to sanitize them at every use, which is not only tedious but also error prone (I can't begin to tell you how many problems this has caused in the past).
I don't see another solution in Java of how to guarantee that the strings are properly sanitzed. As Kristen has mentioned, most client code should define their tag keys and values statically and just refer to them as constants, which has worked out pretty well in practice.

authors: adriancole dinooliva sebright

repo: google/census-java


text:
Produce a single .a for fuzz_target_runner.

Would it be possible for the current build system to produce a single .a for the fuzz_target_runner to link against fuzz targets?
Right now we hardcode the list of .a files we need to link in on FuzzBench: https://github.com/google/fuzzbench/blob/199d51fdbaadbaa13161e4215196dee270cd0d1c/fuzzers/centipede/fuzzer.py#L33 which is very brittle. Globbing for all .a files also doesn't seem to work.

Yes, it should be possible: I'm working on this now.

This particular aspect of the OSS build simplification has been fixed in this PR: https://github.com/google/centipede/pull/33/commits.
What still remains: running with the fork server on is still finicky; I'm working on addressing that now (with Kostya's great inputs).
Closing this issue.

authors: oliverchang ussuri

repo: google/centipede


text:
[NTI] "||" shortcircuit causing warning about return value being "undefined" on namespaced propertiees.

/** @const */ var ns = {};

/** @constructor */
ns.Foo = function() {};

/** @return {!ns.Foo} */
function f() {
  return ns.x || (ns.x = new ns.Foo);
}

/** @type {!ns.Foo|undefined} */ ns.x;

gives:
input0:8: WARNING - Returned type does not match declared return type.
declared : ns.Foo
found    : ns.Foo|undefined

  return ns.x || (ns.x = new ns.Foo);
  ^

0 error(s), 1 warning(s), 100.0% typed

Not sure if this is a limitation of NTI, but it's clear that ns.x is not undefined by the time f returns.
http://closure-compiler-debugger.appspot.com/#input0%3D%252F**%2520%2540const%2520*%252F%2520var%2520ns%2520%253D%2520%257B%257D%253B%250A%250A%252F**%2520%2540constructor%2520*%252F%250Ans.Foo%2520%253D%2520function()%2520%257B%257D%253B%250A%250A%252F**%2520%2540return%2520%257B!ns.Foo%257D%2520*%252F%250Afunction%2520f()%2520%257B%250A%2520%2520return%2520ns.x%2520%257C%257C%2520(ns.x%2520%253D%2520new%2520ns.Foo)%253B%250A%257D%250A%250A%252F**%2520%2540type%2520%257B!ns.Foo%257Cundefined%257D%2520*%252F%2520ns.x%253B%26input1%26conformanceConfig%26externs%26refasterjs-template%26includeDefaultExterns%3D1%26CHECK_SYMBOLS%3D1%26CHECK_TYPES_NEW_INFERENCE%3D1%26LANG_IN_IS_ES6%3D1%26MISSING_PROPERTIES%3D1%26PRESERVE_TYPE_ANNOTATIONS%3D1%26PRETTY_PRINT%3D1%26TRANSPILE%3D1
A similar snippet of code not using namespaced variables gives no warning:
/** @constructor */
var Foo = function() {};

var /** !Foo|undefined */ x;

/** @return {!Foo} */
function f() {
  return x || (x = new Foo);
}

http://closure-compiler-debugger.appspot.com/#input0%3D%252F**%2520%2540constructor%2520*%252F%250Avar%2520Foo%2520%253D%2520function()%2520%257B%257D%253B%250A%250Avar%2520%252F**%2520!Foo%257Cundefined%2520*%252F%2520x%253B%250A%250A%252F**%2520%2540return%2520%257B!Foo%257D%2520*%252F%250Afunction%2520f()%2520%257B%250A%2520%2520return%2520x%2520%257C%257C%2520(x%2520%253D%2520new%2520Foo)%253B%250A%257D%250A%26input1%26conformanceConfig%26externs%26refasterjs-template%26includeDefaultExterns%3D1%26CHECK_SYMBOLS%3D1%26CHECK_TYPES_NEW_INFERENCE%3D1%26LANG_IN_IS_ES6%3D1%26MISSING_PROPERTIES%3D1%26PRESERVE_TYPE_ANNOTATIONS%3D1%26PRETTY_PRINT%3D1%26TRANSPILE%3D1
Real-world example: https://github.com/Dominator008/closure-library/blob/domHelper/closure/goog/dom/dom.js#L75-L94.

This seems to be fixed by https://github.com/google/closure-compiler/commit/f9317f38ca397b4313e88c442302c73a0ca81ae7.

repo: google/closure-compiler


text:
consider try/catch blocks when inlining functions

This issue was imported from Closure Compiler's previous home at http://closure-compiler.googlecode.com
The original discussion is archived at:
http://blickly.github.io/closure-compiler-issues/#1078

@concavelenz I think TurboFan will be fine with those.


text:
Feature request: TypeScript index signature types

I would like to add the ability to parse and transpile TypeScript index signature types. This would be similar to @Dominator008 's previous commit that added the record type
This is needed for Closure Object map type conversion to TS: https://github.com/angular/clutz/issues/390
@rkirov
Is it all right if I add the index signature types support? Are there any concerns?

To clarify, you would like add CodeGenerator support for index signature types right? I thought I added parsing support already: https://github.com/google/closure-compiler/blob/ce5b2aae8644a084607426e8212c28986ecef990/src/com/google/javascript/jscomp/parsing/parser/Parser.java#L1131

That's right. Sorry I missed that part.
I also want a way to instantiate a TypeDeclarationNode representing the index signature types. So something like this. Is this also supported already?

@bowenni Actually I think CodeGenerator support is in place already: https://github.com/google/closure-compiler/blob/d97a43d7cccfc2b2fe8dc7e3c9512382034011c2/src/com/google/javascript/jscomp/CodeGenerator.java#L1224
So only TypeDeclarationNode support needs to be added.

@Dominator008 Michael, will you be able to do code reviews for this?

@dimvar Yes I can.
@bowenni It will be better if you submit the change internally.

Thanks!

will send a CL, thanks!

It turns out that only Parser and IRFactory support for index signature type is needed (which is already in place). TypeDeclarationNode support is not necessary. We have customized compiler passes.
Thanks for helping and sorry for the confusion.

authors: Dominator008 bowenni dimvar


text:
Strange behavior in goog.ui.Component.prototype.addChildAt

This issue was imported from Closure Library's previous home at http://closure-library.googlecode.com
The link to the original issue is:
https://code.google.com/p/closure-library/issues/detail?id=182

This issue is being closed as a preparation step before archiving the repository.  See the README for more detail.

authors: nanaze shicks

repo: google/closure-library


text:
Triangle dataset

Is there a place I can access the triangle data set mentioned in the paper, or it's generator anywhere? Thanks

Currently neither data or code are published. We are looking into options to publish it.

Hi aaronrmm,
here is the colab to generate the data: https://colab.sandbox.google.com/github/google/compare_gan/blob/master/colabs/Convex_Polygons_Dataset.ipynb

authors: Marvin182 aaronrmm mariolucic


text:
Support flashing recovery image

https://dl.google.com/dl/edgedl/chromeos/recovery/recovery.conf have enough info to determine an image to recover from a model name (first part of hwid), which is a best publicly available way.

This has already been impremented.

authors: hikalium mhiramat


text:
InvalidLink false positive

Description of the problem / feature request:
For this code: https://github.com/spotify/mobius/blob/master/mobius-test/src/main/java/com/spotify/mobius/test/InitSpec.java#L68, errorprone v2.4.0 warns that 'warning: [InvalidLink] The reference #when(M) to a method doesn't resolve to anything. Is it misspelt?'.
This is incorrect, the method is defined above and IntelliJ is capable of finding it, at least. Is it caused by the generics, perhaps? In the sense that the 'M' used in the javadoc comment is seen as different than the type parameter M.
Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
See the example above.
What version of Error Prone are you using?
2.4.0
Have you found anything relevant by searching the web?
No; it seems fairly straightforward. :) I will suppress it in our code in the mean time.

Javadoc disagrees with IntelliJ in this case.
$ javadoc --version
javadoc 11.0.6

$ cat InitSpec.java
public interface InitSpec<M> {
  void when(M model);

  /** {@link #when(M)} */
  void whenInit(M model);
}

$ javadoc InitSpec.java
...
InitSpec.java:4: error: reference not found
  /** {@link #when(M)} */
             ^
...

I think you're right to pin this on generics. My understanding is that you refer to the parameter type by its erasure:
  /** {@link #when(Object)} */```

Javadoc seems to like that. I suspect that Error Prone will, too, but I haven't tested.

Possibly Javadoc will support using the type parameter's name someday, but the feature request for that has been open forever: https://bugs.openjdk.java.net/browse/JDK-6422214

A bug from 2006, forever indeed. :)
Thanks for the explanation, I will use the erasure workaround instead!

authors: cpovirk pettermahlen

repo: google/error-prone


text:
Android compilation failing in AS Giraffe - BugPattern: AlmostJavadoc

with this info:
error: An unhandled exception was thrown by the Error Prone static analysis plugin.
the project compiles without issue on AS ElectricEel.
AS info
Android Studio Giraffe | 2022.3.1 Canary 2
Build #AI-223.7571.182.2231.9523943, built on January 25, 2023
Runtime version: 17.0.5+0-17.0.5b653.25-9484017 aarch64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
macOS 12.6.2
GC: G1 Young Generation, G1 Old Generation
Memory: 3072M
Cores: 10
Metal Rendering is ON
Registry:
    external.system.auto.import.disabled=true
    debugger.new.tool.window.layout=true
    ide.text.editor.with.preview.show.floating.toolbar=false
    ide.instant.shutdown=false
    ide.experimental.ui=true

Non-Bundled Plugins:
    net.sf.opk.avro-schema-support (213.4.0)
    com.intellij.ideolog (203.0.30.0)
    Key Promoter X (2022.3.1)
    com.suusan2go.kotlin-fill-class (1.0.17)
    org.intellij.qodana (223.7571.203)

error details
error-prone version: 2.4.0
     BugPattern: AlmostJavadoc
     Stack Trace:
     java.lang.NoSuchFieldError: reader
  	at com.google.errorprone.util.ErrorProneTokens$CommentSavingTokenizer.processComment(ErrorProneTokens.java:85)
  	at jdk.compiler/com.sun.tools.javac.parser.JavaTokenizer.readToken(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.parser.Scanner.nextToken(Unknown Source)
  	at com.google.errorprone.util.ErrorProneTokens.getTokens(ErrorProneTokens.java:57)
  	at com.google.errorprone.util.ErrorProneTokens.getTokens(ErrorProneTokens.java:74)
  	at com.google.errorprone.util.ErrorProneTokens.getTokens(ErrorProneTokens.java:65)
  	at com.google.errorprone.bugpatterns.javadoc.AlmostJavadoc.matchCompilationUnit(AlmostJavadoc.java:80)
  	at com.google.errorprone.scanner.ErrorProneScanner.processMatchers(ErrorProneScanner.java:451)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:557)
  	at com.google.errorprone.scanner.ErrorProneScanner.visitCompilationUnit(ErrorProneScanner.java:152)
  	at jdk.compiler/com.sun.tools.javac.tree.JCTree$JCCompilationUnit.accept(Unknown Source)
  	at jdk.compiler/com.sun.source.util.TreePathScanner.scan(Unknown Source)
  	at com.google.errorprone.scanner.Scanner.scan(Scanner.java:58)
  	at com.google.errorprone.scanner.ErrorProneScannerTransformer.apply(ErrorProneScannerTransformer.java:43)
  	at com.google.errorprone.ErrorProneAnalyzer.finished(ErrorProneAnalyzer.java:152)
  	at jdk.compiler/com.sun.tools.javac.api.MultiTaskListener.finished(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.flow(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.main.JavaCompiler.compile(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.lambda$doCall$0(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.invocationHelper(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.doCall(Unknown Source)
  	at jdk.compiler/com.sun.tools.javac.api.JavacTaskImpl.call(Unknown Source)
  	at org.gradle.internal.compiler.java.IncrementalCompileTask.call(IncrementalCompileTask.java:89)
  	at org.gradle.api.internal.tasks.compile.AnnotationProcessingCompileTask.call(AnnotationProcessingCompileTask.java:94)
  	at org.gradle.api.internal.tasks.compile.ResourceCleaningCompilationTask.call(ResourceCleaningCompilationTask.java:57)
  	at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:54)
  	at org.gradle.api.internal.tasks.compile.JdkJavaCompiler.execute(JdkJavaCompiler.java:39)
  	at org.gradle.api.internal.tasks.compile.daemon.AbstractDaemonCompiler$CompilerWorkAction.execute(AbstractDaemonCompiler.java:135)
  	at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:49)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:43)
  	at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:97)
  	at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:43)
  	at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:32)
  	at org.gradle.workers.internal.FlatClassLoaderWorker.run(FlatClassLoaderWorker.java:22)
  	at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:85)
  	at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:55)
  	at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
  	at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
  	at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:135)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
  	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
  	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
  	at java.base/java.lang.reflect.Method.invoke(Unknown Source)
  	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
  	at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
  	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
  	at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
  	at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
  	at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
  	at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
  	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
  	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
  	at org.gradle.internal.concurrent.ThreadFactoryImpl$ManagedThreadRunnable.run(ThreadFactoryImpl.java:61)
  	at java.base/java.lang.Thread.run(Unknown Source)

I think this was fixed by https://github.com/google/error-prone/commit/29708390e2124c10261a3643f3563a5e7f3f9a9f, upgrading to a newer version of Error Prone should resolve it

thanks! that did work.
still, error-prone tells to open an issue in this case. it may be useful to add to that message a section about upgrading error-prone, or a link to some FAQ-page.

authors: cushon noamfreeman


text:
Add Azure KeyVault signing support

Fixes GH-526

/assign @mikehelmick

repo: google/exposure-notifications-server


text:
Updating the doc

What does this PR do?
The main goal of this PR is to address a small change in the README documentation. I just feel the symbol ">" should be removed as the part of command when users are trying to copy-paste the commands in their respective terminals.
When I was Installing the package I had to delete the entire line as ">" was getting copy-pasted. I get the motivation behind it as it helps in better representing the terminal environment but as a user I am not able to copy-paste the command in its entirety.
Fixes # (issue)
Checklist

[ ✓] This PR fixes a minor issue (e.g.: typo or small bug) or improves the docs (you can dismiss the other
checks if that's the case).
[ ] This change is discussed in a Github issue/
discussion (please add a
link).
[ ] The documentation and docstrings adhere to the
documentation guidelines.
[ ] This change includes necessary high-coverage tests.
(No quality testing = no merge!)

Hey is this ready to be merged or should I add something more to the docs?

repo: google/flax


text:
Add *.rej to .gitignore

Tiny housekeeping thing; now that the HOWTOs HOWTO suggests using git apply --reject, adding *.rej files to .gitignore. I noticed that *.rej files were making their way into commits.

Looks like there's an issue with tensorboard. They just updated to v 2.2.2 today and something broke. Looking into it.

Waiting on #298.

Codecov Report

Merging #297 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #297   +/-   ##
=======================================
  Coverage   79.33%   79.33%
=======================================
  Files          34       34
  Lines        2255     2255
=======================================
  Hits         1789     1789
  Misses        466      466

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c2a6346...04bcb23. Read the comment docs.

Thanks for catching this!

authors: avital codecov-commenter danielsuo


text:
Catch 403 error

There is a 403 error thrown most times:

It seems that this error is part of an additional request that we make… If this is the result of a regular request we should better catch it and keep the console clean.

I don't think that's possible. We don't log or throw these errors programmatically. It's Chrome default behavior (these errors don't show up in Firefox) to log http client error status codes (4XX) to the console.

authors: hundekoerper pmast

repo: google/fusion-tables-archive-visualizer


text:
Please make semver compatible releases

Releases tagged e.g. v0.1.0 are important to play nice with the package management ecosystem. Thank you!

Done.

Joe, FWIW, it would have sufficed to just tag the commit. I don't think it's necessary to use the GH release feature, and I don't think it buys anything for Go. All the Go package management tools just look at tags.

All I did was:
$ git tag v0.1.0
$ git push --tags

on the commandline.
From what I can tell, GitHub created the "release" for me. Since it did that, I figured I'd populate the text for it.

Thanks! A winner is yuo!

authors: dsnet jba peterbourgon

repo: google/go-cmp


text:
Webhook event type pull_request_review_thread missing

It seems like GitHub is sending the webhook event type pull_request_review_thread when someone resolves a comment in a PR.
Webhook event schema can be seen here: https://github.com/octokit/webhooks/blob/d93eebf4c48dc3c36f9c7ceffab356fa444bf9fc/payload-schemas/api.github.com/pull_request_review_thread/resolved.schema.json
The event doesn't seem to be documented in Webhook events and payload though.

Thank you, @taisph !
Could you please ask GitHub tech support to document this webhook event type?

Thank you, @taisph ! Could you please ask GitHub tech support to document this webhook event type?

GitHub Support have created an internal issue to get the documentation updated.

Excellent, thank you, @taisph !
Do you want to make a PR for this, or shall I open it up for other contributors?

Excellent, thank you, @taisph ! Do you want to make a PR for this, or shall I open it up for other contributors?

I'll give it a shot.

authors: gmlewis taisph

repo: google/go-github


text:
Refactor context.Background in tests

This PR is in preparation of adding code coverage improvement PRs which will be much easier to write when the tests consistently create a single background context.
The only changes made in this PR are to pull out context.Background() into a separate assignment in the test files.

That was a lot of files. LGTM.

Yes, sorry for the churn, but this will make future coverage tests easier to maintain.
Thank you for all the reviews, @nightlark !  They are greatly appreciated!
Merging.


text:
Partial response request does not return all folder entries

- Request a list of all folders (my test site has 1000+ entries)
- Set Q to "mimeType = 'application/vnd.google-apps.folder' and trashed = false"
- Set Fields to "items(title,id,parents, userpermission)"

An error is returned on userpermission, not sure why. After leaving that field
out, the first batch (100 entries) is returned properly with just title,id, and
parents. However, NextPageToken field is null! If you don't specify a Fields
setting, then it works properly, and all entries are returned in multiple
batches.

1.9.0 Google APIs Client Library for .NET
Windows 8.1
Target .Net framework is 4.5
IDE is Visual Studio Prof. 2013 Update 4

Original issue reported on code.google.com by pe...@ccscantest.com on 17 Feb 2015 at 6:57

Closing as stale. Please open a new issue if follow-up is required.

authors: GoogleCodeExporter chrisdunelm

repo: google/google-api-dotnet-client


text:
Error in webpage

Hello, I wanted signal the error "For more infromtation" in
https://developers.google.com/api-client-library/dotnet/guide/aaa_oauth
page (the first light blue note).
I reported the error because Google documentation is so well done that I was sorry to see him :D

@chrisdunelm did we get an internal bug logged for this one?

I've just created an internal PR to fix this.
I don't know how long it will take to propagate externally.

Leave this open until its fixed?

Yes please.

No need to be so polite i don't have the power to close it you do :)

This typo is now fixed, @MatteoTasinato thanks for letting us know.

authors: LindaLawton MatteoTasinato chrisdunelm


text:
PHP Version Requirement on Installation Page

https://developers.google.com/api-client-library/php/start/installation states that PHP version 5.2.1 or greater is required. It looks like PHP version 5.4 or greater is required -- array notation [] was introduced in 5.4.

Thanks
Den 19:25 mån 12 sep 2016jmooring notifications@github.com skrev:

https://developers.google.com/api-client-library/php/start/installation
states that PHP version 5.2.1 or greater is required. It looks like PHP
version 5.4 or greater is required -- array notation [] was introduced in
5.4.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/google-api-php-client/issues/1050, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ARICyIGfqywAmQ3x-E8Zxpr4EOdjyTWEks5qpYm2gaJpZM4J60es
.

The page has been updated. Thanks very much for pointing that out.

authors: emestud jmooring owneroverall

repo: google/google-api-php-client


text:
[Bug] CLIENT_ID_VAR in default_credentials.rb scoped incorrectly

It looks like #145 introduced a minor bug. When calling Google::Auth.get_application_default(scopes) there is a code path (new -> from_env -> make_creds(scope: scope))  that hits:
https://github.com/google/google-auth-library-ruby/pull/145/files#diff-213fdc02c21601908a9d0b021cbf527fR56 and raises
Message: <"Unable to read the credential file specified by GOOGLE_APPLICATION_CREDENTIALS: uninitialized constant Google::Auth::DefaultCredentials::CLIENT_ID_VAR\nDid you mean?  Google::Auth::ClientId">
---Backtrace---
/Users/dturner/.gem/ruby/2.3.3/gems/googleauth-0.6.3/lib/googleauth/credentials_loader.rb:94:in `rescue in from_env'
/Users/dturner/.gem/ruby/2.3.3/gems/googleauth-0.6.3/lib/googleauth/credentials_loader.rb:84:in `from_env'
/Users/dturner/.gem/ruby/2.3.3/gems/googleauth-0.6.3/lib/googleauth/application_default.rb:57:in `get_application_default'

I believe the fix is to change the scope of the constant e.g.: Google::Auth::CredentialsLoader::CLIENT_ID_VAR instead of just CLIENT_ID_VAR

Resolved https://github.com/google/google-auth-library-ruby/pull/154

Thank you

authors: TheRoyalTnetennba dturn

repo: google/google-auth-library-ruby


text:
Newbie - cannot build

Ubuntu 18.04 and bazel 0.23.2
Got this:
~/projects/graphd/graphd$ bazel build graphd
ERROR: /home/davew/.cache/bazel/_bazel_davew/e086e969cb9399a22ef830356847de91/external/go_sdk/BUILD.bazel:52:1: no such target '@io_bazel_rules_go//go:toolchain': target 'toolchain' not declared in package 'go'; however, a source directory of this name exists.  (Perhaps add 'exports_files(["toolchain"])' to go/BUILD, or define a filegroup?) defined by /home/davew/.cache/bazel/_bazel_davew/e086e969cb9399a22ef830356847de91/external/io_bazel_rules_go/go/BUILD.bazel and referenced by '@go_sdk//:go_android_arm'
ERROR: While resolving toolchains for target //graphd:graphd: invalid registered toolchain '@go_sdk//:go_android_arm': Analysis failed
ERROR: Analysis of target '//graphd:graphd' failed; build aborted: Analysis failed
INFO: Elapsed time: 3.304s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (10 packages loaded, 28 targets configured)
----end----
Get a similar issue on Win10...
Thanks,  Dave W

Go rules might be out of date?
Can you try replacing the http_archive rules in https://github.com/google/graphd/blob/master/WORKSPACE#L2-L6 to be:
http_archive(
    name = "io_bazel_rules_go",
    urls = ["https://github.com/bazelbuild/rules_go/releases/download/0.18.1/rules_go-0.18.1.tar.gz"],
    sha256 = "77dfd303492f2634de7a660445ee2d3de2960cbd52f97d8c0dffa9362d3ddef9",
)

?

Here is WORKSPACE that was successful:
----begin----
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
    name = "io_bazel_rules_go",
    url =
"https://github.com/bazelbuild/rules_go/releases/download/0.18.1/rules_go-0.18.1.tar.gz",
    sha256 =
"77dfd303492f2634de7a660445ee2d3de2960cbd52f97d8c0dffa9362d3ddef9",
)
http_archive(
    name = "bazel_gazelle",
    urls =
["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.14.0/bazel-gazelle-0.14.0.tar.gz"],
    sha256 =
"c0a5739d12c6d05b6c1ad56f2200cb0b57c5a70e03ebd2f7b87ce88cabf09c7b",
)
load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies",
"go_register_toolchains")
go_rules_dependencies()
go_register_toolchains()
load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")
gazelle_dependencies()
----end----
Note the ....."go:deps.bzl" versus the original "go:def.bzl" in github.
Thanks for your help.
Dave W
On 3/26/19 12:29 PM, teeler wrote:

Go rules might be out of date?
Can you try replacing the http_archive rules in
https://github.com/google/graphd/blob/master/WORKSPACE#L2-L6 to be:
|http_archive( name = "io_bazel_rules_go", urls =
["https://github.com/bazelbuild/rules_go/releases/download/0.18.1/rules_go-0.18.1.tar.gz"],
sha256 =
"77dfd303492f2634de7a660445ee2d3de2960cbd52f97d8c0dffa9362d3ddef9", ) |
?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/google/graphd/issues/26#issuecomment-476811680,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACZAMvyPkOR-Iuresq8nxhQFJVz3t6Qnks5vanUAgaJpZM4cLyrC.

Nice, wanna send the PR?

Just put committed it to:
https://github.com/cetaus/graphd/blob/master/WORKSPACE
From: teeler notifications@github.com
Sent: Tuesday, March 26, 2019 1:23 PM
To: google/graphd graphd@noreply.github.com
Cc: Dave Winters dave1667@gmail.com; Author author@noreply.github.com
Subject: Re: [google/graphd] Newbie - cannot build (#26)
Nice, wanna send the PR?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub https://github.com/google/graphd/issues/26#issuecomment-476833857 , or mute the thread https://github.com/notifications/unsubscribe-auth/ACZAMnt6jSy3lN2rx1pVsLJTJhmEfV84ks5vaoGqgaJpZM4cLyrC .  https://github.com/notifications/beacon/ACZAMi8sEMHNzkBvWf3_r2Eo0I9PTOJUks5vaoGqgaJpZM4cLyrC.gif

LG, send the pull request and i'll merge it
On Tue, Mar 26, 2019 at 2:23 PM Dave Winters notifications@github.com
wrote:

Just put committed it to:
https://github.com/cetaus/graphd/blob/master/WORKSPACE
From: teeler notifications@github.com
Sent: Tuesday, March 26, 2019 1:23 PM
To: google/graphd graphd@noreply.github.com
Cc: Dave Winters dave1667@gmail.com; Author author@noreply.github.com
Subject: Re: [google/graphd] Newbie - cannot build (#26)
Nice, wanna send the PR?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <
https://github.com/google/graphd/issues/26#issuecomment-476833857> , or
mute the thread <
https://github.com/notifications/unsubscribe-auth/ACZAMnt6jSy3lN2rx1pVsLJTJhmEfV84ks5vaoGqgaJpZM4cLyrC>
. <
https://github.com/notifications/beacon/ACZAMi8sEMHNzkBvWf3_r2Eo0I9PTOJUks5vaoGqgaJpZM4cLyrC.gif>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/graphd/issues/26#issuecomment-476858121, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AACxUx162G79d-KBu95XMoFEKu0cdlBaks5vao_CgaJpZM4cLyrC
.

repo: google/graphd


text:
Fixes multiselect task

Fixes #1658

@scolsen @JSunde  PTAL?

/gcbrun

/gcbrun

/gcbrun

/gcbrun

repo: google/ground-android


text:
Is this code neccessary if the input jpeg is already grayscale?

Dear All,
I am trying to understand processor.cc, specially the function bool Processor::ProcessJpegData().
I see an if statement to check if the input has cbcr colorspace HasYCbCrColorSpace().
And, before the function try to optimize the quantization table, I found the following code:
{
JPEGData jpg = jpg_in;
RemoveOriginalQuantization(&jpg, q_in);
OutputImage img(jpg.width, jpg.height);
img.CopyFromJpegData(jpg);
comparator_->Compare(img);
}
MaybeOutput(encoded_jpg);
My question is, if the above code neccessary if my input is always have ycbcr (non grayscale) and always yuv420? Because I also try to optimize the code for our own usage.
https://github.com/google/guetzli/blob/0b78c7cc8b1b6cbaaf3d08b1facb599bcec1d101/guetzli/processor.cc#L839
Thanks,
Sunny

Glad someone is still interested in this software (like I am! Don't expect any answer from anyone here). I only have one hint if you want to optimize the process: Try to further optimize the already optimized version at https://github.com/ianhuang-777/guetzli-cuda-opencl instead this one. I have to say the added '--c' command  reduce processing time by ~30% which is great.
The cuda and opencl optimization  works only till 6 MP big pictures. Anything bigger will abort the process so it's not usable.
Please let me know if you have any success in further optimizing guetzli.

thanks @koboltzz
I intend to optimize the code so that it can be easier for hardware engineer to translate the implementation into one of the hardware description language such as Verilog.
Do you know any active Guetzli forum or website that I can post any question ?

No since for the most people guetzli is too heavy to RAM and CPU, it's kinda dead. The only people that could help you are Google employees and they don't work on guetzli anymore.

thanks again I may have to spend more time to understand the code then.
Refer to my original question, assume we don't modify any parameter.
By default, params_.force_420 and params_.try_420 are set to false,
theforefore, the following for loop are always executed:
for (int downsample = force_420; downsample <= try_420; ++downsample) {
Therefore, the above code can be treated as redundant if we don't modify the parameter.

authors: koboltzz ppoocde

repo: google/guetzli


text:
Not possible to execute the XRT code in the bazel test environment

In a new end to end example, we try to evaluate tfhe-rs-bool code on an FPGA.  To communicate with an Xilinx Alveo FPGA we use the XRT library.
There is a problem during the initialisation of the FPGA device in the Rust code.
let dev = xrtDeviceOpen(0);
println!("xrt Dev open");

let file_path = CString::new(
    "/home/wout/heir/tests/tfhe_rs_bool/end_to_end_fpga/accel.xclbin"
).unwrap();

let xclbin = xrtXclbinAllocFilename(file_path.as_ptr());
xrtDeviceLoadXclbinHandle(dev, xclbin);

Note: All the functions that start with xrt are calls to the C-API of the XRT library.
Once this test is run in with bazel test ... , we got the following output:
# | XRT build version: 2.13.466
# | Build hash: f5505e402c2ca1ffe45eb6d3a9399b23a0dc8776
# | Build date: 2022-04-14 17:43:11
# | Git branch: 2022.1
# | PID: 47
# | UID: 1000
# | [Tue Jan 30 16:22:00 2024 GMT]
# | HOST: fatal runtime error: Rust cannot catch foreign exceptions

The problem already starts with the xrtDeviceOpen function, which won't execute.
The XRT library is clearly started from the Rust code, but from then it is unclear what happens. The error message are generated by the XRT lib, while bazel and/or cargo cannot process them.

@WoutLegiest can you post the bazel rule as you've currently set it up? Or perhaps push a draft PR with the changes so we can take a closer look?
I was reading some docs at https://xilinx.github.io/XRT/master/html/xrt_native_apis.html and see

XCL_DRIVER_DLLESPEC xrtDeviceHandle xrtDeviceOpenByBDF (const char * bdf) PCIe BDF identifying the device to open

One thing we can try is passing in the PCIE BDF (e.g., "0000:03:00.1" from the docs)
A bit more digging, and I expect there is a way to communicate with a PCI=connected device via standard unix filesystem mounts.
Reading the Linux kernel docs, https://docs.kernel.org/PCI/sysfs-pci.html, I suspect the device might be accessible at one of
/sys/devices/xxxx
/sys/bus/pci/devices/xxxx

If this is mounted as a traditional UNIX file, you might be able to make it work by adding these /sys/ paths to bazel's sandbox_writeable_path flag (maybe even adding --sandbox_writeable_path=/sys, but first I'd check to see if you can locate the exact device's location on the file system)

You might also try running findmnt to get the list of all mounted filesystems, and see if the FPGA is in there.

If that doesn't work, I wonder if we could find a minimal working example that I could set up with my machine, by plugging some other (cheap) FPGA into my desktop and tinkering with it. Would the XRT API work with every FPGA?

I already tried to run the xrtDeviceOpenByBDF function, same result. Also found the device location in the /sys/ folder, added it to sandbox path without any outcome.
I found it was possible to run xbutil program from an MLIR file, so talking to the FPGA from the bazel sandbox is possible and works correctly. Possibly the calling of Rust -> C -> FPGA will introduce the problem.

If that doesn't work, I wonder if we could find a minimal working example that I could set up with my machine, by plugging some other (cheap) FPGA into my desktop and tinkering with it. Would the XRT API work with every FPGA?

The XRT library is designed for the AMD Alveo cards, which are all PCIe cards that can be plugged into any pc. More specifically, the u55c, u250, u280 are cards with large FPGA on them, sadly none of them are cheap.

I suspect we have given up on trying to fix this, so closing.

repo: google/heir


text:
Add basic HEIR website

It's a hugo site with the docsy theme, some basic placeholder content, and a github workflow that publishes to the gh-pages branch when main is touched.

Tested at https://j2kun.github.io/heir/


text:
Failed to fuse { indexed_generic -> tensor_reshape -> tensor_reshape -> generic }

This is a simplified pattern from unidirectional_lstm.mlir. Currently the dispatch region is not formed as this, but this will happen after relaxing the condition, as mentioned in #3562.
$ iree-opt --iree-codegen-hlo-to-linalg-pipeline ~/fuse.mlir
module {
  func @main_ex_dispatch_16() attributes {vkspv.num_workgroups_fn = @main_ex_dispatch_16__num_workgroups__} {
    %c0 = constant 0 : index
    %cst_1 = constant dense<1.000000e+00> : tensor<1x10xf32>
    %4 = hal.interface.load.tensor @legacy_io::@arg4, offset = %c0 {operand_result_index = 4 : i32} : tensor<5x1x1xf32>
    %5 = hal.interface.load.tensor @legacy_io::@arg5, offset = %c0 {operand_result_index = 5 : i32} : tensor<i32>
    %6 = "mhlo.torch_index_select"(%4, %5) {batch_dims = 0 : i64, dim = 0 : i64} : (tensor<5x1x1xf32>, tensor<i32>) -> tensor<1x1xf32>
    %7 = "mhlo.reshape"(%6) : (tensor<1x1xf32>) -> tensor<1xf32>
    %8 = "mhlo.broadcast_in_dim"(%7) {broadcast_dimensions = dense<0> : tensor<1xi64>} : (tensor<1xf32>) -> tensor<1x10xf32>
    %9 = mhlo.multiply %8, %cst_1 : tensor<1x10xf32>
    hal.interface.store.tensor %9, @legacy_io::@ret0, offset = %c0 {operand_result_index = 6 : i32} : tensor<1x10xf32>
    return
  }
  hal.interface @legacy_io attributes {sym_visibility = "private"} {
    hal.interface.binding @arg4, set=0, binding=4, type="StorageBuffer", access="Read"
    hal.interface.binding @arg5, set=0, binding=5, type="StorageBuffer", access="Read"
    hal.interface.binding @ret0, set=0, binding=6, type="StorageBuffer", access="Write|Discard"
  }
}

This will be lowered to Linalg on tensors:
func @main_ex_dispatch_16() attributes {vkspv.num_workgroups_fn = @main_ex_dispatch_16__num_workgroups__} {
  %c0 = constant 0 : index
  %cst = constant dense<1.000000e+00> : tensor<1x10xf32>
  %0 = hal.interface.load.tensor @legacy_io::@arg4, offset = %c0 {operand_result_index = 4 : i32} : tensor<5x1x1xf32>
  %1 = hal.interface.load.tensor @legacy_io::@arg5, offset = %c0 {operand_result_index = 5 : i32} : tensor<i32>
  %2 = linalg.indexed_generic {indexing_maps = [affine_map<(d0, d1) -> ()>, affine_map<(d0, d1) -> (d0, d1)>], iterator_types = ["parallel", "parallel"]} ins(%1 : tensor<i32>) {
  ^bb0(%arg0: index, %arg1: index, %arg2: i32):  // no predecessors
    %6 = index_cast %arg2 : i32 to index
    %7 = extract_element %0[%6, %arg0, %arg1] : tensor<5x1x1xf32>
    linalg.yield %7 : f32
  } -> tensor<1x1xf32>
  %3 = linalg.tensor_reshape %2 [affine_map<(d0, d1) -> (d0, d1)>] : tensor<1x1xf32> into tensor<1xf32>
  %4 = linalg.generic {indexing_maps = [affine_map<(d0, d1) -> (d0)>, affine_map<(d0, d1) -> (d0, d1)>], iterator_types = ["parallel", "parallel"]} ins(%3 : tensor<1xf32>) {
  ^bb0(%arg0: f32):  // no predecessors
    linalg.yield %arg0 : f32
  } -> tensor<1x10xf32>
  %5 = linalg.generic {indexing_maps = [affine_map<(d0, d1) -> (d0, d1)>, affine_map<(d0, d1) -> (d0, d1)>, affine_map<(d0, d1) -> (d0, d1)>], iterator_types = ["parallel", "parallel"]} ins(%4, %cst : tensor<1x10xf32>, tensor<1x10xf32>) {
  ^bb0(%arg0: f32, %arg1: f32):  // no predecessors
    %6 = mulf %arg0, %arg1 : f32
    linalg.yield %6 : f32
  } -> tensor<1x10xf32>
  hal.interface.store.tensor %5, @legacy_io::@ret0, offset = %c0 {operand_result_index = 6 : i32} : tensor<1x10xf32>
  return
}

The later LinalgFoldUnitExtentDims pass creates two reshape op like:
  %2 = linalg.indexed_generic {indexing_maps = [affine_map<(d0, d1) -> ()>, affine_map<(d0, d1) -> (d0, d1)>], iterator_types = ["parallel", "parallel"]} ins(%1 : tensor<i32>) {
  ^bb0(%arg0: index, %arg1: index, %arg2: i32):  // no predecessors
    %8 = index_cast %arg2 : i32 to index
    %9 = extract_element %0[%8, %arg0, %arg1] : tensor<5x1x1xf32>
    linalg.yield %9 : f32
  } -> tensor<1x1xf32>
  %3 = linalg.tensor_reshape %2 [affine_map<(d0, d1) -> (d0, d1)>] : tensor<1x1xf32> into tensor<1xf32>
  %4 = linalg.tensor_reshape %3 [] : tensor<1xf32> into tensor<f32>
  %5 = linalg.generic {indexing_maps = [affine_map<(d0) -> ()>, affine_map<(d0) -> (d0)>], iterator_types = ["parallel"]} ins(%4 : tensor<f32>)
 {
  ^bb0(%arg0: f32):  // no predecessors
    linalg.yield %arg0 : f32
  } -> tensor<10xf32>
  %6 = linalg.generic {indexing_maps = [affine_map<(d0) -> (d0)>, affine_map<(d0) -> (d0)>, affine_map<(d0) -> (d0)>], iterator_types = ["parallel"]} ins(%5, %cst : tensor<10xf32>, tensor<10xf32>) {
  ^bb0(%arg0: f32, %arg1: f32):  // no predecessors
    %8 = mulf %arg0, %arg1 : f32
    linalg.yield %8 : f32
  } -> tensor<10xf32>

This pattern is not fused in FusionOfTensorOpsPass. I will look into details later.

Taking this one

authors: MaheshRavishankar hanhanW

repo: google/iree


text:
keyword_spotting_tests failing LLVM backend

Repro:
 bazel run //integrations/tensorflow/e2e/keras:keyword_spotting_tests__model__att_mh_rnn__target_backends__iree_llvmaot

*** END MANGLED STACK TRACE ***
*** Begin stack trace ***
tensorflow::CurrentStackTrace()
    call_ex_dispatch_34

*** End stack trace ***

Pretty sure this is obsolete. I enabled more of the keyword_spotting_tests yesterday, and all the current failures appear to be upstream of IREE (in the python source or in the TF->MLIR lowering): https://github.com/google/iree/commit/a3cb661e32b52aa0c608b416a5ec29fbe67b977a

authors: ScottTodd asaadaldien


text:
Updated validatoin.py  python script

Updated validatoin.py  python script
Fixed the behaviour of validate_user function in validation.py.
Closes: #1
Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

[ ] Tests pass
[ ] Appropriate changes to README are included in PR

Update validate_user function in validation.py file

@googlebot I signed it!

repo: google/it-cert-automation-practice


text:
mio commit 1

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

[x] Tests pass
[x] Appropriate changes to README are included in PR

@googlebot I signed it!


text:
Closes: #1

Updated validations.py python script.
Fixed the behavior of validate_user function in validations.py.
Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

[x] Tests pass
[x] Appropriate changes to README are included in PR

@googlebot I signed it!

@googlebot I signed it!


text:
Closes: #1

Updated validations.py python script.
Fixed the behavior of calidate_user funtion in validations.py.
Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

[ ] Tests pass
[ ] Appropriate changes to README are included in PR

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
📝 Please visit https://cla.developers.google.com/ to sign.
Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.
What to do if you already signed the CLA
Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ Googlers: Go here for more info.

@googlebot I replied!


text:
np.linalg.inv() on GPU

Hi all,
I was expecting to see a difference in terms of performance when running the np.linalg.inv() on a GPU with respect to the CPU case but I see no difference.
Isn't GPU acceleration supported for np.linalg.inv()?
Cheers,

Yes, a GPU should be capable of being significantly faster, at least for sufficiently large matrices and/or batches of matrices. JAX does use GPU acceleration for solve. Keep in mind that sometimes it can be hard to realize the full speed of GPUs with XLA, due to some per-operation overhead.
Can you share what your benchmark looks like? Don't forget about the overhead of asynchronous dispatch: https://jax.readthedocs.io/en/latest/async_dispatch.html

Yes, @shoyer is correct: matrix inversion should be accelerated on GPU. Please share a benchmark that reproduces the problem. Please also read the async dispatch documentation; otherwise you may not be measuring the right timing.

Closing since we don't have a benchmark. Feel free to reopen with a benchmark!

authors: hawkinsp minaskar shoyer

repo: google/jax


text:
Running Pallas issue

Description
Installing jax, jaxlib, and jax-triton nightly builds cause the following error for the examples in the documentation:
File "/home/mehdi/Repos/venvs/jax-dev/lib/python3.10/site-packages/jax/_src/pallas/triton/lowering.py", line 1721, in pallas_call_lowering
backend_config=zlib.compress(kernel_call.to_proto(serialized_metadata)),
TypeError: to_proto(): incompatible function arguments. The following argument types are supported:
1. (self: jaxlib.cuda._triton.TritonKernelCall, arg0: str, arg1: str) -> bytes
Invoked with: <jaxlib.cuda._triton.TritonKernelCall object at 0x7faebfce6ef0>, b''
I0000 00:00:1692795278.467295  180484 tfrt_cpu_pjrt_client.cc:469] TfrtCpuClient destroyed.
Package Version Editable project location
absl-py 1.4.0
filelock 3.12.2
jax 0.4.15
jax-triton 0.1.4 /home/mehdi/Repos/jax-triton
jaxlib 0.4.15.dev20230822+cuda12.cudnn89
ml-dtypes 0.2.0
numpy 1.25.2
opt-einsum 3.3.0
pip 22.0.2
scipy 1.11.2
setuptools 59.6.0
triton-nightly 2.1.0.dev20230714011643
What jax/jaxlib version are you using?
No response
Which accelerator(s) are you using?
GPU
Additional system info
No response
NVIDIA GPU info
2x RTX ada 6000 (CUDA 12.2)

seems like a versioning issue, I've run into tonnes of those using jax-triton due to its use of nightly triton.

Since pallas is integrated into JAX now, there shouldn't be a need to install jax-triton. If you don't install triton or only install triton with the nightly build from the jax-triton repo the error is:
NotImplementedError: MLIR translation rule for primitive 'pallas_call' not found for platform cuda

Since pallas is integrated into JAX now, there shouldn't be a need to install jax-triton.

This is true in principle but we haven't yet done this.
We still need a Triton dependency because we need to use the Triton compiler with Pallas. That is why we depend on jax-triton at the moment. Jax-triton also contains specific logic for creating Triton custom calls.
In the future, we can decouple the two by using the Triton compiler that already ships with XLA. Unfortunately this is a large undertaking because there are no Python bindings exposed and it would take some work to add them.

So the way to run Pallas (currently):
Install JAX (head) and build jaxlib (head) + install jax-triton nightly?

Install JAX (head) and build jaxlib (head) + install jax-triton nightly?

Yes, but unfortunately, jax-triton works with not exactly a Triton nightly but close to Triton nightly. As of today (8/29/23) here is a set of pinned versions I tested and should work:
-e git+ssh://git@github.com/sharadmv/jax@7ec2a8835ecc4ffcb632b43cc47c892d1b323d47#egg=jax
-e git+ssh://git@github.com/jax-ml/jax-triton.git@321a19e901ee318a43345485a47e711944011c77#egg=jax_triton
jaxlib @ https://storage.googleapis.com/jax-releases/nightly/cuda12/jaxlib-0.4.15.dev20230822+cuda12.cudnn89-cp310-cp310-manylinux2014_x86_64.whl
nvidia-cublas-cu12==12.2.4.5
nvidia-cuda-cupti-cu12==12.2.131
nvidia-cuda-nvcc-cu12==12.2.128
nvidia-cuda-nvrtc-cu12==12.2.128
nvidia-cuda-runtime-cu12==12.2.128
nvidia-cudnn-cu12==8.9.4.25
nvidia-cufft-cu12==11.0.8.91
nvidia-cusolver-cu12==11.5.1.129
nvidia-cusparse-cu12==12.1.2.129
nvidia-nvjitlink-cu12==12.2.128
triton-nightly==2.1.0.dev20230722054455

Hi Sarad,
Thanks. I'll give it a try. There are potentially some cool applications of Pallas in XLB that I am planning to explore.
It would be nice to have installation documented by the next release version of JAX.

2023-08-30 14:00:03.110213: F external/xla/xla/python/jax_jit.cc:134] Check failed: global_state.enable_memories.has_value()

This looks like an unrelated issue unfortunately. I will try to triage it.

jaxlib 0.4.25 no longer uses Triton Python APIs for lowering Pallas kernels on GPU, so I will close this issue.

authors: jon-chuang mehdiataei sharadmv superbobry


text:
TypeError: No constant handler for type: <class 'jax.core.Token'>

Details can be found here.
https://github.com/mpi4jax/mpi4jax/pull/111

@mattjj


text:
Add erfcx

Closes https://github.com/google/jax/issues/1987.

So this is a reasonable approach, but I think the reason that scipy.special.erfcx exists is because the straightforward way to compute it leads to overflow issues in the square exponential:
import numpy as np
from scipy import special

def erfcx(x):
  return np.exp(x ** 2) * special.erfc(x)

print(erfcx(30), special.erfcx(30))
# nan 0.018795888861416754

I think that a jax implementation should implement a version that prevents these kinds of overflows. #1987 has some discussion of some possible approaches.

So this is a reasonable approach, but I think the reason that scipy.special.erfcx exists is because the straightforward way to compute it leads to overflow issues in the square exponential:
import numpy as np
from scipy import special

def erfcx(x):
  return np.exp(x ** 2) * special.erfc(x)

print(erfcx(30), special.erfcx(30))
# nan 0.018795888861416754

I think that a jax implementation should use an approach that prevents these kinds of overflows. #1987 has some discussion of some possible approaches.

I just miss the discussion there. Thanks for reminder! I'll update it soon.

@jakevdp Other than the link in the issue, the scipy's implementation seems more robust, but requires a hardcoded expansion table. Do you think it's better in order for numerical stability?
https://github.com/scipy/scipy/blob/master/scipy/special/Faddeeva.cc#L872-L1297

authors: WindQAQ jakevdp


text:
Fix 'atleast_{1,2,3}d' for scalars

Fixes #634.
This still doesn't address the issue that the testing suite doesn't cover the case of int or float literals as input.

Wow, thanks!
Can you add a test case, if even just a manual one that only covers one example? That is, at some point we should improve the test suite to check Python literal types, but for now it would be good just to cover the change made in this PR with at least one test. I'm guessing you tested this yourself, even interactively in a repl, so you could just dump that into a test case somewhere.
What do you think?

Done. I think this fix will have to be done in a few other functions too, but I haven't checked yet.
I'm not too sure how to add testing for these. I was hoping they could go in scalar_shapes in tests/lax_numpy_test.py:45, but rng((), float) has result dtype float64 (for jtu.rand_default()), so there might need to be special cases to handle these.

This looks great! Testing/fixing Python scalars more broadly is something we should do.

authors: hawkinsp kroq-gar78 mattjj


text:
heap-buffer-overflow in jpegli's TokenizeACRefinementScan

Describe the bug
Under certain conditions, num_refinement_bits is computed wrongly causing a heap-buffer-overflow in jpegli's TokenizeACRefinementScan in https://github.com/google/jpegli/blob/main/lib/jpegli/entropy_coding.cc#L255
To Reproduce
There is a need to use a specific scan_info to trigger it. I was able to reproduce it with the following patch:
--- a/jpeg-xl/lib/extras/enc/jpegli.cc
+++ b/jpeg-xl/lib/extras/enc/jpegli.cc
@@ -41,6 +41,19 @@

 namespace {

+constexpr auto kScanConfig = to_array<jpeg_scan_info>({
+    {3, {0, 1, 2}, 0, 0, 0, 1},  //
+    {1, {0}, 1, 5, 0, 2},        //
+    {1, {2}, 1, 63, 0, 1},       //
+    {1, {1}, 1, 63, 0, 1},       //
+    {1, {0}, 6, 63, 0, 2},       //
+    {1, {0}, 1, 63, 2, 1},       //
+    {3, {0, 1, 2}, 0, 0, 1, 0},  //
+    {1, {2}, 1, 63, 1, 0},       //
+    {1, {1}, 1, 63, 1, 0},       //
+    {1, {0}, 1, 63, 1, 0}        //
+});
+
 void MyErrorExit(j_common_ptr cinfo) {
   jmp_buf* env = static_cast<jmp_buf*>(cinfo->client_data);
   (*cinfo->err->output_message)(cinfo);
@@ -448,7 +461,9 @@
     } else {
       jpegli_set_distance(&cinfo, jpeg_settings.distance, TRUE);
     }
-    jpegli_set_progressive_level(&cinfo, jpeg_settings.progressive_level);
+    //jpegli_set_progressive_level(&cinfo, jpeg_settings.progressive_level);
+    cinfo.scan_info = kScanConfig.data();
+    cinfo.num_scans = kScanConfig.size();
     cinfo.optimize_coding = TO_JXL_BOOL(jpeg_settings.optimize_coding);
     if (!jpeg_settings.app_data.empty()) {
       // Make sure jpegli_start_compress() does not write any APP markers.

Steps to reproduce the behavior:

Apply the above patch
Run build cjpegli with ASAN and run with some sample images (Will provide example image if needed)

Expected behavior
num_refinement_bits is computed correctly and there is no heap-buffer-overflow
Environment

Centos 5
Compiler: LLVM 15
CPU type: x86_64
jpeg-xl version 0.10.2

@haixia-meta proposed a possible fix:
@@ -441,18 +441,19 @@
   std::vector<int> processed(cinfo->num_scans);
   size_t max_refinement_tokens = 0;
   size_t num_refinement_bits = 0;
-  int num_refinement_scans[DCTSIZE2] = {};
+  int num_refinement_scans[kMaxComponents][DCTSIZE2] = {};
   int max_num_refinement_scans = 0;
   for (int i = 0; i < cinfo->num_scans; ++i) {
     const jpeg_scan_info* si = &cinfo->scan_info[i];
     ScanTokenInfo* sti = &m->scan_token_info[i];
     if (si->Ss > 0 && si->Ah == 0 && si->Al > 0) {
       int offset = m->ac_ctx_offset[i];
+      int comp_idx = si->component_index[0];
       TokenizeScan(cinfo, i, offset, sti);
       processed[i] = 1;
       max_refinement_tokens += sti->num_future_nonzeros;
       for (int k = si->Ss; k <= si->Se; ++k) {
-        num_refinement_scans[k] = si->Al;
+        num_refinement_scans[comp_idx][k] = si->Al;
       }
       max_num_refinement_scans = std::max(max_num_refinement_scans, si->Al);
       num_refinement_bits += sti->num_nonzeros;
@@ -475,15 +476,17 @@
     size_t new_refinement_bits = 0;
     for (int i = 0; i < cinfo->num_scans; ++i) {
       const jpeg_scan_info* si = &cinfo->scan_info[i];
+      int comp_idx = si->component_index[0];
       ScanTokenInfo* sti = &m->scan_token_info[i];
       if (si->Ss > 0 && si->Ah > 0 &&
-          si->Ah == num_refinement_scans[si->Ss] - j) {
+          si->Ah == num_refinement_scans[comp_idx][si->Ss] - j) {
         int offset = m->ac_ctx_offset[i];
         TokenizeScan(cinfo, i, offset, sti);
         processed[i] = 1;
         new_refinement_bits += sti->num_nonzeros;
       }
     }
+    printf("%llu ?= %llu\n", m->next_refinement_bit, refinement_bits + num_refinement_bits);
     JXL_DASSERT(m->next_refinement_bit ==
                 refinement_bits + num_refinement_bits);
     num_refinement_bits += new_refinement_bits;

and he explains the problem as

[the code] only has a single array int num_refinement_scans[DCTSIZE2] and is supposed to find out for each DCT coefficient how many refinement scans there are. the problem is that there are 3 color components and this can differ between component index. for example scan #2 and #3 has Ah = 0 and Al = 1 for chroma components, but the least significant bit is refined at scans #7 and #8.

repo: google/jpegli


text:
Increase minimum required version of clang to 3.6.

This is intended to address #2874.

I think we concluded that 3.6 should be sufficient. I know 3.8 works.

repo: google/kythe


text:
Upgrade pulldown-cmark to 0.10.2

The latest version of pulldown-cmark-to-cmark now supports pulldown-cmark. This should allow us to upgrade both dependencies.
Had to adjust TagEnd handling and change some types to fix the build failures.
Information on breaking changes in pulldown-cmark is available here. Not sure if I am missing anything. I hope that the tests we have are extensive enough to catch any issues.

Codecov Report
Attention: Patch coverage is 85.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 90.27%. Comparing base (79da7bf) to head (44d639f).

:exclamation: Current head 44d639f differs from pull request most recent head 9b8b1d7. Consider uploading reports for the commit 9b8b1d7 to get more accurate results

Files
Patch %
Lines

i18n-helpers/src/lib.rs
83.92%
0 Missing and 9 partials :warning:

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
- Coverage   90.58%   90.27%   -0.31%
==========================================
  Files          12       12
  Lines        3133     3034      -99
  Branches     3133     3034      -99
==========================================
- Hits         2838     2739      -99
+ Misses        211      207       -4
- Partials       84       88       +4

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

authors: codecov-commenter kdarkhan


text:
Add AgX tone mapping

This adds AgX tone mapping as an option, mostly for comparison and because it's now easy. It has some great improvements over ACES, but for e-commerce it's still far too desaturated. I've also added it as an option in our editor, and made Commerce the editor's default, in preparation for it becoming MV's default in our next major release (which won't be this one). I also added an example to make comparison easy. FYI @donmccurdy @mrdoob.

Two notes (feel free to integrate or ignore them).

ACES is a film industry standard that is widely used...

I think this might be unnecessarily generous to ACES Filmic. While various ACES standards are commonly used in the film industry for working with imagery, I do not believe the ACES Filmic Tone Mapping is one of them. How it became prevalent in realtime, I'm not sure.

[AgX] has the same film-centric drawback of significant desaturation.

Note that introduction of Looks providing (among other things) contrast choices will be a high priority, and I consider it an essential part of supporting AgX. Not sure if Looks are factored into your evaluation?

Thanks for the info! I'll try to clean up the language a bit. I don't know anything about Looks - what can they do? A tone mapper is already an arbitrary function, so if AgX encompasses another arbitrary function called a Look, then I'm not sure what AgX specifies. Any info you have would be appreciated.

@donmccurdy In particular, if you know of any way to construct a "Look" that would make AgX similar to my Commerce tone mapper, that would be very interesting.

I'll borrow some phrasing from Romain —

... a bigger problem I think is that the tone mapper is often used as a look. Instead the tone mapper should do as little as possible (compress the dynamic range, hopefully with knobs) but the look should come from color grading.

I think AgX and Commercial are alike in trying to do less, and that's a good thing.
Troy has described the default Look from AgX this way:

... the default is an attempt to pack high picture signal density, which means that folks will probably want to push the “contrast” ever so slightly; it’s impossible to go the other way.

In the case of AgX, Looks provide some simple color grading presets. I'm unsure, so far, what the pros/cons of having such presets are, compared to exposing a full color grading API.
As for constructing a Look with less desaturation in AgX, I suspect that the "Punchy" or higher contrast looks would move in that direction, but we haven't added them to three.js yet.

Gotcha, so more of a semantic distinction. Is there a "standard" way to apply contrast, or is that curve another black art like tone mapping?

There exists, at least, an "easy" way:
https://github.com/mrdoob/three.js/blob/208f5e48adb750e15c4fb5167f492b414d8bd5bf/examples/jsm/shaders/BrightnessContrastShader.js#L46-L50
AgX does this a bit differently, see the approximation in https://github.com/google/filament/pull/7236.
Whatever the color grading operation, the decision about when to apply it is important. From Real-time Rendering, 4th Ed:

When real-time applications first adopted color grading, the display-referred approach was predominant. However, the scene-referred approach has since been gaining traction due to its higher visual quality.

With AgX, the Look is applied to a log encoding, before encoding to the output color space. I don't understand the practice of tone mapping and color grading design much beyond that, I'm afraid.

Yeah, that's looking pretty black art 🥲. I wonder if applying the Punchy look would be a good default, or at least if you know your output is sRGB? Looks simple enough.

Would need criteria to define "a good default", I think. I'm inclined toward using 'base contrast', as Blender does, as the starting point for new projects. But perhaps knowing the default would be replacing ACES Filmic in existing projects could change the situation. And I'm less in touch with the e-commerce discussions.

authors: donmccurdy elalish

repo: google/model-viewer


text:
Binaries using gflags::ParseCommandLineFlags are not working on MacOS

[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %make test_fz
bin/fz examples/flatzinc/golomb.fzn
ERROR: something wrong with flag 'flagfile' in file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc'.  One possibility: file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc' is being linked both statically and dynamically into this executable.
make: *** [makefiles/Makefile.cpp.mk:483: rfz_golomb] Error 1
[2]─[~/work/master]
[>_<]─corentinl@corentinl-macbookair %./bin/fz
ERROR: something wrong with flag 'flagfile' in file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc'.  One possibility: file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc' is being linked both statically and dynamically into this executable.
[1]─[~/work/master]
[>_<]─corentinl@corentinl-macbookair %

Currently investigating and doing some tests...
My guess:

ortools.dylib AND libfz.dylib are linked against libgflags.a so It may be possible that flagfile is included twice in bin/fz binary thus the bug.
libfz already depends of libortools -> not sure fz should depend on libortools at link time...

It happens with non flatzinc.
Laurent Perron | Operations Research | lperron@google.com | (33) 1 42 68 53
00
Le mar. 29 sept. 2020 à 11:20, Mizux notifications@github.com a écrit :

[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %make test_fz
bin/fz examples/flatzinc/golomb.fzn
ERROR: something wrong with flag 'flagfile' in file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc'.  One possibility: file '/Users/corentinl/work/master/dependencies/sources/gflags-2.2.2/src/gflags.cc' is being linked both statically and dynamically into this executable.
make: *** [makefiles/Makefile.cpp.mk:483: rfz_golomb] Error 1
Currently investigating and doing some tests...
My guess:

ortools.dylib AND libfz.dylib are linked against libgflags.a so It
may be possible that flagfile is included twice in bin/fz binary thus
the bug.
libfz already depends of libortools -> not sure fz should depend on
libortools at link time...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/2171, or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACUPL3IWXYOEXC4YGUKKJQ3SIGRFZANCNFSM4R5RGCEA
.

note: it's working on linux (except the fail of #2070) at least we don't have this 'flagfile' issue

On linux we have:
[0]─[~/work/master]
[^v^]─mizux@nuc10i7 %nm bin/fz | grep fileflag
[1]─[~/work/master]
[>_<]─mizux@nuc10i7 %nm lib/libfz.so | grep flagfile
[1]─[~/work/master]
[>_<]─mizux@nuc10i7 %nm lib/libortools.so | grep flagfile
0000000001f5e380 B _ZN3fLS14FLAGS_flagfileB5cxx11E
0000000001f5e410 b _ZN3fLSL10d_flagfileE
0000000001f5e408 b _ZN3fLSL10o_flagfileE
0000000001f5e3c0 b _ZN3fLSL10s_flagfileE
0000000001f5e400 b _ZN3fLSL16FLAGS_noflagfileE

so flagfile is only defined in libortools.so

while on macOS we have:
0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm bin/fz | grep flagfile
000000010001d5f8 S __ZN3fLS14FLAGS_flagfileE
000000010001d758 b __ZN3fLSL10d_flagfileE
000000010001d750 b __ZN3fLSL10o_flagfileE
000000010001d720 b __ZN3fLSL10s_flagfileE
000000010001d710 b __ZN3fLSL16FLAGS_noflagfileE
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm lib/libfz.dylib | grep flagfile
0000000000086cc0 S __ZN3fLS14FLAGS_flagfileE
0000000000086a48 b __ZN3fLSL10d_flagfileE
0000000000086a40 b __ZN3fLSL10o_flagfileE
0000000000086a10 b __ZN3fLSL10s_flagfileE
0000000000086a00 b __ZN3fLSL16FLAGS_noflagfileE
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm lib/libortools.dylib | grep flagfile
00000000020dd170 S __ZN3fLS14FLAGS_flagfileE
00000000020d6598 b __ZN3fLSL10d_flagfileE
00000000020d6590 b __ZN3fLSL10o_flagfileE
00000000020d6560 b __ZN3fLSL10s_flagfileE
00000000020d6550 b __ZN3fLSL16FLAGS_noflagfileE

Most examples/cpp/*.cc using gflags::ParseCommandLineFlags() are also impacted...

using cmake build:
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm build/lib/libortools.dylib | grep flagfile
0000000001451400 S __ZN3fLS14FLAGS_flagfileE
0000000001455050 b __ZN3fLSL10d_flagfileE
0000000001455020 b __ZN3fLSL10s_flagfileE
0000000001455010 b __ZN3fLSL16FLAGS_noflagfileE
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm build/bin/dobble_ls | grep flagfile
000000010001c608 S __ZN3fLS14FLAGS_flagfileE
000000010001c470 b __ZN3fLSL10d_flagfileE
000000010001c440 b __ZN3fLSL10s_flagfileE
000000010001c430 b __ZN3fLSL16FLAGS_noflagfileE

using make:
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm lib/libortools.dylib| grep flagfile
00000000020dd0b0 S __ZN3fLS14FLAGS_flagfileE
00000000020d6598 b __ZN3fLSL10d_flagfileE
00000000020d6590 b __ZN3fLSL10o_flagfileE
00000000020d6560 b __ZN3fLSL10s_flagfileE
00000000020d6550 b __ZN3fLSL16FLAGS_noflagfileE
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %nm bin/dobble_ls | grep flagfile
0000000100024c50 S __ZN3fLS14FLAGS_flagfileE
0000000100024aa8 b __ZN3fLSL10d_flagfileE
0000000100024aa0 b __ZN3fLSL10o_flagfileE
0000000100024a70 b __ZN3fLSL10s_flagfileE
0000000100024a60 b __ZN3fLSL16FLAGS_noflagfileE
[0]─[~/work/master]
[^v^]─corentinl@corentinl-macbookair %c++filt __ZN3fLSL10o_flagfileE
fLS::o_flagfile

note: cmake don't have the fLS::o_flagfile...

we must build gflags in Release (which is not the case in the Make based build system) but the case for ortools so I suspect we got two flagfile symbols + the extra symbols __ZN3fLSL10o_flagfileE which is not generated in release mode (i;e. -O3 -DNDEBUG)
i.e. add a -DCMAKE_BUILD_TYPE=Release
On my way to create a clean commit (ed we should do it for all our dependencies too) !
Please, wait and see.

I'm sorry to resurrect this old issue, but I'm running into this exact problem. I'm trying to setup a Homebrew formula which runs make third_party and then make cc fz. However, it runs exactly into this issue where I get the error posted in the initial description and when I run nm on the libgflags.a it outputs:
dekker1@Sodium lib % nm libgflags.a | grep flagfile
000000000000b780 S __ZN3fLS14FLAGS_flagfileE
000000000000b518 b __ZN3fLSL10d_flagfileE
000000000000b510 b __ZN3fLSL10o_flagfileE
000000000000b4e0 b __ZN3fLSL10s_flagfileE
000000000000b4d0 b __ZN3fLSL16FLAGS_noflagfileE

This seems to be the exact same problem, but when I look at the build log, it does add -DCMAKE_BUILD_TYPE=Release to the CMake command. Homebrew does setup a clean (and adjusted) build environment that is used during the build steps, but I do not see anything out of the ordinary, so I'm wondering if anyone would have any indication of what environmental variables would be able to induce this behaviour.
Alternatively I'd be happy (maybe even happier) to use the CMake build, but I don't think that contains the FlatZinc solver yet.

or-tools c++ is already on homebrew (non flatzinc).
Laurent Perron | Operations Research | lperron@google.com | (33) 1 42 68 53
00
Le jeu. 19 nov. 2020 à 07:45, Jip J. Dekker notifications@github.com a
écrit :

I'm sorry to resurrect this old issue, but I'm running into this exact
problem. I'm trying to setup a Homebrew formula which runs make
third_party and then make cc fz. However, it runs exactly into this issue
where I get the error posted in the initial description and when I run nm
on the libgflags.a it outputs:
dekker1@Sodium lib % nm libgflags.a | grep flagfile
000000000000b780 S __ZN3fLS14FLAGS_flagfileE
000000000000b518 b __ZN3fLSL10d_flagfileE
000000000000b510 b __ZN3fLSL10o_flagfileE
000000000000b4e0 b __ZN3fLSL10s_flagfileE
000000000000b4d0 b __ZN3fLSL16FLAGS_noflagfileE
This seems to be the exact same problem, but when I look at the build log,
it does add -DCMAKE_BUILD_TYPE=Release to the CMake command. Homebrew
does setup a clean (and adjusted) build environment that is used during the
build steps, but I do not see anything out of the ordinary, so I'm
wondering if anyone would have any indication of what environmental
variables would be able to induce this behaviour.
Alternatively I'd be happy (maybe even happier) to use the CMake build,
but I don't think that contains the FlatZinc solver yet.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/2171#issuecomment-730165920,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACUPL3KWLWAG43N6PHLESHDSQS5G7ANCNFSM4R5RGCEA
.

Hi Laurent,
I've seen that the C++ library was available in Homebrew. That one seems to use CMake to build OR Tools and doesn't seem to have the problem. I'm currently working on the installation of FlatZinc solvers using Homebrew (either being contained in their own Homebrew Tap or added as extensions to the official Formulae). Therefore, I'm trying to create a formula for OR Tools using make as that seems to be the way to create the FlatZinc binary and getting stuck on this problem.

are you building on master or on 8.0 ?
Gflags and glog are removed on master.
Laurent Perron | Operations Research | lperron@google.com | (33) 1 42 68 53
00
Le jeu. 19 nov. 2020 à 08:00, Jip J. Dekker notifications@github.com a
écrit :

Hi Laurent,
I've seen that the C++ library was available in Homebrew. That one seems
to use CMake to build OR Tools and doesn't seem to have the problem. I'm
currently working on the installation of FlatZinc solvers using Homebrew
(either being contained in their own Homebrew Tap or added as extensions to
the official Formulae). Therefore, I'm trying to create a formula for OR
Tools using make as that seems to be the way to create the FlatZinc binary
and getting stuck on this problem.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/2171#issuecomment-730172627,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACUPL3PECZMURLRBH3XOBJLSQS7BDANCNFSM4R5RGCEA
.

Furthermore, the cmake flatzinc part should be quite straightforward.
Just compile all files except parser_main.cc and fz.cc in a library, and
compile fz.cc as a binary.
There is nothing fancy there.
Laurent Perron | Operations Research | lperron@google.com | (33) 1 42 68 53
00
Le jeu. 19 nov. 2020 à 08:07, Laurent Perron lperron@google.com a écrit :

are you building on master or on 8.0 ?
Gflags and glog are removed on master.
Laurent Perron | Operations Research | lperron@google.com | (33) 1 42 68
53 00
Le jeu. 19 nov. 2020 à 08:00, Jip J. Dekker notifications@github.com a
écrit :

Hi Laurent,
I've seen that the C++ library was available in Homebrew. That one seems
to use CMake to build OR Tools and doesn't seem to have the problem. I'm
currently working on the installation of FlatZinc solvers using Homebrew
(either being contained in their own Homebrew Tap or added as extensions to
the official Formulae). Therefore, I'm trying to create a formula for OR
Tools using make as that seems to be the way to create the FlatZinc binary
and getting stuck on this problem.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/or-tools/issues/2171#issuecomment-730172627,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACUPL3PECZMURLRBH3XOBJLSQS7BDANCNFSM4R5RGCEA
.

I'll try to add flatzinc build to cmake today
please wait and see (and follow #2237)

Sorry to resurrect a dead issue, but I think I'm having a related problem when trying to add or-tools to my c++ project.
This is what my cmake looks like.
    include(FetchContent)
    FetchContent_Declare(
            ortools
            GIT_REPOSITORY https://github.com/google/or-tools.git
            GIT_TAG        stable
    )

    set(BUILD_DEPS ON)
    set(BUILD_SAMPLES OFF)
    set(BUILD_EXAMPLES OFF)
    FetchContent_MakeAvailable(ortools)
    set(requiredlibs ${requiredlibs} ortools::ortools)

When I try to build in debug, I get this issue at runtime:
ERROR: Something is wrong with flag 'flagfile' in file 'Users/connorriley/dial-a-ride-dispatcher/cmake-build-debug/_deps/absl-src/absl/flags/parse.cc'. One possibility: file 'Users/connorriley/dial-a-ride-dispatcher/cmake-build-debug/_deps/absl-src/absl/flags/parse.cc' is being linked both statically and dynamically into this executable. e.g. some files listed as srcs to a test and also listed as srcs of some shared lib deps of the same test.
I'm on an apple silicon mac.

authors: Dekker1 Mizux ctriley lperron

repo: google/or-tools


text:
Docker: Can't create ortools wheel package

On manylinux:
creating build/bdist.linux-x86_64/wheel/ortools-6.7.4954.dist-info/WHEEL
creating '/root/src/temp-python2.7/ortools/dist/ortools-6.7.4954-cp27-cp27m-linux_x86_64.whl' and adding '.' to it
...
Repairing ortools-6.7.4954-cp27-cp27m-linux_x86_64.whl
Traceback (most recent call last):
  File "/usr/local/bin/auditwheel", line 11, in <module>
    sys.exit(main())
  File "/opt/_internal/cpython-3.6.4/lib/python3.6/site-packages/auditwheel/main.py", line 49, in main
    rval = args.func(args, p)
  File "/opt/_internal/cpython-3.6.4/lib/python3.6/site-packages/auditwheel/main_repair.py", line 60, in execute
    wheel_abi = analyze_wheel_abi(args.WHEEL_FILE)
  File "/opt/_internal/cpython-3.6.4/lib/python3.6/site-packages/auditwheel/wheel_abi.py", line 73, in analyze_wheel_abi
    get_wheel_elfdata(wheel_fn)
  File "/opt/_internal/cpython-3.6.4/lib/python3.6/site-packages/auditwheel/wheel_abi.py", line 42, in get_wheel_elfdata
    so_path_split[-1])
RuntimeError: Invalid binary wheel, found shared library "_pywrapknapsack_solver.so" in purelib folder.
The wheel has to be platlib compliant in order to be repaired by auditwheel.
Makefile:140: recipe for target 'manylinux1-pypi' failed
make: *** [manylinux1-pypi] Error 1

I think, the issue come from setup.py now is shipped with its ".so" cf #551 so it is not a "pure lib" (has never been one BTW).
So setup.py should contains the same line than for the CMake based one cf https://github.com/google/or-tools/blob/master/cmake/python.cmake#L86
from setuptools import dist

class BinaryDistribution(dist.Distribution):
  def is_pure(self):
    return False
  def has_ext_modules(self):
    return True

setup(
...
distclass=BinaryDistribution,

On my way to test a patch

Step to reproduce:
make python
cd temp-python2.7/ortools
python -m pip install wheel
python setup.py bdist_wheel

cd dist
python3.6 -m pip install auditwheel
python3.6 -m auditwheel show *.whl

Trace of the last line:
Traceback (most recent call last):
  File "/usr/lib/python3.6/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/usr/lib/python3.6/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/usr/local/google/home/corentinl/.local/lib/python3.6/site-packages/auditwheel/__main__.py", line 5, in <module>
    sys.exit(main())
  File "/usr/local/google/home/corentinl/.local/lib/python3.6/site-packages/auditwheel/main.py", line 49, in main
    rval = args.func(args, p)
  File "/usr/local/google/home/corentinl/.local/lib/python3.6/site-packages/auditwheel/main_show.py", line 28, in execute
    winfo = analyze_wheel_abi(args.WHEEL_FILE)
  File "/usr/local/google/home/corentinl/.local/lib/python3.6/site-packages/auditwheel/wheel_abi.py", line 73, in analyze_wheel_abi
    get_wheel_elfdata(wheel_fn)
  File "/usr/local/google/home/corentinl/.local/lib/python3.6/site-packages/auditwheel/wheel_abi.py", line 42, in get_wheel_elfdata
    so_path_split[-1])
RuntimeError: Invalid binary wheel, found shared library "libortools.so" in purelib folder.
The wheel has to be platlib compliant in order to be repaired by auditwheel.

note: auditwheel is only available on Python 3

Since merge of https://github.com/pypa/auditwheel/pull/35 auditwheel believe ortools whl package is a "pure lib" (doesn't use the distclass Distribution informations).
So the best we can do, is disable


text:
Github Actions: Cannot get go standard library version with --lockfile=go.mod

Hi all,
I am trying to use google/osv-scanner/actions/scanner@main to scan my project's go.mod file for any vulnerable dependencies within a github action workflow. To do this, I am specifying the go.mod as the --lockfile for osv-scanner.
      - name: "Run scanner on new code"
        uses: google/osv-scanner/actions/scanner@main
        with:
          scan-args: |-
            --format=json
            --output=results.json
            --lockfile=go.mod
        continue-on-error: false

The step exits in Github actions with Exit code: 127:
cannot get go standard library version, go might not be installed: exec: "go": executable file not found in $PATH

The entirety of the step output is as follows:
Run google/osv-scanner/actions/scanner@main
  with:
    scan-args: --format=json
  --output=results.json
  --lockfile=go.mod
/usr/bin/docker run --name d556ed201026cc19174d3ba8f4dc76845af5f3_5489b5 --label d556ed --workdir /github/workspace --rm -e "INPUT_SCAN-ARGS" -e "HOME" -e "GITHUB_JOB" -e "GITHUB_REF" -e "GITHUB_SHA" -e "GITHUB_REPOSITORY" -e "GITHUB_REPOSITORY_OWNER" -e "GITHUB_REPOSITORY_OWNER_ID" -e "GITHUB_RUN_ID" -e "GITHUB_RUN_NUMBER" -e "GITHUB_RETENTION_DAYS" -e "GITHUB_RUN_ATTEMPT" -e "GITHUB_REPOSITORY_ID" -e "GITHUB_ACTOR_ID" -e "GITHUB_ACTOR" -e "GITHUB_TRIGGERING_ACTOR" -e "GITHUB_WORKFLOW" -e "GITHUB_HEAD_REF" -e "GITHUB_BASE_REF" -e "GITHUB_EVENT_NAME" -e "GITHUB_SERVER_URL" -e "GITHUB_API_URL" -e "GITHUB_GRAPHQL_URL" -e "GITHUB_REF_NAME" -e "GITHUB_REF_PROTECTED" -e "GITHUB_REF_TYPE" -e "GITHUB_WORKFLOW_REF" -e "GITHUB_WORKFLOW_SHA" -e "GITHUB_WORKSPACE" -e "GITHUB_ACTION" -e "GITHUB_EVENT_PATH" -e "GITHUB_ACTION_REPOSITORY" -e "GITHUB_ACTION_REF" -e "GITHUB_PATH" -e "GITHUB_ENV" -e "GITHUB_STEP_SUMMARY" -e "GITHUB_STATE" -e "GITHUB_OUTPUT" -e "RUNNER_OS" -e "RUNNER_ARCH" -e "RUNNER_NAME" -e "RUNNER_ENVIRONMENT" -e "RUNNER_TOOL_CACHE" -e "RUNNER_TEMP" -e "RUNNER_WORKSPACE" -e "ACTIONS_RUNTIME_URL" -e "ACTIONS_RUNTIME_TOKEN" -e "ACTIONS_CACHE_URL" -e "ACTIONS_RESULTS_URL" -e GITHUB_ACTIONS=true -e CI=true -v "/var/run/docker.sock":"/var/run/docker.sock" -v "/home/runner/work/_temp/_github_home":"/github/home" -v "/home/runner/work/_temp/_github_workflow":"/github/workflow" -v "/home/runner/work/_temp/_runner_file_commands":"/github/file_commands" -v "/home/runner/work/chat-bot/chat-bot":"/github/workspace" d556ed:201026cc19174d3ba8f4dc76845af5f3  "--format=json
--output=old-results.json
--lockfile=go.mod"
cannot get go standard library version, go might not be installed: exec: "go": executable file not found in $PATH
Scanned /github/workspace/go.mod file and found 3 packages
Exit code: 127

Looking through the code-base for osv-scanner, I can see that when go.mod is provided as the --lockfile, the code will try to enrich the scanner by running go env GOVERSION, which I believe is where the issue lies:
https://github.com/google/osv-scanner/blob/f81949534035a184bce46fd746cbfb6b5ea4d0a1/pkg/osvscanner/optional_enricher.go#L13
I ran this command in a previous step and it does return a version in the format it is looking for:
Run go env GOVERSION
  go env GOVERSION
  shell: /usr/bin/bash -e {0}
go1.19.5

Please let me know if I am missing anything!
Thanks in advance!

Thanks for the interest in the Github Action!
Couple of different things happening here:

The step exits in Github actions with Exit code: 127:

The 127 exit code is from osv-scanner, however the github action should still be returning successfully with a 0 (The action should have a green tick next to it). Your workflow should still continue like normal.

The 127 exit code is indeed from the the go binary not being found, though this should not actually be an error but just informational, since it's very unlikely for the same go binary version used to build the binary to be running in the osv-scanner CI. I will update this in the next release of osv-scanner.

The binary is not found because it is not added in the osv-scanner action Dockerfile (action.Dockerfile), since it doesn't make sense to report vulnerabilities with the osv-scanner's version of Go for your project. (#453)

We also recommend using google/osv-scanner/.github/workflows/osv-scanner-reusable.yml@main (docs here). The osv-scanner action google/osv-scanner/actions/scanner@main is mostly an implementation detail in osv-scanner-reusable.yml at the moment. We know the reusable workflow is not as flexible as a standalone action, and in the future we want to improve it so it can be more easily used by itself.

If you can't use google/osv-scanner/.github/workflows/osv-scanner-reusable.yml@main  for some reason, can you describe the workflow you are trying to achieve?

Thanks @another-rex, super informative. No that makes complete sense. I am just trying to implement a osv-scan as part of pull request checks on a project of mine. I would like the pull request to be blocked from merge if there are any vulnerabilities found (or if that proves to be too harsh, at least report the vulnerabilities in the projects code analysis or in a comment on the PR or something).
I will have a go at utilising google/osv-scanner/.github/workflows/osv-scanner-reusable.yml@main and see how it goes. Thanks alot for the help, I am happy to close this one out for the time being as this will get me on my way.
Thanks again for the help.

Also have a look at google/osv-scanner/.github/workflows/osv-scanner-reusable-pr.yml@main (docs here: https://google.github.io/osv-scanner/github-action/#scans-on-prs), which is designed specifically for checking PRs for new vulnerabilities (it compares the target branch and the PR branch to only alert on new vulns).

authors: another-rex toddtee

repo: google/osv-scanner


text:
Refactoring do.sh for building extension from source

Remove duplicate input called from compile.flags.
Fix SRC_DIRS to point to lib/closure-library/closure/goog from lib/closure-library

fixes #93

I signed it!

I signed it!

Currently extension builds and runs, but throws Unchecked runtime.lastError while running tabs.executeScript: The extensions gallery cannot be scripted..  There is a call to executeScript in background.js.

Thanks for the fix!

authors: adhintz eewayhsu

repo: google/password-alert


text:
cant trace event

Hi, I traced with above commands, but only get 631 bytes, The trace result is empty in perfetto UI, anything wrong?

Something failed somewhere. Can you attach the logcat output?
Maybe the tracing service is not running at all?
Please see https://perfetto.dev/docs/quickstart/android-tracing for instructions.
P.S. this bug is too vague: you are not saying which version of Android you are using, which device, not attaching any logs.

Closing because the bug is too vague and no follow up happened.

authors: dengxbin primiano

repo: google/perfetto


text:
Android - non-ASCII URl issue

Added the following URl  https://莎士比亚.org/

When displayed in the app, it is corrupt.

Clicking on the link goes to to the wrong location.

If I use the punycode representation xn--jlQ54W7yPemW.org everything works correctly.

Although the Title and Favicon don't load.

Well per the Eddystone Spec:
URLs are written only with the graphic printable characters of the US-ASCII coded character set. The octets 00-20 and 7F-FF hexadecimal are not used. See “Excluded US-ASCII Characters” in RFC 2936.
So I believe this is Working as intended. If you want to use non ascii characters you need to open an issue on the Eddystone repo.

Or, you can use a URL shortener.
Maybe what we should be doing here is automatically detecting these cases and sending it off to a shortener.

I think punnycode / IDN encoding (https://en.wikipedia.org/wiki/Internationalized_domain_name) is the correct way to have a internationalized URL to be encoded into US-ASCII characters.
I think what the app fails to do here is:
1/ to encode the internationalized URL to US-ASCII when programming the beacon.
2/ to decode the encoded URL / domain name to a more readable string
3/ I'm not sure why the punnycode encoded URL won't load the metadata.
1/ and 2/ are features that needs to be implemented.
3/ probably needs more investigation.

The URL seems to be resolved properly by the PWS:
$ curl -d '{"objects":[{"url":"https://xn--jlq54w7ypemw.org/"}]}' https://url-caster.appspot.com/resolve-scan
{"metadata": [{"description": "Terence Eden Terence Eden \u6cf0\u7279\u65af\u00b7\u5b89\u5fb7\u6d1b\u5c3c\u514b\u65af Titus Andronicus \u79d1\u5229\u5965\u5170\u7eb3\u65af Coriolanus \u4ea8\u5229\u56db\u4e16\u4e0a\u7bc7 Henry IV - Part I \u88d8\u529b\u65af\u00b7\u51ef\u6492 Julius Caesar \u7686\u5927\u6b22\u559c As You Like It \u7ec8\u6210\u7737\u5c5e All's Well That Ends Well \u5965\u745f\u7f57 Othello \u7ea6\u7ff0\u738b King John \u96c5\u5178\u7684\u6cf0\u95e8 Timon of Athens \u4ef2\u590f\u591c\u4e4b\u68a6 A Midsummer Night's Dream \u4ea8\u5229\u516d\u4e16\u4e2d\u7bc7 Henry VI - Novella \u8f9b\u767d\u6797 Cymbeline \u7f57\u5bc6\u6b27\u4e0e\u6731\u4e3d\u53f6 Romeo and Juliet \u4ea8\u5229\u516d\u4e16\u4e0b\u7bc7 Henry VI - Part II \u51ac\u5929\u7684\u6545\u4e8b A Winter's Tale \u4ea8\u5229\u4e94\u4e16 Henry V \u9ea6\u514b\u767d Macbeth \u7406\u67e5\u4e09\u4e16 Richard III \u7406\u67e5\u4e8c\u4e16 Richard II \u4ea8\u5229\u56db\u4e16\u4e0b\u7bc7 Henry IV - Part II \u66b4\u98ce\u96e8 The Tempest \u4ea8\u5229\u516b\u4e16 Henry VIII \u6e29\u838e\u7684\u98ce\u6d41\u5a18\u513f\u4eec The Merry Wiv", "title": "\u838e\u58eb\u6bd4\u4e9a.org / Readable Shakespeare Plays In Chinese", "url": "https://xn--jlq54w7ypemw.org/", "rank": 1000, "displayUrl": "https://xn--jlq54w7ypemw.org/", "id": "https://xn--jlq54w7ypemw.org/", "icon": "https://url-caster.appspot.com/favicon?url=https%3A%2F%2Fxn--jlq54w7ypemw.org%2Fapple-touch-icon-iphone.png"}]}

@dinhviethoa yes, I'd agree with that. The app should be normalising & decoding.
@cco3 automatic shortening would also be helpful - it currently just throws an error message.

IMO there are three changes to make.
First, we should detect non-ASCII URLs and automatically send those to the shortener as Conley suggests.  We could try to puny-encode them ourselves but chances are the URL will be too long anyway.
Second, PWS should detect punycode URLs and send the punycode version as siteUrl (PWS calls this field "url") and the Unicode version as displayUrl.  Currently, the punycode version is sent for both fields.
Third, the error message when the auto-shortener chokes on a unicode URL needs to be updated.  It says "URL is too long. Please use a shortener" which is confusing if the URL is clearly short enough.  Fortunately we should never see this since goo.gl is excellent with international URLs.

I like the plan, but I'm thinking that PWS should not return Unicode is displayUrl.
Choosing to display the unicode version sounds like a client decision, and I'm not sure the PWS should be making client UX choices.
It's also not always going to be clear to PWS when a URL is punycode (or to the client for that matter), so perhaps we can just leave the URL in punycode?

I'd like to politely challenge that notion.
No one is going to want to see the punycode version.  It's impossible for anyone to decipher what it is supposed to represent.  I would suggest that displaying the Unicode URl makes the most sense - it will display in the correct character set for the user.
A URl is punycode if it starts with xn-- http://www.atm.tut.fi/list-archive/ietf-announce/msg13572.html
The standard is relatively simple http://tools.ietf.org/html/rfc3492
Overall, adding this will aid adoption for people in non-ASCII territories.

I tried the unicodes URL both in Chrome and Safari on the desktop.

Safari will keep it unicode
Chrome will convert it to punycode

I'd say the experience was more enjoyable to me in Safari.

I don't propose we enforce showing punycode, I'm just suggesting that the client can detect and convert if it so chooses, without the PWS making the choice for it.  By returning unicode in displayUrl, we would be assuming all clients are able to display/use it.
It think the intention of displayUrl was originally meant to represent "the url which will open in browser", as opposed to "the url which we actually fire intent on".  It was used to hide our use of a redirector (which we have long since dropped) -- so displayUrl is currently "unused".
So displayUrl was never meant to actually make UX level decisions about what clients should display, and I wonder if we shouldn't remove the value altogether.
Also, I think clients will want to actually process displayUrl in all sorts of ways.  E.g. if displayUrl is http://www.foobarbaz.com/path/page.html#somevariable clients may choose to just show www.foobarbaz.com or just http://www.foobarbaz.com/path/page.html#... etc.
So I guess I agree our clients shouldn't show punycode, but I think we should not update PWS displayUrl as was previously suggested.
Thanks for pointing out that punycode is unambiguous, I only read the encoding format (suffix), and not the spec for prefix.

authors: cco3 dinhviethoa edent g-ortuno mmocny nondebug

repo: google/physical-web


text:
PW not workin on one android device

Hi all,
I've a problem i can't solve by myself so any help is precious.
First device is a Samsung Galaxy Tab 2014 edition with Android 4.4.2 and beacon detection is great with ChromeDev, PW app, and every other apps.
Second Device is a Samsung Grand Prime with Android 5.1.1 (build 2016-03-01) and beacon location with some app is poor and slow. With Chrome Dev no detection at all (Smartphone and tablet are put side by side).
I'm writing an app with logs and beacon detection is really slow on Smartphone while, same app, is absolutely fast on Tablet.
Obviously the Chrome Dev release is the same: 51.0.2702.22
Maybe something related to Android 5 or a hardware issue?
Thnks for any hint
Mario

Close by myself.
Problem is Lollipop


text:
[PWS] Fixed webui form validation

My previous commit didn't handle correctly webui form validation.
This patch does.

I didn't even know you could do this, cool!

authors: beaufortfrancois mmocny