Distribution Mode save/restore

I'm running the distribution mode in one machine the same as example shows. Can I specify the same saver directory in agent's spec? Will it cause any conflict between agents?

I try to save the agent-0's model and try to load it at single mode, but I got following error when trying to restore from saved model:
NotFoundError (see above for traceback): Key ppo/observe-timestep/optimization/reward-estimation/mlp-baseline/predict/prediction/apply/b not found in checkpoint
	 [[Node: save/RestoreV2_75 = RestoreV2[dtypes=[DT_FLOAT], _device="/job:localhost/replica:0/task:0/device:CPU:0"](_arg_save/Const_0_0, save/RestoreV2_75/tensor_names, save/RestoreV2_75/shape_and_slices)]]

Sorry for the late reply, just getting started with support again. I'll try to look into that soon.

So you train a model in async mode, but then want to load it in single mode? Can you also post the config?

I modify episode_finished(r) and add following:
    if r.episode % 10 == 0 and r.agent.device == "/job:worker/task:0":
        log.info("Saving model...")
        r.agent.save_model("./models/models")

But it seems that I can not load this model from single mode, and in distributed mode, after I load this model, it seems that the network parameters didn't changed.
    nva = agent.model.session.run(agent.model.network.get_variables())
    if os.path.exists(pp+"models/"):
        log.info("Loading models...")
        agent.restore_model(pp+"models/")
        nvb = agent.model.session.run(agent.model.network.get_variables())
        assert not all([np.array_equal(v1, v2) for v1, v2 in zip(nva, nvb)])

I got an assert error here.

I mean the config/arguments used to initialize the agent, that would be helpful. I will then try to reproduce the problem...

Basically I use this command:
python examples/openai_gym_async2.py CartPole-v0 -a examples/configs/ppo.json -n examples/configs/mlp2_network.json -e 10000 -m 200 -W 3

@lynic, finally got around looking into this. First, I wouldn't recommend using the manual save/restore_model API, but instead use the saver_spec argument (fyi: it saves the variables of the global model, not one of the workers). Second, I fixed a problem which caused an error when loading an async trained model in single mode, but this should work now. Let me know if there are still problems...


Initial GraphQL and Seeds

Description:

Set up back end server for GraphQL
Add seeders to populate data for react structuring
Prepopulate users with points(for development)
Implement seed.js functionality

Wrote seeds for category, cause, and user.  Unable to test bc of server bugs.  Pushed code to feature/backend-setup for time being, will not merge to develop until bugs are resolved.


Researchers and Organizations cannot use the Export function.

Describe the bug
Researchers and Organizations cannot use the Export function.
Version：cris-2023.02.03 ; DSpace 7.6.1
web browser：chrome ; Firefox
To Reproduce

User log in.
Go to Researchers (img1) or Organizations (img2), and use the Export function.
Click on any export type in Export, an error message will appear: "There are no items to export with the selected configuration". (img3)
Please refer to the attached images.

Expected behavior
Data should be exported successfully.
[img1]

[img2]

[img3]

This issue has been fixed


add scale props

Hi, I wanted to use this animation in my app but the problem is there is lots of padding in the original animation.
I tried to use autoSize and resizeMode='cover', but no success,
my code is:
const filter = require('../../assets/Filter2.json')
...
<LottieView
                source={filter}
                style={{height:'100%'}}
                speed={2.5}
                ref={(ref)=> this.filterAnimation = ref}
                resizeMode='cover'
                autoSize
                loop={false}
              />

if there was a scale prop, I could use that.
I guess it is available in lottie-android as in this SO answer

We might introduce this feature after updating to the latest lottie-android

@emilioicai
Is there any improvement?

@emilioicai Why did you close this issue? Is there a fix?

I closed it because it seemed to me it was quite stale. I will reopen if it's still an issue with the latest version

I still could not use lottie because of this issue. Can you please tag me when you open a new one?

@WrathChaos I just reopened this one

Need this too 👍

+1

+1

any updates?

Is this feature added? I am facing same issue with animation I am using


Use newer Github header for the sha2 hash

A simple update to use newer Github header X-Hub-Signature-256 to use the sha2 hash. The security tests are already using the sha256 algorithm.
This is the Github documentation for reference
https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/webhook-events-and-payloads#webhook-payload-object-common-properties

@ptejada Thanks :)


fix(build) Update dependencies and restore the build

@angelaigreja I would decline this one in favor of #32 that was merged in the dev branch, so please updates your active topic branches and take in mind that you need npm3


[SPARK-18399][SQL][DOCUMENTATION] Examples in SparkSQL/DataFrame/DataSets guide fails with default configuration

What changes were proposed in this pull request?
With default configuration, examples in Spark SQL, DataFrames and Datasets Guide result in a failure since they try to access HDFS while the paths of people.json, people.txt are assumed to be in local file system.
How was this patch tested?
I test this with manual test: running examples of SparkSQL/DataFrame/DataSets with default configuration. With this fix, it works well without configuring HDFS.

Can one of the admins verify this patch?

I found that my previous PR has a bug(forgot to add "/" before example). Resending PR with the fix.

Before the patch:
scala> val df = spark.read.json("examples/src/main/resources/people.json")
16/11/10 21:02:59 WARN datasources.DataSource: Error while looking for metadata directory.
java.net.ConnectException: Call From ozamac-2.local/<IP_ADDRESS> to localhost:9000 failed on connection exception: java.net.ConnectException: Connection refused; For more details see:  http://wiki.apache.org/hadoop/ConnectionRefused

After the patch:
scala> val df = spark.read.json("file:///" + System.getProperty("user.dir") + "/examples/src/main/resources/people.json")
df: org.apache.spark.sql.DataFrame = [age: bigint, name: string]

We want the examples to be consistent, so wouldn't this affect all examples?
And yes they're supposed to be run with the given script.
Putting the path in the source code might help a bit for people reading it as source code, which is how these examples are really consumed, rather than referring to what the command line args are supposed to be. If it works with spark-example and so do other examples, I think we should leave them.

As far as I know, the default file system should be file:///.
I created a virtual machine with the OS Ubuntu Server Trusty 14.04 amd64, from the link https://oss-binaries.phusionpassenger.com/vagrant/boxes/latest/ubuntu-14.04-amd64-vbox.box
And then, I ran the commands as below:
sudo apt-get install openjdk-7-jdk -y
wget http://d3kbcqa49mib13.cloudfront.net/spark-2.0.1-bin-hadoop2.7.tgz
tar xzvf spark-2.0.1-bin-hadoop2.7.tgz spark-2.0.1-bin-hadoop2.7
cd spark-2.0.1-bin-hadoop2.7/

and I ran
./bin/run-example sql.SparkSQLExample

this seems working fine. Could you please try this?

Thanks a lot for your comments! I see, in fact, I had the problem on Mac OS. Okay, I will try it on your VM, and on my local linux. Please wait a moment.

@srowen it makes sense to me. In that case, we can update our documentation to avoid the problem.

I'm sorry, it's completely

No problem. Thanks for the effort!

+1 for no problem :).


No IP address for "private" IPs

It would be helpful if the documentation mentioned that there's no client IP when the client IP isn't public/routeable due to how django-ipware behaves. I was expecting to see the correct IP address in the logs emitted by django-axes.

@pembo13 would you like to add a PR describing the actual vs. expected behaviour as you would prefer it?

@aleksihakli under what section would this make sense to add it?
Expected behaviour is that internal addresses such as "<IP_ADDRESS>" would show up in the logs as part of the client str, as opposed to None

Whicj


Pin bootstrap-sass

Gem bootstrap-sass 3.3.6 is not compatible with this gem.

Why is 3.3.6 incompatible? I'd prefer to stay on the latest version if possible.

It looks to be https://github.com/twbs/bootstrap-sass/compare/v3.3.5...v3.3.6#diff-db4fa8404e2c8c4f78edc5fcf857dd4dR11

I've left a comment here and will follow up:
https://github.com/twbs/bootstrap-sass/pull/965#issuecomment-159684783

This is the second time a point release has led to a breaking change. It might be wise to leave this pinned even when the issue is fixed. See also https://github.com/twbs/bootstrap-sass/pull/922


Skip checks when we don't find the appropriate version of Exchange

Skip checks that don't apply to 2010.

New output when running remotely against 2010:

Oh, and these changes are also related to #108


Config File

Hi, I tried to run this but first of all, I couldn't find any config file or config section so I figured, I'm missing something here. I'm totally not sure, can you help me out?

@anazhd - Hey there, so the reason I didn't commit my configuration file is that everybody will have their own different local settings. I'll commit the file shortly.

@anazhd - I've committed it. However, I still haven't put together a rules engine or actually tested if the strategy works with live data. It is still a work in progress. Now for, I'm able to cache 24 hours of trade history from bittrex into redis. I'm also able to calculate the moving averages live. If you wish to contribute to this project - I'm looking for help on the best trading strategy to use with cryptocurrencies. The simple moving average formula is a very basic strategy.


Needs support for alias

example
        alias material_index = u32;
        alias color = vec3f;

        struct material {
            index: material_type,
            diffuse: color,
        }

        @group(0) @binding(1) var<storage> materials: array<material>;

reflect returns nothing for this
WgslReflect {structs: Array(0), uniforms: Array(0), storage: Array(0), textures: Array(0), samplers: Array(0), …}
aliases: []
ast: []
entry: EntryFunctions
  compute: []
  fragment: []
  vertex: []
samplers: []
storage: []
structs: []
textures: []
uniforms: []

I guess I missed the memo, but did the keyword type  get renamed to alias, or are those separate things. It looks like type got moved to the reserved list, so I'm guessing it was renamed.
I pushed a change to use alias instead of type. In your example, aliases now has two entries, which are AST Alias objects.

I think this one is completed. If I missed some details, feel free to open a new issue.


GrantView Attributes

Adding Grant Synapse Project and Grant Synapse Team

@aditigopalan Everything looks great! I made two very small changes: added the two new attributes to the list of features included in the GrantView manifest and made them non-required, since we will most likely ingress grant info prior to creating the Synapse project and team. As long as you agree with those changes, I think this PR is ready to merge now!

Thanks @Bankso! Looks good to me


[Bug]: Node Version

What happened?
After updating to the new version from beta (due to numbering issue) I reverted back to Beta to get HSV back and now the plugin states that Node version 18.16 is required. Which did not show up in logs prior to the upgrade and downgrade.
Device Type
Station (Supported)
Plugin Version
beta (Best Effort)
HomeBridge Version
v1.6.x (Supported)
NodeJS Version
v18 (Supported)
Operating System
Other (UnSupported)
Relevant log output
The plugin "homebridge-eufy-security" requires Node.js version of >=18.16.0 which does not satisfy the current Node.js version of v18.15.0. You may need to upgrade your installation of Node.js

config
N/A

Please try upgrading to 2.2.3 then downgrade to beta.

After updating to v2.2.3 the warning about node when away, but once I then downgraded to beta it came back, and the settings in beta do not include any HSV settings.
[8/23/2023, 11:24:37 AM] The plugin "homebridge-eufy-security" requires Node.js version of >=18.16.0 which does not satisfy the current Node.js version of v18.15.0. You may need to upgrade your installation of Node.js - see https://homebridge.io/w/JTKEF [8/23/2023, 11:24:37 AM] Loaded plugin<EMAIL_ADDRESS>[8/23/2023, 11:24:37 AM] Registering platform 'homebridge-eufy-security.EufySecurity' [8/23/2023, 11:24:37 AM] ---

try it now it should be fine. 2.3.0-beta0 nothing has change.

Awesome, worked perfect! thanks again.

thanks, I'm closing this thread.


Added cctl

Added cctl as a submodule to custard.
Added the installation of cctl to the main make install target.

Good work. I've decided that I'll leave this pull request open, though, until cctl is feature complete. Once that happens it will likely replace custardctl as the main user interface with custard.

What should I continue to add? Should I just make the syntax for it custardctl compliant?

Good to know, I'll start working on proper parsing once I get my own setup decently configured. And currently cctl can execute anything that will be sent to the socket, because it just directly sends inputs to the socket with zero parsing (I know, I know).

@0xfi Any updates on this?


Issue implementing the javascript version in behavior tutorial

``First of all, thank you for this guide! These tutorials have proven to be a great starting point and I've learned a lot about alfresco, java, maven and a bit of javascript.
Currently, I'm doing the behaviors tutorial. I have no problem with the java part, but when testing the JavaScript method, I get thiserror after running ./run.sh build_start_it_supported.
behavior-tutorial-ass_1       | org.alfresco.error.AlfrescoRuntimeException: 07170097 api/solr/aclchangesets return status:404 behavior-tutorial-ass_1       | 	at org.alfresco.solr.client.SOLRAPIClient.getAclChangeSets(SOLRAPIClient.java:169) behavior-tutorial-ass_1       | 	at org.alfresco.solr.tracker.AclTracker.checkRepoAndIndexConsistency(AclTracker.java:338) behavior-tutorial-ass_1       | 	at org.alfresco.solr.tracker.AclTracker.trackRepository(AclTracker.java:303) behavior-tutorial-ass_1       | 	at org.alfresco.solr.tracker.AclTracker.doTrack(AclTracker.java:95) behavior-tutorial-ass_1       | 	at org.alfresco.solr.tracker.AbstractTracker.track(AbstractTracker.java:215) behavior-tutorial-ass_1       | 	at org.alfresco.solr.tracker.TrackerJob.execute(TrackerJob.java:47) behavior-tutorial-ass_1       | 	at org.quartz.core.JobRunShell.run(JobRunShell.java:216) behavior-tutorial-ass_1       | 	at org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:563) behavior-tutorial-ass_1       | 2021-08-17 06:52:10.053 ERROR (org.alfresco.solr.AlfrescoCoreAdminHandler@59d4cd39_Worker-35) [   ] o.a.s.t.AbstractTracker Tracking failed for ModelTracker - alfresco
I hope you can help me out.
Best regards,
Mr De Bonis

Thanks for working through the tutorials, I am glad you find them helpful.
The error message you included is from the search server. It is unable to
contact the Alfresco Content Services server. This usually means that
alfresco did not start up properly. There is likely one or more exceptions
earlier in the log that will help you figure out the underlying problem
that caused alfresco not to start.
Also, JavaScript backed behaviors should work, but you should know they are
not used very often. It is far more common to see Java backed behaviors.
On Tue, Aug 17, 2021 at 2:02 AM Mr-DeBonis @.***> wrote:

``First of all, thank you for this guide! These tutorials have proven to
be a great starting point and I've learned a lot about alfresco, java,
maven and a bit of javascript.
Currently, I'm doing the behaviors tutorial. I have no problem with the
java part, but when testing the JavaScript method, I get thiserror after
running ./run.sh build_start_it_supported.
behavior-tutorial-ass_1 | org.alfresco.error.AlfrescoRuntimeException:
07170097 api/solr/aclchangesets return status:404 behavior-tutorial-ass_1 |
at
org.alfresco.solr.client.SOLRAPIClient.getAclChangeSets(SOLRAPIClient.java:169)
behavior-tutorial-ass_1 | at
org.alfresco.solr.tracker.AclTracker.checkRepoAndIndexConsistency(AclTracker.java:338)
behavior-tutorial-ass_1 | at
org.alfresco.solr.tracker.AclTracker.trackRepository(AclTracker.java:303)
behavior-tutorial-ass_1 | at
org.alfresco.solr.tracker.AclTracker.doTrack(AclTracker.java:95)
behavior-tutorial-ass_1 | at
org.alfresco.solr.tracker.AbstractTracker.track(AbstractTracker.java:215)
behavior-tutorial-ass_1 | at
org.alfresco.solr.tracker.TrackerJob.execute(TrackerJob.java:47)
behavior-tutorial-ass_1 | at
org.quartz.core.JobRunShell.run(JobRunShell.java:216)
behavior-tutorial-ass_1 | at
org.quartz.simpl.SimpleThreadPool$WorkerThread.run(SimpleThreadPool.java:563)
behavior-tutorial-ass_1 | 2021-08-17 06:52:10.053 ERROR
@.***_Worker-35) [ ]
o.a.s.t.AbstractTracker Tracking failed for ModelTracker - alfresco
I hope you can help me out.
Best regards,
Mr De Bonis
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/jpotts/alfresco-developer-series/issues/45, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAOP5WYVUZ2A3HF6NMD72BLT5ICRXANCNFSM5CJGQVRA
.
Triage notifications on the go with GitHub Mobile for iOS
https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675
or Android
https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email
.


Create filter for pull request tests on Jenkins

Creates a script for run_tests_matrix.py to call in order to filter out unnecessary tests based on examining file differences between the branch being tested and gRPC:master.
Currently this script only filters out c++, asan, tsan, and msan tests, and filters by checking the beginning and end of changed files, including its directory, for triggers that indicate whether a pull request can skip certain tests.
This still needs work in adding filters for other tests and feedback on how to prevent incorrectly skipping tests.

I think that should address most of your concerns.

nit: compared to the base branch the PR is getting merged into (not necessarily grpc:master)

Updated along with any other instance I mentioned grpc:master

so if none of the diff files are in one of these subdirs, the script is going to skip c++ tests? That's actually not good enough - if someone decides to change Makefile, that can totally break C++, but this script doesn't capture that (and it's going to be hard to capture this in general). Perhaps we need a different approach.

I changed it to like how you described it in your brainstorm. Right now, unless a changed file matches a whitelist condition, all tests will be run - more whitelist conditions will need to be added and the current ones verified.
I am working on making the whitelist conditions around sanitizer tests to be as fine-grained as possible. Currently, if a change is made only in test/core, only core tests are run. If a change is made in only src/cpp or test/cpp, then all c++ tests AND core sanitizer tests are run. If a src/core change is made, all tests are run. I am not if this is correct at all, so please correct me if I'm wrong. I'd also like to change it so core sanitizer tests are skipped if only test/cpp files are changed.

dangerous to hardcode origin/master (people might as well merge into v1.0.x or any other branch in which case this gives them wrong results).

This is changed. When we configure Jenkins jobs, the arguments --filter_pr_tests --base_branch $ghprbTargetBranch should be added when calling run_tests_matrix.py.

string.split() splits on spaces as well and you really should split on newlines only. Also, please check that the splitting works on windows where you might see \r\n line endings (maybe?).

Good catch - I changed it to str.splitlines(), which should work with Windows, but of course I'll test it using Jenkins to make sure.

I don't quite see what you need the "ends_with" triggers for. If you dont' need it now, don't include it and only add it when needed.

I added a use for this; I think it'll be useful to filter out documentation files such as README.md.

I'm a bit afraid to skip tests based on a shortname of a test (the shortname is basically a freeform string). Maybe we could come up something more bulletproof.
On the other hand, perhaps fine for initial experiments (before actually enabling this) if a TODO is added.

TODO added - will address in the future.

CC @nicolasnoble
Feedback would be greatly appreciated.
I was thinking about make whitelisting more intuitive by making it so if I wanted to whitelist src/python from non-Python languages, I would add it to the C++, Node, Ruby, Core, PHP, C#, and Objective-C whitelists but this would lead to the file being very long since whitelisting anything for all but one language will require it being added to the file 7 times. Thoughts?

I went with all of your suggestions in this commit. Just a few things:

Changing something like 'src/node/README.md' now triggers Node tests, and this is something I'd like to change without adding too much complexity.
All of the sanitizer tests will now run or skip; I will change this to be able to specify C vs C++ after reaffirming which situations allow one to be skipped.
I will redesign the TestSuite class after deciding how to better identify when a job belongs to a test suite - finding tags in job.shortname is just temporary.

Currently this is not working when using Jenkins - I tested it on Windows and Linux. I'll commit changes tomorrow to fix this.


add a repository rule for download caching

This repository rule allows to transparently declare external
dependencies in the scratch workspace, as one would do in the regular
workspace, but makes it so that no download actually occurs in the
scratch environment, allowing for better performance through caching
and better isolation (block-network can be used as a test tag even with
those external dependencies).  This repository rule is a no-op for bazel < 0.12.0.
See #63

Before I go into the details, what do you think concerning the overall approach?

@hchauvin I need you to press the "update branch" button here so that it will update the PR from master. I'll re-trigger the build and when it's green I'll merge.

For the approach I am fine with it but I would rather use the cache directly. The cache is actually documented but the documentation was never imported. TBH I don't understand why they added distcache flag instead of simply documenting the cache format / making it nicer.

So you think that, contrary to what is said behind the motivation of
distdir, the interface to the cache is actually likely to be stable? The
reason I used distdir is that the cache format is clearly marked as
internal details at least on the github master.
Le jeu. 26 avr. 2018 à 10:52, Damien Martin-Guillerez <
<EMAIL_ADDRESS>a écrit :

For the approach I am fine with it but I would rather use the cache
directly. The cache is actually documented but the documentation was
never imported https://bazel-review.googlesource.com/c/bazel/+/6933.
TBH I don't understand why they added distcache flag instead of simply
documenting the cache format / making it nicer.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/bazel-integration-testing/pull/71#issuecomment-384564594,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ANLKq3xHQl8RyRwex8mYvgeQndbUh-Lfks5tsYrmgaJpZM4TjJKu
.

I cannot say about the future, I am no longer in the Bazel team, but the
cache structure hasn't changed since its creation. Which mean at the moment
it would work will all supported version of Bazel.
On Thu, Apr 26, 2018, 11:07 AM Hadrien Chauvin<EMAIL_ADDRESS>wrote:

So you think that, contrary to what is said behind the motivation of
distdir, the interface to the cache is actually likely to be stable? The
reason I used distdir is that the cache format is clearly marked as
internal details at least on the github master.
Le jeu. 26 avr. 2018 à 10:52, Damien Martin-Guillerez <
<EMAIL_ADDRESS>a écrit :

For the approach I am fine with it but I would rather use the cache
directly. The cache is actually documented but the documentation was
never imported https://bazel-review.googlesource.com/c/bazel/+/6933.
TBH I don't understand why they added distcache flag instead of simply
documenting the cache format / making it nicer.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<
https://github.com/bazelbuild/bazel-integration-testing/pull/71#issuecomment-384564594
,
or mute the thread
<
https://github.com/notifications/unsubscribe-auth/ANLKq3xHQl8RyRwex8mYvgeQndbUh-Lfks5tsYrmgaJpZM4TjJKu
.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/bazel-integration-testing/pull/71#issuecomment-384568980,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADjHfySnxEMAt1lHzTYphhJs8QR8zaNzks5tsY5UgaJpZM4TjJKu
.

Awesome change!

I'm sorry, too many tabs... I meant the linting one.
Will respond soon but in general I'm very excited about the approach

@damienmg I don't think we can use the cache dir directly inside the scratch workspace because of sandboxing. @hchauvin's approach of using distdir circumvents that because he places the distdir inside the sandbox using runfiles.
Am I mistaken?

@ittaiz I can actually use the cache instead of using the distdir, the only reason I used the distdir was because the cache was clearly marked as an implementation detail.  I just rebased and reformatted everything with the linter.

@hchauvin thanks for the rebase! it makes reviewing much easier.
How can you use the cache itself? I think it won't work with remote execution since only declared inputs are submitted to the remote

So I looked at what the repository cache actually looks like, @damienmg, and it's all in one class, and the architecture is really simple and I cannot see how it can change.  The reason I think experimental_distdir has been introduced is that repository_cache requires you to put each file to cache in a path that includes their digest, whereas experimental_distdir does not, and for some people it could add complexity to their builds.  But I don't see either why the repository_cache directory layout is not public API.
Therefore, as per your suggestion, I nuked experimental_distdir and replaced it with experimental_repository_cache.  Now, the trick works for all the Bazel versions supported by bazel-integration-testing.
@ittaiz It works the same way my version with distdir works.  All the files to download are actually declared as external_deps, then I rebuild the cache myself before invoking Bazel.
I had to nuke as well the test with "ext.zip" because it was not so easy to do with the repository_cache interface.  Instead, I make sure I "freeze" the repository cache by making it read-only, so that I'm sure Bazel won't try to download additional deps, and I have a test for that.  The reason I added this mechanism is that I found out that, somehow, Bazel has the ability to use the network anyway, even in sandboxing with block-network (I didn't investigate further).
Thank you both for the support on this patch!

@hchauvin at the creation of the repository cache there was that idea of having it as an API but we weren't sure about the use case, so it never got far away. Seems like the Bazel team has gone another way now...
Seems like the freezing doesn't work on linux in Buildkite: java.nio.file.FileSystemException: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/16fed208024f2a5f406c7a5382cf5f8b/bazel-sandbox/3936978874677246825/execroot/build_bazel_integration_testing/_tmp/cc02c3dac8b34566a4624971a114d7f6/cache/content_addressable/sha256/90a8e1603eeca48e7e879f3afbc9560715322985f39a274f6f6070b43f9d06fe/file: Read-only file system
Just an idea (maybe add TODO somewhere in the PR): it would be cool to be able to specify not only downloaded artifact but any artifact. E.g. I want //bin/my/special:artifact in the cache and just compute the checksum in the helper (so we can also use build artifact in the helper). I think of integration test where you have some constructed repository, constructed by bazel.

Damien,
Why do you think the team went a different way?
Re your generalization- how do you see the scratch workspace utilizing the
“special:artifact”? The current example works because inside
java_import_external uses the downloaded who uses the cache
On Fri, 27 Apr 2018 at 11:29 Damien Martin-Guillerez <
<EMAIL_ADDRESS>wrote:

@hchauvin https://github.com/hchauvin at the creation of the repository
cache there was that idea of having it as an API but we weren't sure about
the use case, so it never got far away. Seems like the Bazel team has gone
another way now...
Seems like the freezing doesn't work on linux in Buildkite:
java.nio.file.FileSystemException:
/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/16fed208024f2a5f406c7a5382cf5f8b/bazel-sandbox/3936978874677246825/execroot/build_bazel_integration_testing/_tmp/cc02c3dac8b34566a4624971a114d7f6/cache/content_addressable/sha256/90a8e1603eeca48e7e879f3afbc9560715322985f39a274f6f6070b43f9d06fe/file:
Read-only file system
Just an idea (maybe add TODO somewhere in the PR): it would be cool to be
able to specify not only downloaded artifact but any artifact. E.g. I want
//bin/my/special:artifact in the cache and just compute the checksum in the
helper (so we can also use build artifact in the helper). I think of
integration test where you have some constructed repository, constructed by
bazel.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/bazelbuild/bazel-integration-testing/pull/71#issuecomment-384902929,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABUIF5T725zyFEdwF1BFZIDxIE5NETtJks5tstbvgaJpZM4TjJKu
.

Ittai: I cannot comment, I am no longer in the team.

@ittaiz I don't follow you here.  Do you have an example in mind where it should not work?  As far as I know, all the repository rules for including external dependencies have to use some sort of rctx.download mechanism, and if they don't they should not hit the cache.
To be more precise, the class implementing the repository cache in Bazel is com.google.devtools.build.lib.bazel.repository.cache.RepositoryCache.  It is called only in two places: in com.google.devtools.build.lib.bazel.repository.MavenDownloader, where an sha-1 digest is used, and in com.google.devtools.build.lib.bazel.repository.downloader.HttpDownloader, with an sha256 digest.
The HttpDownloader case is I think handed properly, and the MavenDownloader cache is not supported (but only because sha-1 is used), but I think that maven_jar should be deprecated anyway.
If you really want to support maven_jar, adding support for sha-1 as well is easy, but I am not in favor of that.

@damienmg- I meant what makes you say that it “seems the team went another way” and not “why do you think the team decided to go another way”? More importantly do you think the current implementation by @hchauvin will be a problem?
@hchauvin two examples are:

Git repository- doesn’t use rctx.download
Damien’s example of an artifact of the repo
I’m much more interested in solving the first use-case though it’s not blocking for this PR

With respect to freezing I suggest to try and push a commit where you don’t call “freeze” and see if the freeze test fails of passes. If it passes there (meaning it’s frozen by Bazel itself) then maybe you can just check in Freeze if the Filesystem is read only or not and NOP if it is read only already

From https://docs.oracle.com/javase/7/docs/api/java/nio/file/FileSystem.html:
A FileSystem can provide read-only or read-write access to the file system. Whether or not a file system provides read-only access is established when the FileSystem is created and can be tested by invoking its isReadOnly method. Attempts to write to file stores by means of an object associated with a read-only file system throws ReadOnlyFileSystemException.

I meant this new flag is experimental and doesn't tell us what will be its future. The other flag is no longer experimental and the format of the cache hasn't changed since its creation. One idea of the original design was actually to be able to zip the cache dir and bring it to machine without network access to use it.
Anyway the current state is using the cache structure works for all version of Bazel, using the distdir works only for latest release.

ok, did some investigation over at #73
if I remove the call to freeze the "freeze" test fails because the error is about not finding "repo1.maven.org" and not because of permission denied.
I removed the block-network tag to see if the test will pass that point and the bazel run will fail on downloading the jar since the filesystem is read-only but bazel succeeds.
I'm now trying to check if I remove the block-network but keep the freeze call how the test behaves.

when calling freeze without block-network we still fail on RO filesystem.
@hchauvin any chance you can run the build of your branch on linux to see if this is bazel sandbox in general or something buildkite related?
I'm not sure why the filesystem is readonly with respect to permissions but readwrite when trying to add files

@ittaiz Thank you for the work!  I've reach out for a Linux box with a configuration I use for buildkite agents, and the problem is due to the fact that sandboxing works differently on Linux than on Mac OSX (and is more robust on Linux): if you test with --spawn_strategy=standalone, the test passes just fine.  So that's sandboxing-related.
On Linux the block-network directive actually works, so what do you think about simply adding some comments to the freeze() method and silencing the IOException? BTW I now see the behavior of the sandbox on Mac OSX as a Bazel bug, but that shouldn't prevent us from patching together a PR.

I integrated your suggestions in a new commit for better readability.  Feel free to fixup.

@hchauvin Thanks! I did a small fixup and I want to add another test (or change the existing happy path) to show passing two different bazel_external_dependency_archive to a test.
I'm waiting with it since I'm not sure I understand the failure-case test (where I commented) and I'd rather we not have conflicts.

@hchauvin I'd appreciate your review on my last two commits

@hchauvin I’m thinking of a follow up to this API by exposing a method on WorkspaceDriver which given a name (file name?) returns the sha and URLs.
We have this info in skylark and it can remove yet another (third) repetition of the sha256 and URLs in the scratch workspace.
The con is that this is more logic to maintained and it’s a bit magical.
Wdyt?


myqlm version conflict

Seems Like there is a conflict issue whenever i try to install the package (i have tried on multiple distributions and virtual envirenoments but none seems to work and i get the same error each time)

Am i doing something wrong? If yes, would you mind telling me what it is ?
Thanks in advance,
Max

Dear @MaxB0zz ,
Which version of Python and which plateform (e.g. Linux, MacOS) do you use?

Good day,
i have tried on Ubuntu, Windows and Wsl2 with python 3.8.10 and 3.11.1. Same result on each of these versions.

Good morning,
Unfortunatly, myQLM is not supported on Python 3.11 - your error message is expected for this version of Python. Supported version of Python is listed in myQLM documentation
Nevertheless, Python 3.8 is supported for both Windows and Ubuntu. Are you using x84_64 computer?


[AST] Update internal AST logic

Signed-off-by: Jacob Levy<EMAIL_ADDRESS>

@rswarbrick
Can somone help me view what is the failure of the "CI (private) Failing after 18m — Build #20210615.23 failed"
Thnks!

Here's the error:
2021-06-15T11:30:13.2940453Z ## CHIP Simulation Results
2021-06-15T11:30:13.2940872Z ### Tuesday June 15 2021 11:13:20 UTC
2021-06-15T11:30:13.2941486Z ### GitHub Revision: [`cea7ea03d`](https://github.com/lowrisc/opentitan/tree/cea7ea03d992d02142f3b6abea363df74f39537f)
2021-06-15T11:30:13.2942037Z ### Branch: HEAD
2021-06-15T11:30:13.2942546Z ### [Testplan](https://docs.opentitan.org/hw/top_earlgrey/doc/dv_plan/#testplan)
2021-06-15T11:30:13.2943269Z ### Simulator: VCS
2021-06-15T11:30:13.2943538Z
2021-06-15T11:30:13.2943873Z ### Test Results
2021-06-15T11:30:13.2944364Z |  Milestone  |          Name           | Tests              |  Passing  |  Total  |  Pass Rate  |
2021-06-15T11:30:13.2945493Z |:-----------:|:-----------------------:|:-------------------|:---------:|:-------:|:-----------:|
2021-06-15T11:30:13.2946289Z |     V1      |     chip_uart_tx_rx     | chip_sw_uart_tx_rx |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2946939Z |     V1      |  chip_uart_rx_overflow  | chip_sw_uart_tx_rx |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2947587Z |     V1      |      csr_hw_reset       | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2948238Z |     V1      |         csr_rw          | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2948904Z |     V1      |       xbar_smoke        | xbar_chip_smoke    |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2949597Z |     V1      |                         | **TOTAL**          |     0     |    4    |   0.00 %    |
2021-06-15T11:30:13.2950256Z |     V2      |       enable_reg        | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2950870Z |     V2      | tl_d_outstanding_access | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2951515Z |             |                         | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2952165Z |     V2      |   tl_d_partial_access   | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2952784Z |             |                         | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2953434Z |             |                         | **TOTAL**          |     0     |    4    |   0.00 %    |
2021-06-15T11:30:13.2953797Z
2021-06-15T11:30:13.2954163Z ## Failure Buckets
2021-06-15T11:30:13.2954395Z
2021-06-15T11:30:13.2955367Z * `Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'` has 4 failures:
2021-06-15T11:30:13.2956054Z     * Test chip_sw_uart_tx_rx has 1 failures.
2021-06-15T11:30:13.2956538Z         * 0.chip_sw_uart_tx_rx.1\
2021-06-15T11:30:13.2957420Z           Line 264, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_sw_uart_tx_rx/out/run.log
2021-06-15T11:30:13.2957881Z
2021-06-15T11:30:13.2958659Z                 	Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2959427Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2960372Z
2021-06-15T11:30:13.2960788Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2961230Z
2021-06-15T11:30:13.2961457Z
2021-06-15T11:30:13.2961816Z     * Test xbar_chip_smoke has 1 failures.
2021-06-15T11:30:13.2962262Z         * 0.xbar_chip_smoke.1\
2021-06-15T11:30:13.2963193Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.xbar_chip_smoke/out/run.log
2021-06-15T11:30:13.2963616Z
2021-06-15T11:30:13.2964372Z                 	Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2965269Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2965876Z
2021-06-15T11:30:13.2966299Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2966741Z
2021-06-15T11:30:13.2966972Z
2021-06-15T11:30:13.2967342Z     * Test chip_csr_hw_reset has 1 failures.
2021-06-15T11:30:13.2967768Z         * 0.chip_csr_hw_reset.1\
2021-06-15T11:30:13.2968697Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_csr_hw_reset/out/run.log
2021-06-15T11:30:13.2969116Z
2021-06-15T11:30:13.2970225Z                 	Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2970949Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2971566Z
2021-06-15T11:30:13.2971983Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2972443Z
2021-06-15T11:30:13.2972672Z
2021-06-15T11:30:13.2973041Z     * Test chip_csr_rw has 1 failures.
2021-06-15T11:30:13.2973456Z         * 0.chip_csr_rw.1\
2021-06-15T11:30:13.2974354Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_csr_rw/out/run.log
2021-06-15T11:30:13.2974767Z
2021-06-15T11:30:13.2975705Z                 	Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2976449Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2977057Z
2021-06-15T11:30:13.2977499Z                 SYS Clock Period: 10000ps

I've also reached out to you @Jacob-Levy via email to sort out the permission issues.

Hi Tim,
Il looks that the we have mismatch on the NumAlerts.  The AST has 13 and the sensor control has only 11.
I looked at the  sensor_ctrl_reg_pkg.sv and you are missing OT4 (RECOV_OT4) and OT6 ( RECOV_OT5),
Can we coordinate the fix in order to promote the PR?
I am not sure if it is only the sensor_ctrl.hjson that need to be fixed. Probably more files.
Regards,
Jacob

@Jacob-Levy since you already updated sensor_ctrl in this PR, why not go with that?
Do you mean inside the sensor_ctrl and not just the hjson changes?

I got too many comflict so I cosed the PR. Can you update the hjson & what ever needed to suport 13 Alerts and I will reissue the PR.

sorry i'm still trying to bring my environment up to our most current
working model, so it will take a bit longer.
On Wed, Jun 16, 2021 at 1:29 PM Jacob-Levy @.***> wrote:

I got too many comflict so I cosed the PR. Can you update the hjson & what
ever needed to suport 13 Alerts and I will reissue the PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/lowRISC/opentitan/pull/6977#issuecomment-862699695,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAH2RST22X6K76VCTMMEDHLTTECRHANCNFSM46W5COSA
.

Hi Philipp,
Do you know how the check on NumAlerts ended up in CI(private)?
It should have been a warning and not an error since it will always fail if the change it not done at the same PR (for AST and Sensor Control).
Regards,
Jacob

From: Philipp Wagner @.>
Sent: Wednesday, June 16, 2021 11:13:12 AM
To: lowRISC/opentitan @.>
Cc: IC10 Jacob Levy @.>; Mention @.>
Subject: Re: [lowRISC/opentitan] [AST] Update internal AST logic (#6977)
Here's the error:
2021-06-15T11:30:13.2940453Z ## CHIP Simulation Results
2021-06-15T11:30:13.2940872Z ### Tuesday June 15 2021 11:13:20 UTC
2021-06-15T11:30:13.2941486Z ### GitHub Revision: cea7ea03d
2021-06-15T11:30:13.2942037Z ### Branch: HEAD
2021-06-15T11:30:13.2942546Z ### Testplan
2021-06-15T11:30:13.2943269Z ### Simulator: VCS
2021-06-15T11:30:13.2943538Z
2021-06-15T11:30:13.2943873Z ### Test Results
2021-06-15T11:30:13.2944364Z |  Milestone  |          Name           | Tests              |  Passing  |  Total  |  Pass Rate  |
2021-06-15T11:30:13.2945493Z |:-----------:|:-----------------------:|:-------------------|:---------:|:-------:|:-----------:|
2021-06-15T11:30:13.2946289Z |     V1      |     chip_uart_tx_rx     | chip_sw_uart_tx_rx |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2946939Z |     V1      |  chip_uart_rx_overflow  | chip_sw_uart_tx_rx |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2947587Z |     V1      |      csr_hw_reset       | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2948238Z |     V1      |         csr_rw          | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2948904Z |     V1      |       xbar_smoke        | xbar_chip_smoke    |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2949597Z |     V1      |                         | TOTAL          |     0     |    4    |   0.00 %    |
2021-06-15T11:30:13.2950256Z |     V2      |       enable_reg        | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2950870Z |     V2      | tl_d_outstanding_access | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2951515Z |             |                         | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2952165Z |     V2      |   tl_d_partial_access   | chip_csr_hw_reset  |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2952784Z |             |                         | chip_csr_rw        |     0     |    1    |   0.00 %    |
2021-06-15T11:30:13.2953434Z |             |                         | TOTAL          |     0     |    4    |   0.00 %    |
2021-06-15T11:30:13.2953797Z
2021-06-15T11:30:13.2954163Z ## Failure Buckets
2021-06-15T11:30:13.2954395Z
2021-06-15T11:30:13.2955367Z * Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)' has 4 failures:
2021-06-15T11:30:13.2956054Z     * Test chip_sw_uart_tx_rx has 1 failures.
2021-06-15T11:30:13.2956538Z         * 0.chip_sw_uart_tx_rx.1
2021-06-15T11:30:13.2957420Z           Line 264, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_sw_uart_tx_rx/out/run.log
2021-06-15T11:30:13.2957881Z
2021-06-15T11:30:13.2958659Z                    Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2959427Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2960372Z
2021-06-15T11:30:13.2960788Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2961230Z
2021-06-15T11:30:13.2961457Z
2021-06-15T11:30:13.2961816Z     * Test xbar_chip_smoke has 1 failures.
2021-06-15T11:30:13.2962262Z         * 0.xbar_chip_smoke.1
2021-06-15T11:30:13.2963193Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.xbar_chip_smoke/out/run.log
2021-06-15T11:30:13.2963616Z
2021-06-15T11:30:13.2964372Z                    Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2965269Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2965876Z
2021-06-15T11:30:13.2966299Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2966741Z
2021-06-15T11:30:13.2966972Z
2021-06-15T11:30:13.2967342Z     * Test chip_csr_hw_reset has 1 failures.
2021-06-15T11:30:13.2967768Z         * 0.chip_csr_hw_reset.1
2021-06-15T11:30:13.2968697Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_csr_hw_reset/out/run.log
2021-06-15T11:30:13.2969116Z
2021-06-15T11:30:13.2970225Z                    Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2970949Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2971566Z
2021-06-15T11:30:13.2971983Z                 SYS Clock Period: 10000ps
2021-06-15T11:30:13.2972443Z
2021-06-15T11:30:13.2972672Z
2021-06-15T11:30:13.2973041Z     * Test chip_csr_rw has 1 failures.
2021-06-15T11:30:13.2973456Z         * 0.chip_csr_rw.1
2021-06-15T11:30:13.2974354Z           Line 94, in log /azp/agent/_work/1/s/scratch/HEAD/chip_earlgrey_asic-sim-vcs/0.chip_csr_rw/out/run.log
2021-06-15T11:30:13.2974767Z
2021-06-15T11:30:13.2975705Z                    Offending '(ast_pkg::NumAlerts == sensor_ctrl_reg_pkg::NumAlerts)'
2021-06-15T11:30:13.2976449Z                 UVM_ERROR ../src/lowrisc_systems_sensor_ctrl_0.1/rtl/sensor_ctrl.sv(41) @ 0: reporter [ASSERT FAILED] NumAlertsMatch_A
2021-06-15T11:30:13.2977057Z
2021-06-15T11:30:13.2977499Z                 SYS Clock Period: 10000ps
I've also reached out to you @Jacob-Levyhttps://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FJacob-Levy&data=04|01|jacob.levy%40nuvoton.com|61896399ee094c7f4fee08d9309e973a|a3f24931d4034b4a94f17d83ac638e07|0|0|637594279964171577|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=1lq%2ByMcbt%2FgiWihB%2FNcATf8ov5jEUR4cY1lV%2B6XxZPw%3D&reserved=0 via email to sort out the permission issues.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHubhttps://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FlowRISC%2Fopentitan%2Fpull%2F6977%23issuecomment-862152175&data=04|01|jacob.levy%40nuvoton.com|61896399ee094c7f4fee08d9309e973a|a3f24931d4034b4a94f17d83ac638e07|0|0|637594279964181566|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=apXvCgcU4qi2eu5zHt%2B0jzQIVn1S%2FJ7%2BMUeLAacdepU%3D&reserved=0, or unsubscribehttps://apc01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAQA3TGWUSHCDFPG64L3QRA3TTBMJRANCNFSM46W5COSA&data=04|01|jacob.levy%40nuvoton.com|61896399ee094c7f4fee08d9309e973a|a3f24931d4034b4a94f17d83ac638e07|0|0|637594279964181566|Unknown|TWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D|1000&sdata=CsM2pLHJH19pdNllKbZw67TWxllALyCI83wF7sBurHI%3D&reserved=0.

The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.

The assert in sensor_ctrl.sv https://github.com/lowRISC/opentitan/blob/40a76c66e5222ee78d8acc18dd8a501f22f38be7/hw/top_earlgrey/ip/sensor_ctrl/rtl/sensor_ctrl.sv#L40-L41 was added by @tjaychen to ensure consistency between the modules. I didn't dig deeper into how these parameters are used, so I don't know if it could be made a warning instead of an assert. But it's probably best anyways to update ast and sensor_ctrl in the same pull request to avoid the inconsistency in the first place.
Otherwise I'm sure @tjaychen will be able to have a look once he has finished digging through his backlog.


Refactor err codes

#12217 an #12276 taught us that errno codes aren't necessarily organised as a small sequential set of numbers.  In fact, POSIX stipulates that the macros should be used for the codes and that no other assumptions can be made.
We therefore need to rethink how we pack errno codes into our own ERR_LIB_SYS codes.  This may prove more dodgy than we can handle somewhere in the future, but for the moment, the changes made here are hopefully enough to handle what we currently know.
This change also deals with an overlap of ERR_R_ codes (which has already appeared as strange error messages)
Finally, we change test/errtest.c to use EPERM instead of 1.
Fixes #12217
Fixes #12276

This is an alternative to #12289, using similar ideas for the codes.

I think doing all this work for one platform, GNU Hurd, is misplaced effort.

I think doing all this work for one platform, GNU Hurd, is misplaced effort.

It's also for all other possibilities.  We had problems previously on Windows, see how I removed the Windows specific hackery from 02-test_errstr.t.
Also, most of the work is not hurd specific, but is meant to generalise in a more future proof manner.  Plus it deals with certain reason code overlaps that haven't been dealt with for eons (I'm surprised no one else noticed)

The base problem is that OpenSSL doesn't allow enough bits for errno on some platforms. This  doesn't seem to address the fact that, for example, errno is allowed to be an int.  I would expect an explicit errno field added:
struct err_state_st {
  int syserr[ERR_ NUM_ERRORS];
 ...
};

And then teach the "get error string" functions to look at syserr element for LIB_SYS errors.
Also, that cascading ifdef-based list is really ripe for auto-generating.

Also, that cascading ifdef-based list is really ripe for auto-generating.

Yeah, I was thinking of that, mostly because it exists in two places.  We still need a stable source if E macro names, which could be a perl script that takes them from the Errno module.

The base problem is that OpenSSL doesn't allow enough bits for errno on some platforms. This doesn't seem to address the fact that, for example, errno is allowed to be an int. I would expect an explicit errno field added

The base problem is that we have all kinds of info packed into an int.  We could have a small struct that we pass around by value (this is supported in C) instead, and thereby allow the reason code to take on the full size of an int.

Yes, you could turn error into a struct if you didn't care about "most applications will just need to recompile."
My proposal solves the root problem simply.  Want a PR?

Uhmmmmm, how exactly did you mean to do that?  After all, functions like ERR_peek_error, ERR_error_string, the ERR_GET_ functions/macros all return or expect a complete packed error code (i.e that includes the library code and the reason code, and formerly the function code).

I read the confused, and perhaps I'm just missing something, but how does this PR not return a truncated error code?

The added err_pack() repacks hurd error codes by taking away the middle bits. In concrete terms, the code xxyyzztt is re-packed to xxzztt, because it turns out that yy is 00, or ff for negative errors, and is therefore not significant.
For all other platforms, it is indeed truncated to the lowest 24 bits of errno. As far as I have seen so far, that shouldn't be a problem, but if it is, adding code to re-pack another platform's code in err_pack should be fairly simple, and if it turns out not to be true, we'll take the problem then.

Thanks for the explanation, I missed that but still a little confused by your "xxyyzztt" notation, and "xxzztt" is six nibbles not eight.
I still feel that this is a hack for a non-common platform, but oh well.

but still a little confused by your "xxyyzztt" notation, and "xxzztt" is six nibbles not eight.

It means that of bits 0 to 31, I discard bits 16 to 23, because from reading https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h, it seems that those are not significant.

Any @openssl/committers willing to review?

Any @openssl/committers willing to review?

I can do.

but still a little confused by your "xxyyzztt" notation, and "xxzztt" is six nibbles not eight.

It means that of bits 0 to 31, I discard bits 16 to 23, because from reading https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h, it seems that those are not significant.

For the OpenSSL-internal representation for the POSIX error codes
better use the index of the values held in the known_codes array.
This way the encoding is optimal (in the sense of minimal size)
and no assumption is needed about the external encoding of the values.
Note that according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
the only assumption we can make about the external encoding  is that they are distinct positive values.

In other words,
simply re-enumerate the defined POSIX error names/macros and use the resulting values as their internal encoding.
In this way we are entirely independent of the encoding defined in errno.h and the max value is as small as
NUM_SYS_STR_REASONS OSSL_NELEM(known_codes) - 1

For the OpenSSL-internal representation for the POSIX error codes
better use the index of the values held in the known_codes array.
This way the encoding is optimal (in the sense of minimal size)
and no assumption is needed about the external encoding of the values.
Note that according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
the only assumption we can make about the external encoding is that they are distinct positive values.

However, on systems that still support the deprecated sys_nerr and sys_errlist[] interfaces (even with compile-time warnings), it is perhaps not unreasonable to be much less pessimistic. Such systems do use consecutive small integers for errno values.  This includes recent glibc, where on a Fedora 31 system I find sys_nerr == 135, and 133 as the highest defined errno value in errno.h.
Which is not to say that pessimism could not be the simplest global strategy, just adding context...

For the OpenSSL-internal representation for the POSIX error codes
better use the index of the values held in the known_codes array.
This way the encoding is optimal (in the sense of minimal size)
and no assumption is needed about the external encoding of the values.

Except the system errors are expected to be set like this:
    ERR_raise_data(ERR_LIB_SYS, errno, "open()");
    SYSerr(SYS_F_OPEN, errno);

So it would mean that we need to keep track of the index used for each errno, i.e. search that array.  I agree that it's doable!  Is that a better choice?

Well, although most POSIX implementation will follow that straightforward and minimal encoding model,
we meanwhile know at least one exception to it (Hurd).
It would be dangerous to try recognizing and enumerating all those exceptions and coming up with a condensation scheme for each of them. Which may even break in the future for those exceptions we have already been handling.
The solution that I suggested is pretty simple, optimal in internal error code size, and fully general without the risk of breaking due to potential future changes to the external encoding provided in whatever variant of errno.h.

Except the system errors are expected to be set like this:
    ERR_raise_data(ERR_LIB_SYS, errno, "open()");
    SYSerr(SYS_F_OPEN, errno);

So it would mean that we need to keep track of the index used for each errno, i.e. search that array. I agree that it's doable! Is that a better choice?

Correct that in this use likely an inefficient search (or some fancy optimization) is needed, but this is done rarely (only when preparing error output).
I still think that such an encoding has more benefits in terms of stability and encoding size.

... and what do we do with errno's that we haven't pre-registered in that array? Do we add them on the fly? That means memory allocation, possible failure of memory allocation, which is something we try to avoid in the ERR sub-system

I don't think this is really needed.
Why bother much with errno values that are not even known statically at build time?
I would simply add a catch-all value for them, with a name like UNKNOWN_ERRNO.

Correct that for this use case likely an inefficient search (or some fancy optimization) is needed,
but this is done rarely (only when preparing error output).

Er, no, it would happen any time ERR_set_error() is called (which happens at the SYSerr() / ERR_raise_data() calls), because it needs to pack that as a reason code into the error record.

Except the system errors are expected to be set like this:

    ERR_raise_data(ERR_LIB_SYS, errno, "open()");
    SYSerr(SYS_F_OPEN, errno);

Also in terms of places in the code where such things are needed this seems negligible.
fgrep " errno" crypto/ ssl apps test -r|grep -e 'ERR_raise_data\|SYSerr'
yields just
crypto/conf/conf_def.c:        ERR_raise_data(ERR_LIB_SYS, errno,
crypto/store/loader_file.c:            ERR_raise_data(ERR_LIB_SYS, errno,
apps/lib/apps.c:        ERR_raise_data(ERR_LIB_SYS, errno,

[ahem]
$ git grep -E -e 'ERR_raise(_data)? *\( *ERR_LIB_SYS'
apps/lib/apps.c:        ERR_raise_data(ERR_LIB_SYS, errno,
crypto/bio/b_addr.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_addr.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_addr.c:                ERR_raise_data(ERR_LIB_SYS, 1000,
crypto/bio/b_addr.c:                ERR_raise_data(ERR_LIB_SYS, 1000 + h_errno,
crypto/bio/b_addr.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_addr.c:                    ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock.c:        ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/bio/b_sock.c:            ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/bio/b_sock.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:        ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_sock2.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/bss_acpt.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/bss_conn.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/bss_conn.c:                    ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/bss_conn.c:                ERR_raise_data(ERR_LIB_SYS, i,
crypto/bio/bss_file.c:        ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/bio/bss_file.c:            ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/bio/bss_file.c:            ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/bio/bss_file.c:            ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
crypto/conf/conf_def.c:        ERR_raise_data(ERR_LIB_SYS, errno,
crypto/store/loader_file.c:            ERR_raise_data(ERR_LIB_SYS, errno,
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:    ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
engines/e_devcrypto.c:        ERR_raise_data(ERR_LIB_SYS, errno, "calling ioctl()");
include/openssl/err.h:#  define SYSerr(f, r) ERR_raise_data(ERR_LIB_SYS, (r), NULL)
ssl/ssl_cert.c:        ERR_raise_data(ERR_LIB_SYS, get_last_sys_error(),
ssl/ssl_lib.c:    ERR_raise_data(ERR_LIB_SYS, ERR_R_INTERNAL_ERROR, "calling sendfile()");
util/err-to-raise:s/SYSerr\(\w+, *(\w+)\)/ERR_raise(ERR_LIB_SYS, $1)/;

(the ERR_LIB_SYS line in ssl/ssl_lib.c is terribly wrong, btw)

Re UNKNOWN_ERRNO...  so far, we have had the reason text reason(nnn) with the actual reason number for any error where we don't have a text.  So for an errno 132 that we hadn't got a text for, we would display reason(132), which gave people a chance to find out what's going on even if we couldn't tell.  Maybe people don't care, I dunno, but suddenly serving them a generic "we have no clue" error text feels...  like a bit of a downgrade.
I have no idea if I'm over inflating the issue...

Correct that for this use case likely an inefficient search (or some fancy optimization) is needed,
but this is done rarely (only when preparing error output).

Er, no, it would happen any time ERR_set_error() is called (which happens at the SYSerr() / ERR_raise_data() calls), because it needs to pack that as a reason code into the error record.

This is what I meant by "preparing error output".
That is, only when an error situation has been detected, which may be at many places in the code,
but not often in terms of run-time execution, such that an inefficient search called at this point should not be a problem.

$ git grep -E -e 'ERR_raise(_data)? *( *ERR_LIB_SYS'
apps/lib/apps.c:        ERR_raise_data(ERR_LIB_SYS, errno,
crypto/bio/b_addr.c:                ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
crypto/bio/b_addr.c:            ERR_raise_data(ERR_LIB_SYS, get_last_socket_error(),
...

Right, but even if there are many such occurrences this is not an efficiency problem.
The mapping of the POSIX error code to the internal one would be done in (an updated version of) err_pack(),
which is stable and independent of the OS, without the need for hacks like #ifdef __gnu_hurd__).

Re UNKNOWN_ERRNO... so far, we have had the reason text reason(nnn) with the actual reason number for any error
where we don't have a text. So for an errno 132 that we hadn't got a text for, we would display reason(132), which gave
people a chance to find out what's going on even if we couldn't tell. Maybe people don't care, I dunno, but suddenly serving > them a generic "we have no clue" error text feels... like a bit of a downgrade.

Even better than UNKNOWN_ERRNO :)

Even better than UNKNOWN_ERRNO :)

So how do you propose that when recording the error (i.e. the errno)?

Even better than UNKNOWN_ERRNO :)

So how do you propose doing that when recording the error (i.e. the errno)?

Good question. How about the following?

If the errno value is found in the enumeration table we record internally a new pseudo-lib code (such as ERR_LIB_ERRNO) and the found enumeration index (which is < NUM_SYS_STR_REASONS) and have a mnemonic text to display.

In the (not very likely) case that the errno value is not found in the enumeration table,
we can display for instance UNKNOWN_ERRNO=132 in analogy to your above example.
To this end we can record internally, similarly as before,
ERR_LIB_SYS and errno & 0xfffff (since unfortunately we only have 24 bits at our disposal),
hoping that this gives somewhat useful info to any interested user.

If the errno value is found in the enumeration table we record internally a new pseudo-lib code (such as ERR_LIB_ERRNO) and the found enumeration index (which is < NUM_SYS_STR_REASONS) and have a mnemonic text to display.

Hmm, I'm frankly a bit ambivalent re another mysterious pseudo-lib...  an alternative would be another RFLAG for error code indirection, having the value 0x8 (I'll explain why).

In the (not very likely) case that the errno value is not found in the enumeration table, we can display for instance UNKNOWN_ERRNO=132 in analogy to your above example.  To this end we can record internally, similarly as before, ERR_LIB_SYS and errno & 0xffffff (since unfortunately we only have 24 bits at our disposal), hoping that this gives somewhat useful info to any interested user.

That would be a recorded error without the RFLAG I'm proposing, and considering I'm proposing that it gets the highest bit, this would take down the truncation by only one bit, 0x7fffff.
@sebastianas, would such truncation be acceptable to you?  it would essentially meant that if we don't have internal knowledge of some Hurd specific error code - say EMACH_SEND_IN_PROGRESS for the hell of it -, it would be displayed as reason(1), which, as far as I can read https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h, would possibly mean any of EPERM, EMACH_SEND_IN_PROGRESS, or EKERN_INVALID_ADDRESS.  Is that acceptable?

Re UNKNOWN_ERRNO... so far, we have had the reason text reason(nnn) with the actual reason number for any error where we don't have a text. So for an errno 132 that we hadn't got a text for, we would display reason(132), which gave people a chance to find out what's going on even if we couldn't tell. Maybe people don't care, I dunno, but suddenly serving them a generic "we have no clue" error text feels... like a bit of a downgrade.
I have no idea if I'm over inflating the issue...

I agree that losing code points is very much not a good idea. In particular, on platforms that have a known sufficiently small sys_nerr we should probably avoid the encoding entirely.
On which systems is any of this needed?

On which systems is any of this needed?

Potentially any that made their own numbers instead of copying errno.h from BSD...  this has been a latent problem waiting to happen, I'm actually surprised it hasn't happened before now...
Regarding sys_nerr and sys_errlist, they are not something we look at.  strerror() is sufficiently old that we can count on its presence (or well, variants thereof, all wrapped with openssl_strerror_r()

@vdukhovni this PR is because of a problem found on Gnu HURD.

@vdukhovni this PR is because of a problem found on Gnu HURD.

Thanks for the context. Might it make sense to restrict the solution to the problem platforms?  Otherwise the errno value may need to be removed from the OpenSSL error object, and stored separately.  I very much not fond of tables with "well-known" errors.

So basically, you're asking for more checks of the __gnu_hurd__ macro

I very much not fond of tables with "well-known" errors.

As @richsalz points out, we could generate it on a per-system basis.  But for your consideration, we have always assumed something "well-known"...  before this PR, the assumption has been that the errno range is 1..127 (which is a false assumption if you listen to POSIX).

But for your consideration, we have always assumed something "well-known"... before this PR, the assumption has been that the errno range is 1..127 (which is a false assumption if you listen to POSIX).

A known range is very different from a known list of codepoints. Of course 127 is rather tight. With glibc on Fedora 31, the current highest errno is 133.  The real problem is packing errno values into less than 32 bits, the real solution has to be to stop doing that, not make up some ad-hoc compression scheme.

As @richsalz points out, we could generate it on a per-system basis. But for your consideration, we have always assumed something "well-known"... before this PR, the assumption has been that the errno range is 1..127 (which is a false assumption if you listen to POSIX).

Yes - as already mentioned above, the problem addressed in this PR is that according to https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html the only assumptions that we can safely make on the errno values that
they are defined as macros which shall expand to integer constant expressions with type int, distinct positive values.
In earlier times, the guarantees were even weaker (namely, that they are non-zero values).

So am I to get this right, that we should only do the indirect errno solution on platforms where we know that they don't fit in 20 bits?  Is a pre-defined table of mappings acceptable in that case?  What should we do with errno's that aren't in that table?

So am I to get this right, that we should only do the indirect errno solution on platforms where we know that they don't fit in 20 bits? Is a pre-defined table of mappings acceptable in that case? What should we do with errno's that aren't in that table?

It would not feel right to me, also from the usage perspective, to make a case distinction between systems where (it looks like that) the errno values are integers fit in 20 bits and where not.

Might it make sense to restrict the solution to the problem platforms? Otherwise the errno value may need to be removed from the OpenSSL error object, and stored separately.

Meanwhile I think the most straightforward solution would be to record in our 32-bit error code only the fact that a system error occurred and
let our default error queue printing code, as well anyone else interested in the details, simply consult errno by calling an accessor function like get_last_socket_error().

let our default error queue printing code, as well anyone else interested in the details, simply consult errno by calling an accessor function like get_last_socket_error().

What makes you think the errno has been  preserved all the way 'til ERR_print_errors or similar is called?

What makes you think the errno has been preserved all the way 'til ERR_print_errors or similar is called? Or when something as simple as ERR_GET_REASON() is called?

Ah, right, we'll need to copy and store it along with the respective error queue entry.

Ah, right, we'll need to copy and store it along with the respective error queue entry.

This is the solution that @richsalz proposed, but there are issues with that proposal, i.e. all the functions that pop the error off the queue (such as ERR_get_error_all, which is used in ERR_print_errors_cb)...  you get the recorded error code, and the queued entry is essentially gone (it may still be there, but it may also be overwritten by another recorded error), i.e. if you have an indirect reason that expects the real reason code to be in the queued entry, we have a problem.

Ah, right, we'll need to copy and store it along with the respective error queue entry.

This is the solution that @richsalz proposed, but there are issues with that proposal, i.e. all the functions that pop the error off the queue (such as ERR_get_error_all, which is used in ERR_print_errors_cb)... you get the recorded error code, and the queued entry is essentially gone (it may still be there, but it may also be overwritten by another recorded error), so if you have an indirect reason that expects the real reason code to be in the queued entry, we have a problem.

I think that this is an independent issue (namely, of being able to get random access to the queue entries) and,
above of that, also the solution to somehow squeeze the errno value in the error code would have the same issue.
Thus I don't see this issue as a valid argument against any way of storing (most of) the errno value along with the queue entry.

I think that this is an independent issue (namely, of being able to get random access to the queue entries) and,

Er, I have no idea how you see this as a path going forward.  With such a solution, we're fixing one issue by knowingly introducing another much more complex issue.

above of that, also the solution to somehow squeeze the errno value in the error code would have the same issue.

I disagree.  There's a huge difference between a squeeze that gives some value and loosing that value altogether.

The situation could, of course, be resolved by changing the ERR sub-system to have a larger type for the packed error code.  Say, uint64_t.  Unfortunately, we've promised to stay source compatible, so we're stuck with 32-bit values.

I think that this is an independent issue (namely, of being able to get random access to the queue entries) and,

Er, I have no idea how you see this as a path going forward. With such a solution, we're fixing one issue by knowingly introducing another much more complex issue.

above of that, also the solution to somehow squeeze the errno value in the error code would have the same issue.

I disagree. There's a huge difference between a squeeze that gives some value and loosing that value altogether.

Looks like there is some misunderstanding among us here. I'll try to be a little more detailed/explicit:
The issue I just mentioned above as a side note, namely that we only offer queue-like access to the error queue entries, not allowing random access, is not something that I propose to change (nor to discuss further) here.
The issue that (the copy of) the errno value, when stored along with a queue entry indicating a system error, can become inaccessible by chopping off queue entries, is independent of how in detail the value is stored.
Or which essential difference do you see in this respect comparing the approaches discussed so far?
1 Store the whole errno value in a separate queue entry component as suggested by @richsalz, which I meanwhile favor.
2a Store a (depending on the system, potentially somewhat squeezed) version of the errno value in the lower 20 bits of the err code, as currently done in this PR.
2b Store in the lower  lower 20 bits of the err code an index of the errno value as far as it is known to us, else its lower 20 bits (as I had suggested yesterday and adapted by you from 24 to 20 bits).

I think the core problem cannot be solved the "right way" without breaking source compatibility, which means it would have to wait for a future release.  Until then, it seems that only GNU HURD is affected (the rest are theoretical), which doesn't seem to be an important platform right now.

There are parts of this PR that isn't about Hurd...  I'll break them apart so they can be reviewed separately.

On 2020-06-30 05:45:48 [-0700], Richard Levitte wrote:

@sebastianas, would such truncation be acceptable to you?  it would essentially meant that if we don't have internal knowledge of some Hurd specific error code - say EMACH_SEND_IN_PROGRESS for the hell of it -, it would be displayed as reason(1), which, as far as I can read https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h, would possibly mean any of EPERM, EMACH_SEND_IN_PROGRESS, or EKERN_INVALID_ADDRESS.  Is that acceptable?

I had a patch where I shifted the upper bit so that it is gets not
truncated. Would that work? I don't mind restricting this shifting to
Hurd and keep a test to find other platform with "different" number
scheme where we would lose information.
I think this shifting didn't work because there is a list that saves the
error numbers which are pre-generated at library start time.
Why do we need that? According the the manpage for `strerror()' that
string isn't modified by any other library function. This seems to be
also true Hurd's libc.
In terms of "acceptable". What does this that the error code gets
truncated for the user? Does he lose information like EAGAIN / ESHUTDOWN
or is this just a consequence for the textual representation of the
error code?
Sebastian

I had a patch where I shifted the upper bit so that it is gets not truncated. I don't mind restricting this shifting to Hurd and keep a test to find other platform with "different" number scheme where we would lose information.

That's exactly what this PR does

I think this shifting didn't work because there is a list that saves the error numbers which are pre-generated at library start time.
Why do we need that? According the the manpage for `strerror()' that string isn't modified by any other library function. This seems to be also true Hurd's libc.

We pre-load the error strings to fit into our error string structure.  Previous to this PR, we simply used the range 1..127, which doesn't fit what you'd expect on Hurd, since the POSIX numbers (which are the most commonly used) are in the range 0x40000001..0x40000079 (according to https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h), so for Hurd, we're entirely missing the target.

I've moved the non-Hurd bits to #12343

So I had another wild...  but workable thought last evening.  This comes from the fact that errno value shall fit in a int and be positive, so for a 32-bit int, they have the range 0x1..0x7fffffff.
So what if we seriously played with bits (something we should be quite good at, all things considered :wink:), and encoded our error records as follows?
System errors:
<---------------------------- 32 bits -------------------------->
+-+-------------------------------------------------------------+
|1|                  system error number (31 b)                 |
+-+-------------------------------------------------------------+

OpenSSL errors:
<---------------------------- 32 bits -------------------------->
+-+---------------+---------------------------------------------+
|0| library (8 b) |                  reason (21 b)              |
+-+-------------------------------------------------------------+

No system error number crunching needed whatsoever.  Only one flipped bit, which is easily special cased in our error code, and even in our ERR_GET macros.

I also wonder, why do we pre-load the system errors?  Yeah ok, if we want to make it easily fit in OpenSSL error messages without special casing anything...  but if we go along with my wilder idea, isn't that a moot point?  That saves us from the whole sys string reason array mess, just call openssl_strerror_r() when we actually need the string.

So I had another wild... but workable thought last evening. This comes from the fact that errno value shall fit in a int and be positive, so for a 32-bit int, they have the range 0x1..0x7fffffff.
So what if we seriously played with bits (something we should be quite good at, all things considered wink), and encoded our error records as follows?
System errors:
<---------------------------- 32 bits -------------------------->
+-+-------------------------------------------------------------+
|1|                  system error number (31 b)                 |
+-+-------------------------------------------------------------+

OpenSSL errors:
<---------------------------- 32 bits -------------------------->
+-+---------------+---------------------------------------------+
|0| library (8 b) |                  reason (23 b)              |
+-+---------------+---------------------------------------------+

No system error number crunching needed whatsoever. Only one flipped bit, which is easily special cased in our error code, and even in our ERR_GET macros.

Excellent thought! I fully support the essence of this idea.
I'd like to bring in just two minor further tweaks:

Use the 0 bit for the errno value, such that our error code then reads identical to it:

<---------------------------- 32 bits -------------------------->
+-+-------------------------------------------------------------+
|0|                  system error number (31 b)                 |
+-+-------------------------------------------------------------+

Consequently use the 1 bit for the OpenSSL-specific errors.
2. If there is a chance to use just 7 bits for the library info, would be nice to keep the byte-wise alignment as follows:
<---------------------------- 32 bits -------------------------->
+-+---------------+---------------------------------------------+
|1| library (7 b) |                  reason (24 b)              |
+-+---------------+---------------------------------------------+

In German, I would comment on the current state of the discussion:
"Jetzt wird ein Schuh draus" :)
(But maybe better not try automatic translation on that :wink:)

Ouai, et comme je n'pige pas l'allemand, je n'suis pas foutu de savoir c'que tu viens de dire.
(feel free to try and translate :wink:)

Ouai, et comme je n'pige pas l'allemand, je n'suis pas foutu de savoir c'que tu viens de dire.
(feel free to try and translate wink)

I had used the German idiom directly because I like it but was not sure how to express it best in English.
I had hoped that English, German, and Swedish are close enough that you could infer the meaning from the context :)
The meaning of the German idiom "Jetzt wird ein Schuh draus", as far as I understand its common use, is that
"a good achievement (usable product, true insight, or acceptable deal) has been reached".
I've just got a nice hint from a German colleague to try https://www.deepl.com/translate#de/en/Jetzt wird ein Schuh draus
This not only gives the literal translation: "Now it's becoming a shoe"
but after some fiddling: https://www.deepl.com/translate#de/en/es wird ein Schuh draus also provided
"it'll make a shoe", which sounds pretty reasonable to me, but I'm not sure if this idiom also works in English.

Unfortunately, I have nothing similar in my cultures...  thanks for the explanation!

Sorry for being the wise guy, but the original proverb is slightly different:

Umgekehrt wird ein Schuh draus.

The word "umgekehrt" has a dual meaning as "Reversed, ..." (in the literal sense) and "Conversely, ..." (in the figurative sense):
The literal meaning is:

You need to "reverse" the leather to (sew it from both sides to the shoe bed and) produce a high quality shoe.

The figurative meaning is

You need to "think around the corner"  (change your viewpoint, think the other way around) to obtain a good result.

See for example this explanation in german, or its english google translation.

This is a great idea, @levitte.

Thanks @mspncp for the further explaination/detail.

Umgekehrt wird ein Schuh draus.

Yes, that's the original idiom, with the original meaning you gave.
Yet meanwhile there are variations, including the one I used, with naturally different (or at least broader) sense.

You're welcome. It's always fun to have off-topic discussions about foreign language idioms.

Getting back to the technical content,
I wrote above:

If there is a chance to use just 7 bits for the library info, would be nice to keep the byte-wise alignment as follows:

<---------------------------- 32 bits -------------------------->
+-+---------------+---------------------------------------------+
|1| library (7 b) |                  reason (24 b)              |
+-+---------------+---------------------------------------------+

Meanwhile I had a look again at err.h and have the impression that if we change
# define ERR_LIB_USER            128

to
# define ERR_LIB_USER            127

is should be possible to get along with 7 bits for the lib info, right?

Or if the intention was to give user libs a whole range of values with the leading bit being set,
we may go for
# define ERR_LIB_USER            64

and at least after condensing the current values <= 60
we would have still some room for OpenSSL libs/module IDs.

Meanwhile I had a look again at err.h and have the impression that if we for instance change
# define ERR_LIB_USER            128

to
# define ERR_LIB_USER            127

is should be possible to get along with 7 bits for the lib info, right?

No, because that's the starting point...  there can be more than one.  Case in point, each loaded provider gets its own "library" number.

Or if the intention was to give user libs a whole range of values with the leading bit being set, we may go for
# define ERR_LIB_USER            64

and at least after condensing the current values <= 60
we would have still some room for future further OpenSSL lib/module IDs.

Sorry, didn't see this in time.
I'd rather keep the 8 bits for now...  after all, we've hugely expanded the space for reason codes, I don't see a reason to restrict the fairly small library section by one bit merely for some byte boundary purism.

Anyway, I have this new idea underway as added work in #12343, and will close this PR shortly

I'd rather keep the 8 bits for now... after all, we've hugely expanded the space for reason codes, I don't see a reason to restrict the fairly small library section by one bit merely for some byte boundary purism.

I see.
And how about the suggestion to use the leading 0 for errno values and 1 for OpenSSL-specific errors?

And how about the suggestion to use the leading 0 for errno values and 1 for OpenSSL-specific errors?

I was thinking back and forth on that, and it seemed to me like that means code fiddling => more potential mistakes.  I forget the details for the moment, but will get back on that.

Closing this, please have a look at #12343 and continue discussions there

On 2020-07-01 14:48:49 [-0700], Richard Levitte wrote:

I had a patch where I shifted the upper bit so that it is gets not truncated. I don't mind restricting this shifting to Hurd and keep a test to find other platform with "different" number scheme where we would lose information.

That's exactly what this PR does

Okay.

I think this shifting didn't work because there is a list that saves the error numbers which are pre-generated at library start time.
Why do we need that? According the the manpage for `strerror()' that string isn't modified by any other library function. This seems to be also true Hurd's libc.

We pre-load the error strings to fit into our error string structure.  Previous to this PR, we simply used the range 1..127, which doesn't fit what you'd expect on Hurd, since the POSIX numbers (which are the most commonly used) are in the range 0x40000001..0x40000079 (according to https://github.com/lattera/glibc/blob/master/sysdeps/mach/hurd/bits/errno.h), so for Hurd, we're entirely missing the target.

So, one more Hurd detail: strerror() for number which is not implemented
(defined) will segfault.
This works:
diff --git a/crypto/err/err.c b/crypto/err/err.c
index 9a37c426252f3..ca21aa7f6cee9 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -168,6 +168,8 @@ static ERR_STRING_DATA *int_err_get_item(const ERR_STRING_DATA *d)
 CRYPTO_THREAD_read_lock(err_string_lock);
 p = lh_ERR_STRING_DATA_retrieve(int_error_hash, d);

if (p && p->string == NULL && ERR_GET_LIB(d->error) == ERR_LIB_SYS)

p->string = strerror(ERR_GET_REASON(d->error));
CRYPTO_THREAD_unlock(err_string_lock);
return p;
@@ -208,13 +210,14 @@ static void build_SYS_str_reasons(void)
for (i = 1; i <= NUM_SYS_STR_REASONS; i++) {
ERR_STRING_DATA *str = &SYS_str_reasons[i - 1];

   str->error = ERR_PACK(ERR_LIB_SYS, 0, i);

   str->error = ERR_PACK(ERR_LIB_SYS, 0, EPERM - 1 + i);

+#if 0
/*
* If we have used up all the space in strerror_pool,
* there's no point in calling openssl_strerror_r()
*/
if (str->string == NULL && cnt < sizeof(strerror_pool)) {

       if (openssl_strerror_r(i, cur, sizeof(strerror_pool) - cnt)) {

       if (openssl_strerror_r(EPERM - 1 + i, cur, sizeof(strerror_pool) - cnt)) {
           size_t l = strlen(cur);

           str->string = cur;

@@ -235,6 +238,7 @@ static void build_SYS_str_reasons(void)
}
if (str->string == NULL)
str->string = "unknown";
+#endif
}
 /*

diff --git a/include/openssl/err.h b/include/openssl/err.h
index aa8ffa9765f8d..d224b782fe5cf 100644
--- a/include/openssl/err.h
+++ b/include/openssl/err.h
@@ -163,13 +163,18 @@ struct err_state_st {
define X509err(f, r) ERR_raise_data(ERR_LIB_X509, (r), NULL)
endif
-# define ERR_PACK(l,f,r) ( \

   (((unsigned int)(l) & 0x0FF) << 24L) | \

   (((unsigned int)(f) & 0xFFF) << 12L) | \

   (((unsigned int)(r) & 0xFFF)       ) )

+# define ERR_PACK(l,f,r)                                \

    (((unsigned int)(l) & 0x0FF)      << 24L) | \

    (((unsigned int)(f) & 0xFFF)      << 12L) | \

    (((unsigned int)(r) & 0x40000000) >> 19L) | \

    (((unsigned int)(r) & 0x7FF)            )

define ERR_GET_LIB(l)          (int)(((l) >> 24L) & 0x0FFL)
define ERR_GET_FUNC(l)         (int)(((l) >> 12L) & 0xFFFL)
-# define ERR_GET_REASON(l)       (int)( (l)         & 0xFFFL)
+# define ERR_GET_REASON(l) ( \

   (((unsigned int)(l) & 0x800) << 19L) | \

   (((unsigned int)(l) & 0x7FF)            ))

define ERR_FATAL_ERROR(l)      (int)( (l)         & ERR_R_FATAL)
ifndef OPENSSL_NO_DEPRECATED_3_0
diff --git a/test/errtest.c b/test/errtest.c
index f7a6f85470cd3..f020f92c5c8fe 100644
--- a/test/errtest.c
+++ b/test/errtest.c
@@ -42,7 +42,7 @@ static int test_print_error_format(void)
if (!TEST_ptr(bio = BIO_new(BIO_s_mem())))
return 0;

ERR_PUT_error(ERR_LIB_SYS, 0, 1, "errtest.c", 30);

ERR_PUT_error(ERR_LIB_SYS, 0, EPERM, "errtest.c", 30);
ERR_print_errors(bio);
if (!TEST_int_gt(len = BIO_get_mem_data(bio, &out), 0))

Any comments on that? Or should I put this into a pull for comments?
Sebastian

@sebastianas, you're lagging behind I'm afraid.  Have a look at what's happening in #12343


[AutoPR billing/resource-manager] Remove billingProfile from PartnerLed transfer APIs.

Created to sync https://github.com/Azure/azure-rest-api-specs/pull/6888

(message created by the CI based on PR content)


Bigint backend pt 3: Move ec point conversion to TS

Incremental step on top of #139
This PR part of a multi-PR series to move JSOO / Wasm conversion methods from bindings.js to TS:

[x] Conversion of wires, gates, and different kinds of field vector
[x] Conversion of curve points and polynomial commitments
[ ] Conversion of verification key
[ ] Conversion of oracles
[ ] Conversion of proof
[ ] Cleanup conversion modules

Discussed offline with Gregor: I would be in favor or always having the corresponding o1js PR to have the tests and be sure the code is correct. Atm, we only use a linter in the CI. As the code is moved to pure TS, tests could be added. That would be important IMO.
I reviewed by looking at the correspondances between the JS and TS code.
Gregor told me the final PR tests everything. I can accept the PR as it is, modulo my comments above.

Discussed offline with Gregor: I would be in favor or always having the corresponding o1js PR to have the tests and be sure the code is correct. Atm, we only use a linter in the CI. As the code is moved to pure TS, tests could be added. That would be important IMO. I reviewed by looking at the correspondances between the JS and TS code. Gregor told me the final PR tests everything. I can accept the PR as it is, modulo my comments above.

I would open a follow issue for the tests in TS

Is it possible to add in the CI the code typechecks?

Discussed offline with Gregor: I would be in favor or always having the corresponding o1js PR to have the tests and be sure the code is correct

Running CI with o1js main + the latest version of this PR here: https://github.com/o1-labs/o1js/pull/1117

Is it possible to add in the CI the code typechecks?

that would be very nice indeed, will think about it. probably just means an extra tsconfig file here and make CI install TS


Avoid error when enumerating of undefined value in AVM1.

Converts the error in http://swf.codeazur.com.br/archive/dd/43/69/e2/dd4369e279f78efb970c33253526995445da581e0042d12ebf875e9da2b8935b.swf to a warning.

r? @tschneidereit
/botio test

Yup, makes sense. r=me.

Reviewed 1 of 1 files at r1.
Review status: :shipit: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from the review on Reviewable.io


Realme 7i, Android 11 face detector not working even  sample app

Realme 7i, Android 11 Face detection is not working could you please help to resolve the issue.

@ChanakaWeerasinghe: we do not have all the devices to test using all the available Android OS variants and manufacturer, and we cannot use the emulator in all the cases. Please provide more detail of your issue so maybe another developer with the same hardware can step in and help. Also, we released 0.8.0 yesterday, please give it a try.


server: check regions in background goroutine.

cherry pick #963. Also the ScanRegions utils are cloned.

LGTM

/run-all-tests tidb=release-1.0 tikv=release-1.0 tidb-test=release-1.0

CI failure is not related. I'm going to merge it.


Remove ripple effect

Is there a way to remove ripple affect? The action under onClickListener can only be triggered after ripple effect finished. So, I would like to remove the animation. thanks.

Try adding the following in your XML layout:
app:animate="false"

thanks, I'll try it out.


How to run this SwiftUI project with Swift development toolchain?

Hi Peter,
great blog post and project!
I am able to compile but I am not able to run this project. I am using Xcode 12.4 and toolchain swift-DEVELOPMENT-SNAPSHOT-2021-03-07-a.xctoolchain.

dyld: Symbol not found: _$sSo12NSFileHandleC10FoundationE9readToEndAC4DataVSgyKF
Referenced from: /System/Library/Frameworks/SwiftUI.framework/SwiftUI
Expected in: /Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2021-03-07-a.xctoolchain/usr/lib/swift/iphonesimulator/libswiftFoundation.dylib
in /System/Library/Frameworks/SwiftUI.framework/SwiftUI

Apparently I am not alone when it comes to running the Swift development toolchain in SwiftUI projects: https://forums.swift.org/t/swift-development-toolchain-crashes-in-swiftui-projects/43592
I was wondering how you were able to overcome this issue?

Hi Marco,
just delete the offending file (yeah, I know - it sounds counterintuitive...):
rm /Library/Developer/Toolchains/swift-DEVELOPMENT-SNAPSHOT-2021-03-05-a.xctoolchain/usr/lib/swift/iphonesimulator/libswiftFoundation.dylib
I filed a radar: FB9035762
Cheers,
Peter

Thanks Peter, that helped me! Awesome :)


Feat: Add user preferences to allow custom indent/space

Addresses issue #82 to allow users to set their preferred space/indent size.
What does this pull request do?
Adds a PreferenceProvider (React Context) that can be reused to store and persist user preferences throughout the application. It currently only supports the indent field for the space number. The indent can be set using a new field added beside the theme mode toggle button.

Why Preference instead of a specific Indent/Space Context?
To introduce a general interface that can be extended for updates that would benefit from a persisted global user preferences object. The PreferenceProvider is designed to adapt automatically whenever a new attribute is added on the Preferences interface.
For now, only a small non-intrusive input field was added in the bottom of the main interface to allow modification of the spacing. Future additions of user preferences can benefit from a dedicated settings/preference view where this field can be migrated along with new fields.
The input field definitely requires additional tweaks to match the look-and-feel of the website but it serves it current purpose well enough.

Great job @ShaneMaglangit 🤩


Making debugging easier with stetho

Added stetho debugging bridge which lets chrome debugging options available for android app.

We could make this only load & compile in debug mode!
https://stackoverflow.com/questions/30172308/include-stetho-only-in-the-debug-build-variant#31483962