Windows feature generate_pdb_file should be applicable to opt builds

Description of the problem / feature request:
We would like to generate .pdb files for release builds of Envoy for Windows for use debugging potential production issues. This works well with Bazel building locally as all intermediate/compiler/linker generated files live on the host. We use opt mode, the MSVC cl /Z7 option, and linker options /DEBUG:FULL. However, building Envoy with RBE in CI, the expected .pdb file output is not accessible. It is not an explicit output of the cc_binary rule on Windows and is not downloaded to the host running the build (or presumably any rules that were to depend on the binary target, not sure if a genrule workaround could work to grab the relevant file). This is in contrast to Linux where cc_binary rules create an implicit <target>.dwp target that can be used to access debug info. Using --remote_download_outputs=all (the default) or --features=generate_pdb_file does not work as the feature is only enabled for dbg and fastbuild compilation modes.
It would be great if the restriction of the feature to dbg and fastbuild modes was relaxed
Feature requests: what underlying problem are you trying to solve with this feature?
See above, generate .pdb files as when building Envoy with RBE in opt compilation mode.
Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.
Build a cc_binary rule in opt compilation mode and link with MSVC link /DEBUG options to generate a .pdb file in RBE on Windows. See that no .pdb file is downloaded even when using --features=generate_pdb_file or --remote_download_outputs=all.
What operating system are you running Bazel on?
Windows (2019)
What's the output of bazel info release?
release 3.6.0
What version of rules_cc do you use? Can you paste the workspace rule used to fetch rules_cc? What other relevant dependencies does your project have?
https://github.com/envoyproxy/envoy/blob/d382fa60412b1bf3c8b1d82883119c15216d69fc/bazel/repository_locations.bzl#L535-L546
What Bazel options do you use to trigger the issue? What C++ toolchain do you use?
C++ toolchain: https://github.com/envoyproxy/envoy-build-tools/tree/master/toolchains/configs/windows/msvc-cl/bazel_3.6.0 (generated via bazel-toolchains)
Have you found anything relevant by searching the web?
Not as of yet
Any other information, logs, or outputs that you want to share?
Not as of yet

While this is describing what should be emitted as a result of linking, there is a parallel PR concerned with emitting the .pdb results corresponding to each .obj file, and then compressed into a library-scope .pdb using fission at lib construction time.
It would be nice if the underlying bazel rules_cc mechanics follow similar toggles. https://github.com/bazelbuild/rules_cc/pull/78

The pdb file is generated by the link action but not in the default output group of cc_binary (therefore not retrieved in RBE build). You can access it by specifying the output_group in the following ways:

In command line
bazel build //:hello-world --output_groups=pdb_file (--output_groups=pdf_file,default if you want both the binary and the pdb file)
In BUILD file

filegroup(
    name = "my_pdb_file",
    srcs = ["//:hello-world"],
    output_group = "pdf_file",
)

Can you try if this fixes the issue with RBE?
Related code: https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java;l=696?q="pdb_file"&ss=bazel

Using the option 2. above, we were successful at envoy in adding an envoy-static-exe target that captures both the default and the pdf_file group. Thank you for the guidance! Documenting this for future reference could be helpful?

Documenting this for future reference could be helpful?

Yes, we should definitely improving the documentation on this!

Hmmm, I tested locally with a hello-world cc_binary, it should be working. Can you also confirm if it works for you in a local build?
pcloudy@PCLOUDY2-W MSYS ~/workspace/my_tests/bazel
$ git diff
diff --git a/examples/cpp/BUILD b/examples/cpp/BUILD
index f450f67242..ad7e7cf70c 100644
--- a/examples/cpp/BUILD
+++ b/examples/cpp/BUILD
@@ -14,6 +14,20 @@ cc_binary(
     deps = [":hello-lib"],
 )

+filegroup(
+    name = "hello-world-pdb",
+    srcs = [":hello-world"],
+    output_group = "pdb_file",
+)
+
+filegroup(
+    name = "hello-world-all",
+    srcs = [
+       ":hello-world",
+       ":hello-world-pdb",
+    ],
+)
+
 cc_test(
     name = "hello-success_test",
     srcs = ["hello-world.cc"],

pcloudy@PCLOUDY2-W MSYS ~/workspace/my_tests/bazel
$ bazel build examples/cpp:hello-world-all
...
INFO: Analyzed target //examples/cpp:hello-world-all (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //examples/cpp:hello-world-all up-to-date:
  bazel-bin/examples/cpp/hello-world.exe
  bazel-bin/examples/cpp/hello-world.pdb
INFO: Elapsed time: 0.273s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

So the first solution may not work under distributed RBE either?

It's possible the RBE code doesn't respect the output groups, I'm not sure if this is a bug.
/cc @coeuvre Do you happen to know what files Bazel choose to retrieve from RBE?

Confirming, local build of all varieties we've discussed, as well as our existing logic, all emit the pdb file to bazel-out//x64_windows-opt/bin/source/exe/envoy-static.pdb - but that isn't helping in our RBE CI pipeline where unrecognized nuggets are discarded. What we must avoid is any situation where the .pdb is triggered and invokes the linker, and the .exe is triggered and invokes the linker a second time. MSVC is basically incapable of performing a deterministic build (clang-cl is a little closer to that goal.)

This commit sort of makes me want to cry, @meteorcloudy - I don't understand the delta, why the previous case fails in a GCP RBE environment, and the second wins, but here we are, there is a workaround, not quite one I expected;
https://github.com/envoyproxy/envoy/pull/16043/commits/e1f7b791d4cd1afca68f8153941a24a8983f692f
Any thoughts?

@wrowe I set up a remote build and did some debugging on this.
It turned out what I said before was not entirely true. The pdb file is always an output of the linking action in dbg and fastbuild modes. No matter what output groups you specific, it should always be retrieved from remote as part of the linking action output.
The reason it didn't work for you is because you have optimized mode (-c opt) enabled for your remote build, in that case, the pdb file is not generated at all and is of course not an output of the linking action.
I believe if you simply remove the -c opt option the pdb file will be retrieved.

The pdb file is always an output of the linking action in dbg and fastbuild modes. No matter what output groups you specific, as long as it triggers the linking action it should always be retrieved from remote as part of the linking action outputs.

This has long been our observation, agreed.

The reason it didn't work for you is because you have optimized mode (-c opt) enabled for your remote build, in that case, the pdb file is not generated at all

This is not correct at all. As a default behavior no pdb is generated, but in a release binary, all vendors capture .pdb for windows just as they build linux/bsd release binaries with -g, and then either strip and retain the un-stripped binary, or use objcopy to split symbolic information into a distinct .debug file, pointing the release executable at that parallel .debug symbol file (which is exactly what a .pdb file on windows is).
In this case envoy and most projects add /Z7 symbols to each .obj file although these are still compiled optimized, and we compile release as "/debug /opt:ref /opt:icf" for envoy this logic lives here and here for compilation and linking;
https://github.com/envoyproxy/envoy/blob/ca4ca983195f337a840618b45b9b33cda82c1224/bazel/envoy_internal.bzl#L50
https://github.com/envoyproxy/envoy/blob/ca4ca983195f337a840618b45b9b33cda82c1224/bazel/envoy_binary.bzl#L63
[There is a related ask, which would also need to support .pdb output from /Zi rather than /Z7 for each .obj compilation even in -c opt mode, see https://github.com/bazelbuild/rules_cc/pull/78]

and is of course not an output of the linking action.

Yes, this issue describes the fact that a pdb file itself needs to be captured in the linking action even in -c opt build.

I believe if you simply remove the -c opt option the pdb file will be retrieved.

Yes, that might be the case, but that is not our bazel use case. Our use case is for bazel to generate release binaries optimized for the end user, capturing the un-stripped symbolic information for the released binary for forensic crash dump analysis. No responsible vendor ships an executable without this; in proprietary source this is either withheld for internal use only, perhaps further stripped for public consumption with only public symbols, while in open source there is no reason to withhold or conceal this data.  It's not part of the "binary distribution" but typically a distinct "-dbg", "-debug", "-debuginfo" corresponding package.
Back to our ask, we are capturing the un-stripped linux executable we distribute from envoy opt builds. The same functionality is required on windows. Any help you can offer would be greatly appreciated.
There is the totally distinct question of whether this should be the default for all -c opt output. A stripped linux binary is equivalent to "just the .exe" on Windows. And we've elected not to emit symbolic info for the hundreds and hundreds of test .exe files, since they are not production code shipping to the public, their inputs are predictable, and they can be rebuilt with symbolic info on an individual basis for deeper troubleshooting of a test case failure.

@wrowe Thanks for the explanation! I think now I understand your use case.
IIUC, you want to build with opt mode but still generate the pdb file by passing flags like /DEBUG:FULL by yourself. But the problem is that Bazel only recognizes the pdb file as an output of the linking action when the feature generate_pdb_file is enabled, see here.
However, currently generate_pdb_file is only triggered in debug and fastbuild modes, and it is also restricted to be used in those two modes, check the feature definition:
        generate_pdb_file_feature = feature(
            name = "generate_pdb_file",
            requires = [
                feature_set(features = ["dbg"]),
                feature_set(features = ["fastbuild"]),
            ],
        )

Fortunately, I see you use your own generated msvc toolchain in the remote build:
https://github.com/envoyproxy/envoy-build-tools/blob/main/toolchains/configs/windows/msvc-cl
Then how about this solution:

Remove those lines that restricts the generated_pdb_file feature to dbg or fastbuild
Enable the generated_pdb_file feature. You can do it by:

Either in command line with --features=generated_pdb_file, this will enable the feature for all cc_binaries.
Or add features = ["generate_pdb_file"] to the cc_binary you want the pdb file generated, maybe this fits your use case better?

I manually tested this solution should work as long as you ensure /DEBUG flag is passed, otherwise Bazel will complain the pdb file is not generated.

Let's make it so, drop the restrictions, as quickly as 4.1.0-rc5 if at all possible, and leave all defaults. Note I find the same logic to be corrected in the rules_cc repository, cc/private/toolchain/windows_cc_toolchain_config.bzl
This makes it possible for us to avoid deploying a forked bazel build to our GCP RBE environment, and proceed with testing various combinations of opt with pdb and fastbuild omitting pdb.
Thanks for your continued research @meteorcloudy

I have limited flexibility to test these changes in our GCP RBE environment, it would be helpful if you could do so. Once in bazel, testing the various combinations in our toolchain should be straightforward here.

I have verified the solution in a GCP RBE environment. I'm sending a change to both Bazel and rules_cc.
I don't think the fix will get into a Bazel release very soon since it's likely we won't have a rc5 for 4.1.0.
Shouldn't you have full control over https://github.com/envoyproxy/envoy-build-tools? You'll need to update this even when you have the fix in Bazel, right?

@meteorcloudy are you suggest we put in custom bazel build into build docker image? This will complicate things quite a bit.
When is the next bazel release?

Correct, we regenerate envoy-build-tools on every bazel or dependency bump that we want to capture.
However, we drive bazel from bazelisk, so it has to be something, even by-tag, I can have bazelisk grab. I didn't see any interesting flags to have bazelisk point to a a flavor of bazel from a non-bazelbuild repo.

@meteorcloudy are you suggesting we put in custom bazel build into Envoy's build docker image? This will complicate things quite a bit.

Nope, I thought you are using the toolchains checked in under https://github.com/envoyproxy/envoy-build-tools/tree/main/toolchains/configs/windows/msvc-cl/bazel_4.0.0/cc, so you can just fix the toolchain here. But looks like you're actually using the generated toolchain directly, then my suggestion probably won't work.

When is the next bazel release?

4.1.0 will be released soon, after that we'll start preparing for 4.2.0, we don't have a fixed timeline yet.
See https://github.com/bazelbuild/bazel/issues/13099#issuecomment-831358571

As observed at https://github.com/envoyproxy/envoy/pull/16043 this may be largely resolved although the solution could be simpler.

Thanks for the notice, seems this issues can be closed.


Port to python3

Mostly this was done by running futurize (pip install future).

This looks pretty good. Let me know when you can get Travis to pass.

The rdkit release 2014.09.2 (py34) installed from https://binstar.org/rdkit/rdkit is just broken...
This script segfaults:
$ cat test.py
from rdkit import Chem

mol = Chem.MolFromSmiles('CC(=O)OC1=CC=CC=C1C(=O)O')
with open('out', 'wb') as f:
    w = Chem.SDWriter(f)
    w.write(mol)

It also seffaults if you open in mode='w'.


MAINT: misc patches

Summary by Sourcery
Enhance test configurations by adding a new Nox session and updating test cases to specify 'moltype' for sequence data. Improve test assertions for floating-point comparisons.
Enhancements:

Add a new Nox session 'test_new_type' to run tests with a specific environment variable set.

Tests:

Update test cases to specify 'moltype' as 'dna' for sequence data, ensuring consistency across tests.
Replace direct equality assertions with 'assert_allclose' for floating-point comparisons in test cases.

Pull Request Test Coverage Report for Build<PHONE_NUMBER>0
Details

0 of 0   changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 92.222%

Totals

Change from base Build<PHONE_NUMBER>5:
-0.02%

Covered Lines:
205313

Relevant Lines:
222630

💛  - Coveralls


When parents in separate vcf, indicate on de-novos why allele counts for parents are not available

Good find, but this seems very minor, we should encourage people to just use a multi-sample VCF


Deprecation warning leads to pod eviction

Bug Description
Some nginx pods with ephermal-storage limit of 100 Mi are evicted 2-3 times a day because of a deprecation warning in the sidecar-proxy:
2021-10-07T09:26:08.836929Z warning envoy misc Deprecated field: type envoy.config.listener.v3.Listener Using deprecated option 'envoy.config.listener.v3.Listener.deprecated_v1' from file listener.proto. This configuration will be removed from Envoy soon. Please see https://www.envoyproxy.io/docs/envoy/latest/version_history/version_history for details. If continued use of this field is absolutely necessary, see https://www.envoyproxy.io/docs/envoy/latest/configuration/operations/runtime#using-runtime-overrides-for-deprecated-features for how to apply a temporary and highly discouraged override.

This happens every few minutes and logs 141 messages each time. The nginx itself does not log anything besides a few lines on startup.
Regardless of this specific deprecation message the sidecar should log such a message once and not pollute ephermal storage of every pod that has a sidecar.
Version
$ istioctl version
client version: 1.10.4
control plane version: 1.10.4
data plane version: 1.10.4 (132 proxies)
$ kubectl version --short
Client Version: v1.20.11
Server Version: v1.20.11

Additional Information
No response

But it is not clear, why this is evicting the pod?

We intend to warm for EnvoyFilter deprecated filter use in the control
plane (with webhook and analyze). It's plausible it's not 100% accurate but
I think we use the same source of data envoy uses so seems pretty solid.
I don't think anything else is logged at misc error level last I checked
On Fri, Oct 8, 2021, 2:17 AM Christoph-Raab @.***>
wrote:

Reopened #35504 https://github.com/istio/istio/issues/35504.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/istio/istio/issues/35504#event-5433476074, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAEYGXNVZD7AJFELFVFZP33UF2ZLHANCNFSM5FRDS7ZA
.
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&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

Problem would be, that we administrate istio, but the dev teams that deploy the applications have no access to the control plane. When a dev team would deploy an EnvoyFilter with a deprecated feature they don't see the deprecation message this way.

Ok, thanks. We're using the default now and will figure out how to help our dev teams to handle deprecations.


ESP32-S3 deep sleep

This PR adapts #574 for the ESP32-S3. The PR also includes some minor changes to the ESP32 implementation based on esp-idf.
cc #375

esp-idf's rtc_init, which is the basis of base_settings seems to have a bunch of code that should run on startup (LDO bias settings for example). I am unsure how to approach those. Currently, everything is part of base_settings but I'm fairly certain that applying a low LDO bias for sleep could actually not let the MCU wake up properly, in a stable manner.
This probably means we'll need to tie some of the sleep infra into clock initialization, for example.

I have hijacked RtcSleepConfig a bit to actually implement MCU-specific sleep logic, partitioned somewhat similarly to esp-idf: by including a start_sleep and a finish_sleep function. I've moved the register access code from the parent module into RtcSleepConfig, hopefully this will help more in terms of MCU support than it hurts by requiring duplication.

works for me

oh wow I didn't actually test this because I need to implement GPIO wakeup for my stuff, but that's great to hear!


add terminating threshold message to object status

What this PR does / why we need it:
Which issue(s) this PR fixes

Fixes #1408

This is taking the work that @lenriquez did and removing the preferences specific parts of it. We've extracted that out in to a broader conversation https://github.com/vmware-tanzu/octant/discussions/2694
The outcome of that conversation will be the TODO comment that was added to the code.


Twitter isn't working

I was surprised today to see a twitter user's pronouns. However when going to the site and trying to link my account, i was encountered with this message
oh, and, when trying to log in to twitch (the only available website back when i got the extension) it said my account didn't exist? I just made a new one, but thought that was odd since I've been using it forever.

Hey there, I think there is no issues here but some misunderstandings. Let me help to clarify:

Regarding the message, PronounDB is a "third-party app", in the sense that it is not managed or operated by Twitter. This message should show up at the bottom of the page for everyone who uses a "Login via Twitter" feature. If the login is simply not working, try on a desktop; Twitter since the weird fat guy has been getting more and more broken, it wouldn't surprise me this is simply broken on Twitter's end.

PronounDB have had support for 4 platforms the day of its release. You might have encountered the other "Twitch chat pronouns" extension, which is way more popular but exclusively works on Twitch - this is probably where you set your pronouns initially.

This was on a desktop. I guess it's just broken on their end.
And you were right! It was the other add-on. Odd, because it's been working with this extension, even though I never set my own pronouns until now with it.

The PronounDB extension doesn't fetch pronouns from other sources than pronoundb.org, it was probably some other extension (e.g. FFZ which IIRC has integration with the other extension), or the other extension itself.
As for the Twitter login, I checked again and it works fine for me. I double checked with my alt account to confirm it's not working "just for me" due to me being the owner of the Twitter 3rd party app (and you can clearly see how broken this looks, especially the footnote "Made by . Read undefined's (...)" 🤣


10.0.19045.2130 Not supported?

Nay updates on support for 10.0.19045.2130 ? Thanks.
Tried Sync, update and AutoSupport. Didn't work.

https://github.com/anhkgg/SuperRDP/releases/tag/v<IP_ADDRESS>


Added site to blacklist

This domain seems to redirect to the correct myetherwallet.com - confirmed by Kevin to not be an official myetherwallet.com domain. Blacklisting.
Thanks for the PR moverperfect.


Support validation by schema

It would be great if one could define a validation schema that is passed to a function that validates one or more objects.
i.e: validator.validate(schema, ...objectsToValidate)
A validation schema would look like this:
const schema = {
  username: {
    isAlphanumeric: true, // checks username is alphanumeric with default locale and default error message
    isLength: {
      options: [{min: 3, max: 33}],   // options passed to isLength
      message: 'Username should be 3 to 33 characters long'   // display this message if isLength returns false
    },
  }
  email: {
    optional: true,    // if email isEmpty, just ignore it == dont return false
    isEmail: {
      options: [{ allow_display_name: true}]
    }
  }
}

=====================
express-validator uses this. I was hoping it could be directly integrated into this library.

I've previously said no to such requests, saying that it was outside the scope of the library, however I'm warming up to the idea. It seems like people end up implementing their own scheme which is error prone.

That's the exact reason why I suggested it.I thought it would be better if it was supported in this library, which a lot of other libraries depend on.

This would take put this library one giant step closer to being easily implemented both client and server side, too, as validation schemas could be easily passed and applied.

As we desperately needed something like this half a year ago, I did exactly this plus added additional features in the json-inspecotor library.


Use hive.domain-compaction-threshold for dynamic filters

Benchmarks hive-lazy-df-compact 32 vs 100.pdf
Benchmarked with 5 r5.8xlarge worker nodes with 1000 scale orc format files on S3.


[ownership] Re-architect ownership pipeline so that we do not strip o…

…ssa from external functions.
Specifically, previously the plan was as follows:
-Onone:

* Run Diagnostic Passes.
* Serialize.
* Lower ownership.
* IRGen

-O:

* Run Diagnostic Passes
* Lower Ownership from all but transparent functions and register a
  deserialization notification to ensure any further non-transparent functions
  we linked in were stripped as well.
* Run Performance Passes Before Serialization.
* Serialize.
* Lower ownership from all functions and register a notification to ensure all
  further functions that are linked in are lowered on link.
* Run rest of performance passes.
* IRGen.

There is a problem with this setup though: what happens if we lower ownership
from a function, dce it, and then reload the function. Our declaration and new
definition will be inconsistent! After some thought, I realized that it really
did not make sense to just hide such inconsistency since the model in SIL is
that once ownership is removed, it can not be added back.
This commit works around this problem by creating 3 different styles of
ownership model eliminator, each varying what it will eliminate. Specifically:

Eliminate all + everything deserialized.
Eliminate non-external. Leave deserialized things alone.
Eliminate non-transparent, non-external only. Leave deserialized things alone.

We change the scheme such that we only lower external functions once we know we
will never DCE/relink any functions.
-Onone:

* Run Diagnostic Passes.
* Serialize.
* (1).
* IRGen

-O:

* Run Diagnostic Passes
* (2).
* Run Performance Passes Before Serialization.
* Serialize.
* (3).
* Run rest of performance passes.
* (1).
* IRGen.

When the option is disabled, we always lower ownership for local functions right
before mandatory inlining in the diagnostic passes, in the same place we used to
run an instance of (1). This is safe to do since if all functions before they
are serialized have ownership lowered, all serialized SIL should only have
lowered ownership implying we can never deserialize a function with ownership
not lowered, the previous state of the world. ☐.

@swift-ci smoke test

Backing up even farther, I thought the purpose of deserialization callbacks was to ensure that functions are deserialized into the current stage in the proper state.

I'm going to do this a different way.

When I say a different way, I mean that I am solving this problem in a different way (just accepting the state from the serialized definition instead of the declaration).


Tf 13 patch

Hi Phil @philschmid ,
Charles from AWS here. Big fan of your work.
Based on this thread:
https://github.com/hashicorp/terraform/issues/31692
It looks like TF 1.4 will fix the experiments = [module_variable_optional_attrs] that makes this module currently incompatible with 1.3.
In the meantime can you update the code so that it works with 1.3?
Or at the very least keep a branch tagged that does?
Warm regards,
Charles

@philschmid   Thank you. Can we get this update on TF registry?
Looks like it's still 0.6: https://registry.terraform.io/modules/philschmid/sagemaker-huggingface/aws/latest

Thanks for letting me know! It seems they changed what versions they respect. It is now available: https://registry.terraform.io/modules/philschmid/sagemaker-huggingface/aws/0.7.0


Make runner non-nullable as it always is.

Sorry for jumping in with autosubmit. I'm using this PR to test my Cocoon changes.


[MINOR] Reduce unnecessary code paths

Reduce unnecessary code paths

Change Logs
Describe context and summary for this change. Highlight if any code was copied.
Reduce unnecessary code paths during judgment expression
Impact
Describe any public API or user-facing feature change or any performance impact.
none
Risk level (write none, low medium or high below)
If medium or high, explain what verification was done to mitigate the risks.
none
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".

The config description must be updated if new configs are added or the default value of the configs are changed
Any new feature or user-facing change requires updating the Hudi website. Please create a Jira ticket, attach the
ticket number here and follow the instruction to make
changes to the website.

Contributor's checklist

[x] Read through contributor's guide
[x] Change Logs and Impact were stated clearly
[x] Adequate tests were added if applicable
[x] CI passed

@hudi-bot run azure


Fabric runs createsuperuser on every deploy

The current fabfile tries to create a superuser every time it deploys. This is inconvenient if you're just deploying a new release. Can this be made optional?

#493 should fix this (albeit simply by removing the fabfile entirely ;)

Hi @GregoryAveryWeir, as David indicated, we are in the process of deprecating the Fabfiles because we think we now have much better deployment mechanisms. Do you think these cover all your use cases? Please weigh-in in #493 :)

The Fabfile is now removed ;)


Yarn PnP

https://yarnpkg.com/features/pnp
Please +1 and link your public repository if you're using yarn pnp.

https://github.com/sveltejs/kit/issues/11578#issuecomment-1884974247

Yarn PnP is broken, and we don't support it — if it ever worked, it was by accident. It is a frequent source of misery for OSS maintainers, and we strongly recommend using nodeLinker: 'node-modules' or (better) just using npm/pnpm.

Personally, I don't think this resolver should support Yarn PnP

Please +1 and link your public repository if you're using yarn pnp.

Please link your project with yarn pnp setup here for the possibility of reopening this issue.

Personally, I don't think this resolver should support Yarn PnP

We can provide yarn pnp support with a feature flag.
Supporting yarn pnp is a big maintenance burden for us, but I'm happy to add another maintainer who is willing to implement, maintain and triage issues related with yarn pnp, since triaging a resolution error usually takes an hour at least.

Probably the biggest and most useful reference for Yarn monorepo would be Yarn itself: https://github.com/yarnpkg/berry
AFAIK Yarn (berry) is still used well on corporate projects due to its highly configurable nature but hasn't seen wide usage outside of it. I'd love to help this issue but I think I should look more deep into oxc before promising anything ;)

Note that I implemented a PnP resolver in Rust as a package: https://github.com/arcanis/pnp-rust; I had a prototype integrating it with Parcel and the work was quite minimal.
So if you're interested, I'm more than happy to help integrate the pnp-rs crate with oxc, and move it to the official yarnpkg org so it keeps being maintained in the future.

If I get a bug report that looks similar to the 120+ open issues in yarn relating to pnp, how much time should I spend on triaging them? https://github.com/search?q=repo%3Ayarnpkg%2Fberry+pnp++&type=issues&state=open

The repo issues aren't the right metric - rather, the interesting one is Esbuild, which has natively supported PnP for a while. There's been ~60 issues in ~3 years, most of them closed. In your case it'd be lower if you leveraged the pnp crate, as we'd take ownership of many of these issues.

Note that I implemented a PnP resolver in Rust as a package: https://github.com/arcanis/pnp-rust; I had a prototype integrating it with Parcel and the work was quite minimal.
So if you're interested, I'm more than happy to help integrate the pnp-rs crate with oxc, and move it to the official yarnpkg org so it keeps being maintained in the future.

If I get a bug report that looks similar to the 120+ open issues in yarn relating to pnp, how much time should I spend on triaging them? https://github.com/search?q=repo%3Ayarnpkg%2Fberry+pnp++&type=issues&state=open

The repo issues aren't the right metric - rather, the interesting one is Esbuild, which has natively supported PnP for a while. There's been ~60 issues in ~3 years, most of them closed. In your case it'd be lower if you leveraged the pnp-rs crate, as we'd take ownership of many of these issues.

Thank you @arcanis. The pnp-rs crate + redirecting issues is a good solution.
Let's collaborate and integrate.

The Rust side is complete.

Now I need a collaborator to help me test this in their tool.

I'm willing to test, but I'm not a contributor. I am looking to set the resolution for @swc-node/register/oxc-resolver@npm:??? to point to a version that includes this fix.

This is now behind a feature flag https://github.com/oxc-project/oxc-resolver/blob/509dc008544ab288e352cbea9c0c107fb9ee9236/Cargo.toml#L105-L106
Feel free to create an issue for integration.


[question] xrdp and native smart card support

When is the ETA for native smart card support? date and version please

Which branch is the smart smart card support being worked in? dev?

I have a smartcard branch that I need to work on.  I don't really want to do any more work on the replace libpcsc method and instead work on the pcscd emulation.
https://github.com/jsorg71/xrdp/commits/pcsc


Pull control logic out of Multiway into a separate system

This is a first draft at pulling the control logic out of Multiway into a system.  It's not fully tested, and a lot of the names are temporary, but since there have been a couple of multiway related suggestions lately I thought I should present this alternative.  @matthijsgroen  @Wanderer101

The control system lets you define dpads using the same syntax that Multiway uses, and emits events with a directional vector
A new "Dpad" component allows you to tie callbacks to specific dpad events.  Multiway uses this concept to update the objects speed when necessary.
The new system has options for normalized/clamped input (#1047) and axis-only movement (#1046, aka only listen to one direction button at a time)
Behind the scenes, instead of listening for keyup/keydown events, the control system checks which keys are down each frame, and update the direction based on that info.  That makes it easier to maintain a consistent state.
To remove any ambiguity in when motion/control events happen, I tried introducing a new top level event (called "EnterFrameInput" for now) that fires before "EnterFrame".  The idea here would be to first update any state based on user input, and then actually process the game frame.  I'm not committed to this idea, but it does seem useful.

TODO

[x] Fix tests

Great to see this component get an overhaul! I'll try to learn something as well by reading up on the changes. Thanks for addressing this issue.

I updated this with a few bug fixes, so the new features actually work now.  :)
When you disable multiple directions, it will always stick with the initial direction when you press additional keys.  I think that's what most people would want, but perhaps I'm wrong?

Not to be difficult but could it offer both perhaps set by the options object? I haven't tried the controls when bound to EnterFrame (or EnterFrameInput) yet, but at least my use case needs the new button press to override the previous held one.

Not to be difficult but could it offer both, perhaps set by the options object? I haven't tried the controls when bound to EnterFrame (or EnterFrameInput) yet, but at least my use case needs the new button press to override the previously held one.

Refactored it to allow this strategy -- the option is now multipleDirectionBehavior and takes "all", "first", or "last" as values.  (It now tracks when an "input group" is first active -- so it doesn't care about the individual key you're pressing.)

Thanks a bunch! I can't wait to try out this new functionality.

I hope to test run this in my game soon, I hope to have time for this tomorrow!

I'm going to go ahead and merge this, since it seems to work in my basic tests.  But the internals of the change will remain undocumented for a little bit -- I want to see how easy it is to adapt other types of inputs to use this control pattern, and what changes might be necessary to the underlying system.

@starwed  I'm trying to grasp the logic of the new split-up components, but some parts are not really clear to me.

There is a 'ControlSystem' here, you define a virtual gamepad
Next link an event of this virtual gamepad to a handler (directionalInput)

As far as I can see, only mouse and keyboard are supported for this virtual gamepad (not a real gamepad)
I like the idea of having controls just emit events, and the entity responds to those events (regardless if it came from keyboard or mouse). But it doesn't seem open for extension to support a real HTML5 gamepad.
So, what is the philosophy? Extend the crafty control system to support real gamepads, or duplicate the virtual gamepad mechanic to emit the same triggers?
Now the gamepad has a special 'GamepadMultiway' component, but I guess this should now work with a normal multiway component when a proper virtual gamepad could be constructed for the real gamepad?
I'm a bit confused what the correct upgrade strategy would be to support https://github.com/svenjacobs/crafty-gamepad/blob/master/src/crafty.gamepad.js
within Crafty 0.80

The main goal here was to separate the existing Crafty controls.  The new pattern is to essentially create three layers:

Input code: listens for HTML events and emits Crafty input events
Control code: listens for Crafty input events and maps them to control events.
Game state code: listens for control events and produces state change in the game; controllable entities are the obvious example

Previously Crafty jumbled up the second two.  The code changes in 0.8 mostly focus on separating those out, but don't yet provide a lot of support for adding new types of input->control adapters -- it's hardcoded to handle the types that we already handled.   The idea is to iterate on this with real world use cases such as gamepad input.
The next step would probably be to more formally define the contracts that new adapters need to provide.  There's a vague nod in that direction with the two "ToggleInput" objects -- we have both MouseButtonToggleInput and KeyboardToggleInput classes, which implement a simple isDown() method.  Right now everything is hard coded, but in the future we could allow you to register additional input types and make the define functions more generic.  (In addition to gamepads, another obvious use case would be to move an entity in the direction you click/touch.  I have a stub of that work in a branch I'll probably work on once 0.8.0 is released.)
It's possible that we should rename defineDpad to something like defineKeyboardDpad and reserve defineDpad for a future generic api -- thoughts?

I'm a bit confused what the correct upgrade strategy would be to support (the existing gamepad component) within Crafty 0.8.0

A start would be taking the existing code and getting it to emit control events instead of directly setting the entities state.  Then it might become clearer how to pull that work into the existing controls system (and, at the same time, how to make the existing control system more generalizable.)   That said, it doesn't look like those components rely on Multiway, so perhaps they would continue to work as is for now?

Good job on improving this system! Unfortunately both (!) of my pcs died after the summer and I have just gotten recently to installing a development environment. The control system has been released to public. I have some ideas that should simplify things but would break the API considerably. Is this something that would be ok if I made such a PR in the future, or is it too late now?

I have some ideas that should simplify things but would break the API considerably. Is this something that would be ok if I made such a PR in the future, or is it too late now?

I'm not opposed to breaking the api if the changes are worthwhile!  I'd suggest opening an  issue or stub PR sketching out the approach you're thinking of first.

My main motivation was figuring out the linkInput and unlinkInput methods, as it didn't seem very Crafty-esque that the controls take callbacks instead of dispatching appropriate events. However, after looking at the code in detail, they are mere convenience wrappers around binding to actual events.
linkInput and unlinkInput could be removed by triggering and binding to events with the triggerName / dpadName as the eventName instead.
I like the rest of the functionality provided by the control system, it opens up many different possibilities without the need to code your own stuff, good job 👍


Model.get_local_copy() returns empty directory

Context
Assuming a first task logs a model, a second task (cloned from the first one) tries to access to it:
parent_task = Task.get_task(Task.current_task().parent)
local_model = parent_task.models['output'][-1].get_local_copy()
logger.report_text(str(local_model))

The two tasks ran on the same agent, one after the other (the first one cloned itself and enqueued the second one). The first task finishes successfully:
020-05-18 14:20:34,261 - trains.Task - INFO - Completed model upload to file:///root/.trains/venvs-builds/3.6/task_repository/my-repo.git/checkpoint/test_conversion_pipeline/test_2.83871052c05c4f0a853a3870dbac302d/models/best_checkpoint_accuracy=0.0000.pt
2020-05-18 14:20:34,560 - trains.Task - INFO - Waiting to finish uploads
2020-05-18 14:20:48,567 - trains.Task - INFO - Finished uploading

Note: The saving of the models is handled by pytorch-ignite using TrainsSaver, every epoch. The files exist locally.
Problem
The code above leads to the following warning when executed in the second task:
2020-05-18 15:04:32,203 - trains - WARNING - Exception [Errno 21] Is a directory: '/root/.trains/venvs-builds/3.6/task_repository/my-repo.git/checkpoint/test_conversion_pipeline/test_2.83871052c05c4f0a853a3870dbac302d/models/best_checkpoint_accuracy=0.0000.pt'
Failed extracting zip file /root/.trains/venvs-builds/3.6/task_repository/my-repo/checkpoint/test_conversion_pipeline/test_2.83871052c05c4f0a853a3870dbac302d/models/best_checkpoint_accuracy=0.0000.pt

Indeed the local_model is a path to an empty directory.
Config
agent.python_binary =
agent.package_manager.type = pip
agent.package_manager.pip_version = >\=20.1

Hi @H4dr1en
The bug is verified, it is actually a trains bug.
Just making sure I understand, there is a local file that should be accessible at /root/.trains/venvs-builds/3.6/task_repository/my-repo.git/checkpoint/test_conversion_pipeline/test_2.83871052c05c4f0a853a3870dbac302d/models/best_checkpoint_accuracy=0.0000.pt
The correct behavior should be:
parent_task.models['output'][-1].get_local_copy() returns the local path, or None (and prints a warning) if the local path does not exist.
We try not to raise exception on these things, so you do not have to wrap every such call in try/except block, and just check for None. That said, we do print a warning, so If you do try to pass the None to another function, you can understand why you got it in the first place.
Makes sense to you?

Just making sure I understand, there is a local file that should be accessible at /root/.trains/venvs-builds/3.6/task_repository/my-repo.git/checkpoint/test_conversion_pipeline/test_2.83871052c05c4f0a853a3870dbac302d/models/best_checkpoint_accuracy=0.0000.pt

Exactly, instead, this path points to an empty directory.
I would rather have an exception raised, because it is a critical point of my script: why whould I call parent_task.models['output'][-1].get_local_copy() without using it afterwards? This is a resource that I will need if I create it.
get_local_copy(raise_on_failure=False) Could be a middle-ground: Let users decide if this is critical for them to have the model.

@H4dr1en, sounds good , a fix will be included in the next RC (including the raise_on_error :)

Hi @H4dr1en , there is an RC with a fix out, would you care to test?
pip install trains==0.15.0rc0

Now it is failing with trains==0.15.0rc0, (because of raise_on_error=True) with the following error:
Retrieving model...
2020-05-24 11:39:32,731 - trains - WARNING - Failed accessing local file: file:///root/.trains/venvs-builds/3.6/task_repository/my-repo/checkpoint/test_conversion_pipeline/test_2.dcf4a719f7f94062a9f03ef4393123d9/models/best_checkpoint_accuracy=0.0000.pt
Traceback (most recent call last):
File "piping.py", line 105, in
main(parse_args)
File "piping.py", line 78, in main
local_model = parent_task.models['output'][-1].get_local_copy(raise_on_error=True)
File "/root/.trains/venvs-builds/3.6/lib/python3.6/site-packages/trains/model.py", line 432, in get_local_copy
return self.get_weights(raise_on_error=raise_on_error)
File "/root/.trains/venvs-builds/3.6/lib/python3.6/site-packages/trains/model.py", line 274, in get_weights
return self._get_base_model().download_model_weights(raise_on_error=raise_on_error)
File "/root/.trains/venvs-builds/3.6/lib/python3.6/site-packages/trains/backend_interface/model.py", line 425, in download_model_weights
"failed downloading {}".format(self.model_id, uri))
ValueError: Could not retrieve a local copy of model weights e25455c8d0a94233a3ba1db94374a2cb, failed downloading file:///root/.trains/venvs-builds/3.6/task_repository/my-repo/checkpoint/test_conversion_pipeline/test_2.dcf4a719f7f94062a9f03ef4393123d9/models/best_checkpoint_accuracy=0.0000.pt

Hi @H4dr1en
Just making sure, the file /root/.trains/venvs-builds/3.6/task_repository/my-repo/checkpoint/test_conversion_pipeline/test_2.dcf4a719f7f94062a9f03ef4393123d9/models/best_checkpoint_accuracy=0.0000.pt exists in runtime ?
If it doesn't exists then raising an exception is the correct behavior if called with raise_on_error=True

Hi @bmartinn,
Closing here, this time the bug was one my side 👍
More details:
I was storing the model output locally in a relative path, which was appended to the current directory, which is the local git repository. I was expecting a second task to get the output model, but the agent simply removed the folder and cloned again the repo when running the second task (because it was the same repo), deleting the model outputs.


Should fix failing tests in Travis

@jabrah can this be merged? or is it sstill pending something else?


Support watching multiple projects from command-line

Make it possible to watch multiple csproj files that may not be connected in the P2P graph without having to create a "watch.proj" file.
Usage: add -p|--project that takes multiple files.
dotnet watch -p Web/web.csproj -p Test/test.csproj

@natemcmaster Did you and me talk about being able to do this differently now? If so, should be update the text of this bug?

This is something we can pretty easily implement, and I think it's a useful scenario, esp when/if global tools happen.

By the way, if we do this, we should also do https://github.com/aspnet/DotNetTools/issues/348. This is how I would use it:
dotnet watch -p Web/web.csproj -p Test/test.csproj -x ./build-and-run.sh

#!/usr/bin/env bash
# build-and-run.sh
dotnet build MyApp.sln
dotnet test --no-restore --no-build -p Test/test.csproj
dotnet run --no-restore --no-build -p Web/web.csproj

@DamianEdwards @shanselman What do you guys think about this (and #348)?
I think implementing these together should resolve the issue in #186 as well.

ping

Had this been abandoned?

It's in our backlog. We would consider a well-written PR to address this feature.

I am interested in taking this task. Looking at the code, I see there are a few comments around how one might go about doing this. Are there any additional suggestions or gotchas to be aware of?

One potential gotcha: current working directory. Currently, when you run dotnet watch -p MyProject/MyProject.csproj run, it starts a new process that invokes dotnet run with the working directory set to MyProject/.
https://github.com/aspnet/DotNetTools/blob/9b598446caa060374081b27c00f4c357d064c680/src/dotnet-watch/Program.cs#L152
We did this so users didn't need to invoke dotnet watch -p MyProject/MyProject.csproj -- run -p MyProject/MyProject.csproj.
My proposal: when multiple projects are specified, the inner process dotnet-watch starts should just use the current working directory of dotnet-watch rather than trying to adjust it.

I am still working on this. So far I've got --list with multiple projects working and I'm about to tackle the watch part. Before I get too far I'd like to get feedback on how I'm planning on approaching the watching of multiple projects. I'd really appreciate any comments or suggestions you might have. So here's some of what I did so far and what I'm planning.
Updated MsBuildProjectFinder.FindMsBuildProject
From
MsBuildProjectFinder.FindMsBuildProject(string searchBase, string project)

to
MsBuildProjectFinder.FindMsBuildProjects(string searchBase, IList<string> projectsList)

I did this so that I can better know that multiple projects were passed in as arguments and so that I can always operate on a list of projects, even if the list is just one when you don't pass in any --project arguments or only one project.
Plan for MainInternalAsync
Inside MainInternalAsync I'm planning on looping over each project that gets passed in and adding the DotNetWatcher tasks to a list.
            var watcherTasks = new List<Task>();
            foreach (var projectFile in projectFiles)
            {
                // build file set factory
                // build ProcessSpec
                watcherTasks.Add(new DotNetWatcher(reporter)
                    .WatchAsync(processInfo, fileSetFactory, cancellationToken));
            }

            await Task.WhenAll(watcherTasks);

As for the WorkingDirectory, I was going to set that if the list of projects passed into MainInternalAsync has no items or if it is null — I need to check if it is possible for it to be null.
Testing
I'm still looking at how I might go about writing some functional tests for this. I think I can write a few tests around watch using the WatchableApp. I'm still trying to understand how those work a bit more right now. From what I saw I might have to make some changes to the test harnesses to handle working with multiple projects. Any pointers around this would be really helpful.
Thanks again for any feedback you might have and for the time spent helping me out!

@smerrell ACK saw your comment. I'll take a closer look at this in a few days. I suspect there will be issues in your plan for MainInternalAsync. Currently working on time-sensitive issues so don't have time to go into details. Please ping me again if I don't surface for air by Fri. Nov. 2.

Awesome, thanks a ton! No rush on getting back to me. I'll keep poking around and try to make progress / learn what won't work.

@smerrell the main issue I see with the proposal for MainInternalAsync is that this basically is like launching multiple instances of dotnet-watch, which is something users can already do. This behavior watches multiple projects and runs multiple commands. The behavior I was hoping for watches multiple projects and then runs a single command. I believe this can be implemented by modifying MsBuildFileSetFactory to accept multiple files.
Under the hood, MsBuildFileSetFactory would only need to invoke MSBuild once to collect from all project files. This could be done by adding a new target to DotNetWatch.targets which accepts a semi-colon separated list of project files. Something like this:
dotnet msbuild DotNetWatch.Targets "-p:Projects=p1.csproj;p2.csproj;p3.csproj" -t:GenerateWatchList

  <PropertyGroup>
    <_CollectWatchItemsDependsOn Condition=" '$(Projects)' != '' ">
      _CollectWatchItemsFromProjects;
    </_CollectWatchItemsDependsOn>
  </PropertyGroup>

  <Target Name="_CollectWatchItemsFromProjects" Returns="@(Watch)" >
    <ItemGroup>
      <_Projects Include="$(Projects)" />
    </ItemGroup>

    <MSBuild Projects="@(Projects)"
             Targets="_CollectWatchItems"
             BuildInParallel="true"
             RemoveProperties="Projects" >
      <Output TaskParameter="TargetOutputs" ItemName="Watch" />
    </MSBuild>
  </Target>

Oh I see, that makes a lot of sense now. Thanks for pointing that out! I'll start looking at MsBuildFileSetFactory and learning about what all will need to be done. Thanks a ton for all the advice so far.


page-align native libraries

Android expects native libraries to be uncompressed and page-aligned unless the manifest tells the system to extract them to disk. Without this, generated Apk files that have set extractNativeLibs to false will fail to install.

:+1:
I will implement it when I get home

Great !
It is valid to have bin XML resource entry with name like res/layout/splash.so Please make it something more specific for example
String name = entry.getName();
int alignment;
if(name.startsWith("lib/") && name.endsWith(".so")){
    alignment = ALIGNMENT_PAGE;
}else {
    alignment = mAlignment;
}

Thanks

Done


fix: upgrade underscore from 1.12.1 to 1.13.6

Snyk has created this PR to upgrade underscore from 1.12.1 to 1.13.6.
See this package in npm:
https://www.npmjs.com/package/underscore
See this project in Snyk:
https://app.snyk.io/org/roma8389/project/7873cf15-3c44-4d49-b218-ced57450efdb?utm_source=github&utm_medium=referral&page=upgrade-pr

merged


Where is the MMIO and interrupt simulation?

In the paper, the authors claim that "The CPU is the bus master which interacts with the accelerator by writing task descriptors to memory-mapped registers inside the accelerator." And in Figure 1 there is an interrupt interface. However I cannot find corresponding code in this codebase. Could you care to point it out for me?

Hi @Z-KN, the paper describes a model to integrate the accelerator in a system context. However, the interrupt interface is out of scope of this tool.


Documentation error: Installation

Hi --
The example code in the Installation section of your main page does not work.  Specifically, the command as listed,
wget https://github.com/rabix/bunny/releases/download/v1.0.5/rabix-1.0.5.tar.gz -O rabix-1.0.5.tar.gz && tar -xvf rabix-1.0.5.tar.gz

yields the error,
HTTP request sent, awaiting response... 404 Not Found
2018-04-20 14:32:14 ERROR 404: Not Found.

I'm imagine this is an easy fix, and one which those just getting started would appreciate.
Thanks for looking into it.
Matt

wget https://github.com/rabix/bunny/archive/v1.0.5-1.tar.gz -O rabix-1.0.5.tar.gz && tar -xvf rabix-1.0.5.tar.gz

should work

Hi Jeremiah -
Thanks! I did get it figured out.
My suggestion is to get a stable URL (“latest.tar.gz”) which will keep your
documentation from regularly breaking.
Regards -
Matt
On Tue, Apr 24, 2018 at 18:07 Jeremiah H. Savage<EMAIL_ADDRESS>wrote:

wget https://github.com/rabix/bunny/archive/v1.0.5-1.tar.gz -O rabix-1.0.5.tar.gz && tar -xvf rabix-1.0.5.tar.gz
should work
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/rabix/bunny/issues/443#issuecomment-384107959, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AFCZSmlTkdt7POG1NLKz4rBOMpO88Np9ks5tr7AwgaJpZM4Td-5q
.

--
Matthew A. Wyczalkowski
<EMAIL_ADDRESS>


Seems cannot load pretrained model

these are the values of the reward supposed to be
https://i.imgur.com/MPAmU9M.png
what I get when I tried to retrain a model
https://i.imgur.com/GU1Tv7U.png
I've uncommented those two lines
https://i.imgur.com/jyTutI7.png
Besides the reward, the actions in retraining also look like random. I think I just started a new training instead of retraining a pretrained model? How to solve it? Thank you.

how do you solve this problem?  thanks


status: detect when DaemonSets and Deployments are rolling out

There are a few additional states that indicate a Deployment or DaemonSet is progressing and not yet fully deployed:

ObservedGeneration < Generation means the relevant controller has not yet observed the changes

UpdatedReplicas / UpdatedNumberScheduled too small means that the update has not fully rolled out.
Fixes: #413 which was somehow reverted in a refactor

Signed-off-by: Casey Callendrello<EMAIL_ADDRESS>

Test failure is a flake connecting to google.com. Definitely not related to this PR.
re-pushed anyways to address goimport issue.


macOS: rebranding

This PR is a revived version of #359. It replace all OS X reference to #{MacOS.os_name} except that

The files in the vendor folder
The files in the cask folder
The files without a .rb extension
Reference of a specific version e.g. OS X 10.8.2, OS X Yosemite

It also replace all OS X reference to macOS on all comments except for reference of a specific version.
This PR fixes #961.
There are 2 OS X references in brew.sh. The tricky part is that even /usr/bin/sw_ver -productName returns Mac OS X on macOS 10.12, so I don't know a safe way (except if-else on productVersion) to replace OS X string in shell scripts.

[x] Have you followed the guidelines in our Contributing document?
[ ] Have you checked to ensure there aren't other open Pull Requests for the same change?
There is similar pull request #359, this PR covers the works on #359.
[x] Have you added an explanation of what your changes do and why you'd like us to include them?
[x] Have you written new tests for your changes? Here's an example.
[x] Have you successfully run brew tests with your changes locally?

As mentioned in https://github.com/Homebrew/brew/issues/961#issuecomment-247779301 by now I'm strongly in favor of maximum simplicity and using “macOS” everywhere as soon as 10.12 is released. However, if we really wanted to be serious about adapting our output, the distinction will need to be three-fold (“Mac OS X” for 10.0-10.6, “OS X” for 10.7-10.11, and “macOS” for 10.12+).

As mentioned in #961 (comment):
The API reference at https://developer.apple.com/reference/ only ever speaks of “macOS” when talking about when an API was introduced, even if it's “macOS 10.0+”.

This is by far the strongest argument for me so I'm 👍 with just using macOS all over.
@JLHwung I'm so sorry to send you on such a wild chase for this, my apologies. I hope you've learnt some stuff through doing so and will continue to contribute to Homebrew. Let me know if you'd rather doing the mass-macOS replacement or if you'd rather we did. Thanks so much for your contributions.

@MikeMcQuaid Sorry for late reply as I am in vacation now. I've learnt a lot more than it appears at the beginning, and thank you for your review. I cannot spare time on replacing before 20th so it'd better assigned to other people.
As I am working on this pr, I find many macOS-dependent code does not locate in os/mac or extend/os/mac, shall we split the platform-dependent codes or just let it be?

Thanks you for your work here!

As I am working on this pr, I find many macOS-dependent code does not locate in os/mac or extend/os/mac, shall we split the platform-dependent codes or just let it be?

This is an ongoing process and we started it fairly recently in an attempt to make Homebrew truly platform-agnostic (to the extent it makes sense) and to help move back code from Linuxbrew into Homebrew with proper abstractions and cleaner code structure. If that's a topic that interests you, we're more than happy to have someone help us with this endeavor.


A new Safari bug?

What do you think about this strange behaviour?
http://stackoverflow.com/questions/44025290/flexbox-strange-behaviour-with-cssgrid
It works good on Chrome, not on Safari (iPad/iPhone devices and Mac).
Safari 10.1, latest today.

Thanks for reporting. However, this issue does not contain all the information needed as outlined in the reporting guidelines. Please update it with more information or close it if it's no longer relevant.


Improved logging

Summary
This PR was triggered by some recent investigation into community error logs:

I realized a few things, especially after digging into the code:

We log too many things as errors that should instead be warnings.
Unlike the main server, we didn't have any logs for when an incoming http event occurred.
It was hard to tie together logs from the same request, much less consistently identify the user in question.
We logged the "server caller" but not the "plugin caller"
We had 4 different logging patterns!!

This PR improves logging by:

Eliminate mlog, bot.Logger and pluginapi.Log in favour of just logrus. (See comments below.)
Consistently leverage logrus.WithError to pull out errors into a dedicated error property in the JSON log output.
Only log >= 500 API errors as errors, otherwise just a warning
Log a Received HTTP request before and a Handled HTTP request after each handler invocation.
Generate a request_id property on a logger for use to tie together logs from the request handler, e.g. errors with the original request that triggered it. With the history, we can now find when the request started, what user triggered it, and the HTTP method and URL. (A future PR might pass that logger into the service subsystem so that deeper logs there are grouped accordingly.)
Log the "plugin_caller" not just the "server caller" (see (see https://github.com/mattermost/mattermost-plugin-api/pull/118 for the implementation).
Documented philosophy around when to log an error vs. warning, etc.
Lots of tidying up of logging to pull fields out discretely.

Why logrus over mlog? I think structured logs are easier to do Logrus, plus you can scope and reuse such loggers:
logger := logrus.WithFields(logrus.Fields{
	"method":     r.Method,
	"url":        r.URL.String(),
	"user_id":    r.Header.Get("Mattermost-User-Id"),
	"request_id": requestID,
})
r = r.WithContext(context.WithValue(r.Context(), requestIDContextKey, requestID))

// Includes fields above
logger.Debug("Received HTTP request")

next.ServeHTTP(&recorder, r)

// Includes fields above
logger.WithField("status", recorder.statusCode).Debug("Handled HTTP request")

Note that this PR depends on https://github.com/mattermost/mattermost-plugin-api/pull/118. (I'll update the commit properly once that PR is merged.)
Example
Here's a batch of logs:
{"timestamp":"2022-06-27 13:28:33.290 -03:00","level":"debug","msg":"Received HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","url":"/api/v0/bot/connect","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"mdpwesbhhibhd8ds79ibh5owoe","method":"GET","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:67"}
{"timestamp":"2022-06-27 13:28:33.290 -03:00","level":"debug","msg":"Received HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"7ymj5g3ppirepjcyu6fcf8hf6r","method":"GET","url":"/api/v0/settings","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:67"}
{"timestamp":"2022-06-27 13:28:33.291 -03:00","level":"debug","msg":"Handled HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","request_id":"7ymj5g3ppirepjcyu6fcf8hf6r","status":"200","method":"GET","url":"/api/v0/settings","user_id":"5aedxdss3brg3p6bekcbydjxiy","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:71"}
{"timestamp":"2022-06-27 13:28:33.354 -03:00","level":"debug","msg":"Received HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","method":"POST","url":"/api/v0/query","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"dt9hjizn1idjbkaszu1k58n6be","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:67"}
{"timestamp":"2022-06-27 13:28:33.355 -03:00","level":"debug","msg":"Received HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"jb5hwraufpdwufuu3smhr1zsyh","method":"GET","url":"/api/v0/stats/playbook?playbook_id=i9s3k1tarpywbef3qbsm8gibir","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:67"}
{"timestamp":"2022-06-27 13:28:33.367 -03:00","level":"debug","msg":"Handled HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","url":"/api/v0/bot/connect","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"mdpwesbhhibhd8ds79ibh5owoe","status":"200","method":"GET","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:71"}
{"timestamp":"2022-06-27 13:28:33.369 -03:00","level":"warn","msg":"Not authorized","caller":"app/plugin_api.go:943","plugin_id":"playbooks","request_id":"dt9hjizn1idjbkaszu1k58n6be","error":"does not have permissions\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.init\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:15\nruntime.doInit\n\truntime/proc.go:6222\nruntime.doInit\n\truntime/proc.go:6199\nruntime.doInit\n\truntime/proc.go:6199\nruntime.main\n\truntime/proc.go:233\nruntime.goexit\n\truntime/asm_arm64.s:1263\nuserID 5aedxdss3brg3p6bekcbydjxiy to access playbook\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.(*PermissionsService).PlaybookViewWithPlaybook\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:356\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.(*PermissionsService).PlaybookView\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:343\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.(*RootResolver).Playbook\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/api/graphql_root.go:26\nreflect.Value.call\n\treflect/value.go:556\nreflect.Value.Call\n\treflect/value.go:339\ngithub.com/graph-gophers/graphql-go/internal/exec.execFieldSelection.func2\n\tgithub.com/graph-gophers/graphql-go@v1.4.0/internal/exec/exec.go:211\ngithub.com/graph-gophers/graphql-go/internal/exec.execFieldSelection\n\tgithub.com/graph-gophers/graphql-go@v1.4.0/internal/exec/exec.go:231\ngithub.com/graph-gophers/graphql-go/internal/exec.(*Request).execSelections.func1\n\tgithub.com/graph-gophers/graphql-go@v1.4.0/internal/exec/exec.go:81\nruntime.goexit\n\truntime/asm_arm64.s:1263","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/graphql_root.go:27"}
{"timestamp":"2022-06-27 13:28:33.369 -03:00","level":"error","msg":"Error executing request","caller":"app/plugin_api.go:940","plugin_id":"playbooks","request_id":"dt9hjizn1idjbkaszu1k58n6be","error":"graphql: Not authorized","operation":"Playbook","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/graphql.go:135"}
{"timestamp":"2022-06-27 13:28:33.369 -03:00","level":"warn","msg":"Not authorized","caller":"app/plugin_api.go:943","plugin_id":"playbooks","request_id":"jb5hwraufpdwufuu3smhr1zsyh","error":"does not have permissions\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.init\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:15\nruntime.doInit\n\truntime/proc.go:6222\nruntime.doInit\n\truntime/proc.go:6199\nruntime.doInit\n\truntime/proc.go:6199\nruntime.main\n\truntime/proc.go:233\nruntime.goexit\n\truntime/asm_arm64.s:1263\nuserID 5aedxdss3brg3p6bekcbydjxiy to access playbook\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.(*PermissionsService).PlaybookViewWithPlaybook\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:356\ngithub.com/mattermost/mattermost-plugin-playbooks/server/app.(*PermissionsService).PlaybookView\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/app/permissions_service.go:343\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.(*StatsHandler).playbookStats\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/api/stats.go:96\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.withLogger.func1\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:48\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2084\nmain.(*Plugin).getErrorCounterHandler.func1.1\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/plugin.go:386\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2084\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.MattermostAuthorizationRequired.func1\n\tgithub.com/m
attermost/mattermost-plugin-playbooks/server/api/api.go:103\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2084\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.LogRequest.func1\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:69\nnet/http.HandlerFunc.ServeHTTP\n\tnet/http/server.go:2084\ngithub.com/gorilla/mux.(*Router).ServeHTTP\n\tgithub.com/gorilla/mux@v1.8.0/mux.go:210\ngithub.com/mattermost/mattermost-plugin-playbooks/server/api.(*Handler).ServeHTTP\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/api/api.go:48\nmain.(*Plugin).ServeHTTP\n\tgithub.com/mattermost/mattermost-plugin-playbooks/server/plugin.go:84\ngithub.com/mattermost/mattermost-server/v6/plugin.(*hooksRPCServer).ServeHTTP\n\tgithub.com/mattermost/mattermost-server/v6@v6.3.0/plugin/client_rpc.go:456\nreflect.Value.call\n\treflect/value.go:556\nreflect.Value.Call\n\treflect/value.go:339\nnet/rpc.(*service).call\n\tnet/rpc/server.go:381\nruntime.goexit\n\truntime/asm_arm64.s:1263","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/api.go:75"}
{"timestamp":"2022-06-27 13:28:33.369 -03:00","level":"debug","msg":"Handled HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","method":"POST","url":"/api/v0/query","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"dt9hjizn1idjbkaszu1k58n6be","status":"200","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:71"}
{"timestamp":"2022-06-27 13:28:33.370 -03:00","level":"debug","msg":"Handled HTTP request","caller":"app/plugin_api.go:934","plugin_id":"playbooks","status":"403","user_id":"5aedxdss3brg3p6bekcbydjxiy","request_id":"jb5hwraufpdwufuu3smhr1zsyh","method":"GET","url":"/api/v0/stats/playbook?playbook_id=i9s3k1tarpywbef3qbsm8gibir","plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:71"}

and now I can filter down to just one request_id to trace the messages in question:
cat logs/mattermost.log | jq -c 'select(.plugin_id=="playbooks") | select(.request_id=="dt9hjizn1idjbkaszu1k58n6be") | .msg'
"Received HTTP request"
"Not authorized"
"Error executing request"
"Handled HTTP request"

Looks like using logrus also allows us to detect unsanitized user input making it's way into logs -- previously masked, perhaps? Or did just touching this code trigger the detection? Either way, I'll take a look at these :)

One thing to note: right now, plugin_caller is accurate, but because of our handler semantics, often just points to a helper method, e.g. "plugin_caller":"github.com/mattermost/mattermost-plugin-playbooks/server/api/logger.go:67". I'd love to either find a way to notify the logger that this is a helper a such, or move these logs back closer to where they are actually invoked. (This isn't an issue where we don't abstract away logging.)

Added myself as a reviewer to this PR, since it contains multiple CodeQL alerts.

Following up on the CodeQL issues at https://community.mattermost.com/core/pl/7zsxojoqpigif8odyuw5rdw41y.

@srkgupta, any idea what's going on with the code scanner above?

@lieut-data do you think we could separate in a fresh pr the fix of the log levels before we change the approach for logging?

Thanks, @trilopin! I considered this, but thought it might be easier just to switch to the context model. For now, the underlying logger is still encoded within the http.Request context, but it's now accessed as per a *Context parameter passed to all APIs.
Hopefully this unblocks us :)

(Note that I haven't switched us to mlog until that package can be used seamlessly across the RPC boundary like we can do for logrus.)

@crspeller / @trilopin, I've fixed the build issues -- my upgrade of the plugin-api brought in the "new" EnsureBot and triggered https://community.mattermost.com/core/pl/9c8txjwkytdczc6kw8ngggyour. Fixed by making the OwnerId explicit for now.
@srkgupta, are the the security messages here just false positives again? What we can do to fix this from happening? I'm worried about all the future logging messages...

@srkgupta, are the the security messages here just false positives again? What we can do to fix this from happening? I'm worried about all the future logging messages...

Hi @lieut-data
My apologies. Somehow this missed my attention before. I have taken a look at the new alerts and have dismissed them as Won't Fix (Not Affected). Unfortunately we need to manually review these alerts and ensure that we aren't accidentally leaking sensitive credentials in the logs (or) directly logging user controlled inputs without proper sanitisation.
I have added a follow-up task for this and will pick this up and check how we can reduce such false positives.
Thanks

Thanks, @srkgupta! What's weird is that we've been logging all along without these warnings showing up in code reviews, and now that we're using logrus, CodeQL's automated checks seem to "discover" these logs. That makes me think all our existing logging in other applications is probably just getting ignored. Would we consider disabling these checks for now until they can catch all types of logs consistently? Otherwise, CodeQL may become very noisy in all subsequent PRs for Playbooks after this PR is merged.

Once these alerts are analyzed and dismissed, they will not be re-triggered again in the subsequent PRs unless there are additional similar code changes.

Thanks, @srkgupta! So, just to confirm, will new log messages trigger alerts in new PRs?

will new log messages trigger alerts in new PRs?

As long as they are some how directly logging a user controlled input as is, yes they will trigger new alert.

Thanks, @srkgupta! To explore this further, I created https://github.com/mattermost/mattermost-plugin-playbooks/pull/1465 using the existing logging infrastructure and referencing a user-controlled input that triggered a warning in the same way as this PR: https://github.com/mattermost/mattermost-plugin-playbooks/pull/1271/files#diff-6e3ba3ce59f140effc0a4e689feb56b2d7eb2f14529117cc7e20474962095028R296, but CodeQL did not generate a warning there.

@gbochora, I'd love to get this merged after we cut v7.3 final. Would you be able to take a look at the approach here? It's already been partially reviewed, but we just want that extra confidence to avoid churn later on :)

Thanks, @srkgupta! To explore this further, I created #1465 using the existing logging infrastructure and referencing a user-controlled input that triggered a warning in the same way as this PR: https://github.com/mattermost/mattermost-plugin-playbooks/pull/1271/files#diff-6e3ba3ce59f140effc0a4e689feb56b2d7eb2f14529117cc7e20474962095028R296, but CodeQL did not generate a warning there.

Got it. Thanks for the explanation @lieut-data. I might have to dig deep dive into the CodeQL tool to understand when & how such alerts are triggered. I will take this up along with the task and investigate this in detail. Meanwhile this PR can be merged as there are no open CodeQL alerts. Thanks.

@gbochora, I'd love to get this merged after we cut v7.3 final. Would you be able to take a look at the approach here? It's already been partially reviewed, but we just want that extra confidence to avoid churn later on :)

Sure, I'm in the process of review and will finish today.


FeedWrangler: Can't use plus sign (+) when renaming a feed

The plus sign doesn't sync to the server when used in a name like "Food + Drink" for a feed.

There is a new variable that we've added to URLComponents via an extension in RSWeb that might help you out with this one.  It is enhancedPercentEncodedQuery.  Many thanks to @quanganhdo for the fix!


SomeOne is selling this on codecanyon

https://codecanyon.net/item/btc-faucet-rotator-earn-passive-cryptocurrency/20550719

SomeOne is selling this on codecanyon

you need to report him

Hi Jalakiva,
Thank you for messaging me about this. It irritates me that someone is willing to sell an open-sourced project for their own profit. The whole reason why I made this open-source is that the most likely users cannot afford to pay for it, and it is a way to give back to the Bitcoin/open-source communities.
Upon visiting the CodeCanyon link, I see the item has been removed. If you see any more instances of this happening, please do let me know. While people are free to modify the script however they please, I DO NOT approve of anyone selling the script.


LoadFilteredPolicy not filtered as expected

I'm not sure whether my expectations of this functionality are incorrect or it is a defect.
Expected: The following filter should return policies where one of the policy params is equal to "data1" AND that policy assigned to user "alice".
e.LoadFilteredPolicy(&pgxadapter.Filter{ 		P: [][]string{{"", "data1"}}, 		G: [][]string{{"alice"}}, 	})
Actual behavior: It will return all the policies where one of the policy params is equal to "data1" (doesn't matter whether it is assigned to "alice" or not).
SQL statement generated by this filter will look like:
SELECT "p_type", "v0", "v1", "v2", "v3", "v4", "v5" FROM <table_name_here> WHERE (p_type = $1 AND (v1 = $2 )) OR (p_type = $4 AND ((v0 = $5)))
Which basically  will return all records which matches P filter OR G filter.
As result further call to enforcer.GetPolicy() will return policies which were not assigned to user.

I think your expectation for the filter functionality is not accurate. LoadFilteredPolicy is not meant to drill down to the exact row that has what you want. But rather to just narrow down the list of policies to filter through. As long as subsequent Enforce call works as expected then this function works as expected.
You also misunderstand what the filter is doing. The filter is getting P policies that has "data1" as its second value, and G policies that has "alice" as its first value. Keep in mind that P policies and G policies are kept as separate rows. If you want this function to return a policy that is both a P policy and a G policy then it will just return nothing.
Why do do you want to use GetPolicy?

i.e. I have lots of policies for different users, on api request I just want to load policies for specific user, since I don't want to load entire dataset for all users (make sense ?). With the behavior you described LoadFilteredPolicy basically become useless since it will load entire dataset.

2)Don't know how you came to that conclusion, I gave the query which adapter will produce, it is pretty clear to me how it works. I question the design if it is not a defect.

i.e. I want to create an API which will return all policies assigned to user. So, my expectation I can just load policy for specific user LoadFilteredPolicy and then call getPolicy (for simplicity let's say I just reload policy completely, so there is no policies assigned to other users).

** To be honest at this point the only peace of library which I use is just enforce part, I had to write my own code to update/read etc policies etc... most APIs  are just useless for real application.. no possibility save policy in a transaction with other updates..  even loading functionality is flawed, why I cannot just pass a pgx.Pool to the adapter and it should get connections from it ? instead it will create a new connection , which will miss runtime configuration - another defect..

Please try the query that you want on the PostgreSQL and tell me what it produces.
You should try GetFilteredPolicy after LoadFilteredPolicy.

You completely misunderstood what LoadFilteredPolicy does. It is described in a section called Policy Subset Loading. It loads a subset, not a single row. If you are complaining about the design then this is the wrong place friend. You should complain with Casbin creators. This repo is just an adapter.

I copied that query from my app and removed some data, in order to sho the logic, I have a little bit different columns in my table, I was hopping you can get the logic.... Ok,  Check this out http://sqlfiddle.com/#!17/cc6db0/1

"You completely misunderstood what LoadFilteredPolicy does" --- Yeah I'm loading subset for user 'alice' as defined in your example, why then it is loading policies for admin ?... or can you explain what is the practical use of this if you understand:
e.LoadFilteredPolicy(&pgxadapter.Filter{ P: [][]string{{"", "data1"}}, G: [][]string{{"alice"}}, })

So I thought you wanted to turn the OR query into an AND query? OR did I misunderstand? What is the exact query that you want the adapter to produce?

Logically I would expect something like this  http://sqlfiddle.com/#!17/cc6db0/2 (not exact query, but the output)
I would check how other (official ?) adapters work, but this implementation looks strange to me.

First of all, I wasn't rude to you and raising problems is also a contribution, don't take it personal. I think I pretty clearly described the issue and made extra step (provided working example) to make sure my point is clear. On opposite you did't pay attention to what I wrote and attacked me from the first message... that I "completely misunderstood"..
I read official docs for casbin https://casbin.org/docs/en/policy-subset-loading, what I see there is this example:
`adapter := fileadapter.NewFilteredAdapter("examples/rbac_with_domains_policy.csv")
enforcer.InitWithAdapter("examples/rbac_with_domains_model.conf", adapter)
filter := &fileadapter.Filter{
P: []string{"", "domain1"},
G: []string{"", "", "domain1"},
}
enforcer.LoadFilteredPolicy(filter)`
In this example they use model with domains, practical use of this use case they basically brake down policies by domains, so they filter out policies and users from different domain. i.e.
p, admin, domain1, data1, read p, admin, domain1, data1, write p, admin, domain2, data2, read p, admin, domain2, data2, write g, alice, admin, domain1 g, bob, admin, domain2
It still not optimal, cos you will be loading all policies and users for the domain, but at least I can see logic here.
In your example:
e.LoadFilteredPolicy(&pgxadapter.Filter{ P: [][]string{{"", "data1"}}, G: [][]string{{"alice"}}, })
It is very confusing, since you filter by resource (or object) "data1" and user "alice" ... From this example it looks like it will load policies for specific user, but it is not true and if that the case I can't think of any practical use case where you would want to filter like this since there can be great amount of policies which mention access rule for "data1" which will by loaded.  Mentioning "data1" in P filter will not filter out policies which are not associated with user "alice" and mentioning "alice" in G filter will not eliminate groups which are not related to "data1"..
So, could you please tell me what I missing or wrote incorrect, so I finally can understand ?
p.s. I appreciate that  you contributing to open source software, but don't take it personal when people raising problems.

I contributed significantly to the official adapter https://github.com/casbin/casbin-pg-adapter. So maybe I do know a lot more than you about what a correct implementation looks like. Unqualified remarks such as "defect", or "strange implementation" are not welcome at all. You better have a water-tight case when you come to me with remarks like that.
Since this is free work, the burden of reading and understanding the example and the code fall largely on the user. This adapter maintains the same interface proposed by the official documentation so I don't understand what else I need to explain. The casbin-pg-adapter has the same example, yet countless people are able to use it without any problem. My advice to you, think harder, read the documentation harder, then you will understand.
And don't complain about documentation. This is free work. Want to have better documentation? Feel free to fork.
You need to be more charitable with your thought process and how you evaluate other people's works. I really will not tolerate any more unqualified remarks.

So maybe I do know a lot more than you about what a correct implementation looks like.
You still didn't explain the logic, while I explained the case to the full.. not sure what else should I include .. you just saying you know something... I'm sorry but what I see is that you just mirrored the lib without actually __thinking and understanding___what it is doing and how it is going to be used...

Unqualified remarks such as "defect", or "strange implementation"
What is Unqualified here ? I gave you real problems with your lib and explained use case pretty well... you can't say nothing meaningful other than you know something..  and yeah there a few defect exists which I mentioned, but seems to be you don't care...

and how you evaluate other people's works
This is a technical discussion, I brought specific problems to discuss, but you can't say nothing about the actual problems.

Well, good luck maintaining this.


Semi-bold text

Please could we make sure the following elements are semi-bold (weight 600)?

Bar labels
Annotation labels
Legend labels

Thank you!

Thanks for setting this - happy for it to be marked as closed.


net: TestLookupGoogleSRV flaky on linux-arm bot

Looking at build.golang.org right now, all 4 linux-arm failures are due to TestLookupGoogleSRV flakes:
--- FAIL: TestLookupGoogleSRV (20.05s)
	lookup_test.go:71: lookup _xmpp-server._tcp.google.com on <IP_ADDRESS>:53: no such host
FAIL
FAIL	net	22.518s

http://build.golang.org/log/18b91c9dd84cfa397ae48f92f5806eb64be62d41
http://build.golang.org/log/31c93edec34c46a3b140db465f905f2412b8ef2f
http://build.golang.org/log/afede4adbc79168d91a430d5033cbd9d962d2729
http://build.golang.org/log/e6e72e6ad1f44c4ecd87884374d964bb8497582f
This is also causing trybot flakes; e.g., I had linux-arm flakes on golang.org/cl/21231 and golang.org/cl/21232.

The scaleway ARM builders have:
# cat /etc/resolv.conf
domain cloud.scaleway.com
search cloud.scaleway.com
nameserver <IP_ADDRESS>
nameserver <IP_ADDRESS>

Which explains the error message.

Maybe the external net tests should hard-code <IP_ADDRESS>?

I feel like it should be reasonable for tests to assume /etc/resolv.conf points to reliable DNS servers.  What about just configuring the Scaleway ARM VMs to use "nameserver <IP_ADDRESS>" in their /etc/resolv.conf instead of changing any tests?
We should probably also warn Scaleway that their DNS servers appear unreliable.

Done. Sent https://go-review.googlesource.com/21194 and deployed.


Add tcp port to peer authn test

The failing test flakes for the test flakes described in #54146 (potentially caused by #54073) are sending traffic to port 9090 while the test in question only has port rules for the http workload port (with a corresponding service port of 8080). If we add the workload tcp port to the policy, it should fix the flakes

Converting to draft because I think I found the root cause

@keithmattix: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name
Commit
Details
Required
Rerun command

integ-pilot-multicluster_istio
d233057cdbded3fdeedcd145a716c2b1629c4f1c
link
true
/test integ-pilot-multicluster

Instructions for interacting with me using PR comments are available here.  If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

/retest


Revert "shaderObject: Add VK_EXT_discard_rectangles"

This reverts commit 5135e76e8a4fee74e7d73bbf8c90d678ce94f62f.

CI Vulkan-ExtensionLayer build queued with queue ID 4798.

CI Vulkan-ExtensionLayer build queued with queue ID 4798.

The psychic energy of the universe has informed me that this change is somehow incorrect.

CI Vulkan-ExtensionLayer build # 404 running.

CI Vulkan-ExtensionLayer build queued with queue ID 4805.

CI Vulkan-ExtensionLayer build queued with queue ID 4805.

CI Vulkan-ExtensionLayer build # 405 running.

CI Vulkan-ExtensionLayer build # 405 passed.


Added download info to summary

Add a line which shows where the mod is going to be downloaded from to the Metadata page.
Right now it's a mystery, would be helpful for debugging or knowing if some location is having a download problem

wrong log issue list


Revert "feat: deprecate ETH_POPULATE_SYNTHETIC_CONTRACT_RESULTS env…

Reverts "feat: deprecate ETH_POPULATE_SYNTHETIC_CONTRACT_RESULTS env (#2768)"
This reverts commit e0fc47328865514b6cd5fa4254f80d81fb5f7285.
Description:

Related issue(s):
Fixes #
Notes for reviewer:

Checklist

[ ] Documented (Code comments, README, etc.)
[ ] Tested (unit, integration, etc.)

Not needed.  PR #2784


invalidate JWTs when products change

When the product list app bindings change, we should try to remove any cached JWTs that would no longer be valid because of the change.

Detail on recreating the issue:

Make API call with JWT, where the client_id is for an app that is not subscribed to any product with istio services
As expected, the Apigee mixer adapter rejects the request.
(However, the Apigee mixer adapter caches the resulting token from the call to ${customer_base}/verifyApiKey)
(This token contains the API products list associated with the given client_id)
Now go on Apigee Edge, and associate the App with a proper product that has the Istio service
Make API call using the exact JWT from step one.
Apigee rejects the request, even though the client_id is now associated with a product that has the Istio service.

You have to either delete the mixer adapter pod (to clear the cache), or wait till the cached token expires. This looks like a bug. I would have expected the mixer adapter to clear the cache entries associated with the client_id ...when its product list changes.

After investigation, unfortunately, this isn't directly possible. There is no way to tell if a cached client_id might be associated with new products. The best we could do would be to address the other direction: If a product no longer exists, we could potentially invalidate any cached Client IDs that are associated with it. Given the limited cache lifetime and infrequency of change, I'm not sure this is a worthwhile enhancement.
Another potential enhancement could be to watch the product list and refresh all cached Client IDs if anything on the product list changes. For the same reasons as above, I'm not sure this is a reasonable enhancement either.
As this point, I'm going to close this issue until and unless we have a customer request to consider it.


main branch protection

@yannham could you protect the main branch? We just pushed to it instead of making a PR, it seems we didn't mess everything by pushing again with --force without the new commit, but it would be nice to protect that in the future :sweat_smile:

main should now be protected against direct commit.


Parcel abstract base classes

Hi, I've got an abstract base class which has some fields I need to parcel. But Parceler generates a construction of the abstract class and of course it's not possible. Compilation fails as expected. Is there some suggested way to parcel the fields of an abstract base class? As a temporary workaround I made my base class non-abstract, but that's not good in the long term.

Hm... I've just removed the @Parcel annotation from the abstract class and looks like it works. But is this the official way?

Parceler will analyze the superclasses of your annotated class and handle serialization of all properties, unless configured not to.


Duplicate 'bar' on pages that are direct book children in sidebar tree list

Also update DummyContentSeeder to create books that would show this issue.


How to provide a KubeSchedulerConfiguration ?

Hi,
I'm trying to configure topo-lvm provisionner into k3s.
Unfortunately, kube-scheduler need to be re-configured to ignore topo-lvm PVC creation, via this config:
https://github.com/topolvm/topolvm/blob/main/deploy/README.md#configure-kube-scheduler
https://github.com/topolvm/topolvm/blob/main/deploy/scheduler-config/scheduler-config-v1beta2.yaml
It seem impossible to do so into K3S seem kube-scheduler and his API is not accessible ?
Thanks for hard work,

you can pass args to kube-scheduler via the --kube-scheduler-arg flag on servers. For example, --kube-scheduler-arg=config=/var/lib/scheduler/scheduler-config.yaml. Is there something beyond this that does not seem possible?

I got a similar problem.

For example, --kube-scheduler-arg=config=/var/lib/scheduler/scheduler-config.yaml.

That command throws an --kube-scheduler-arg=config=: command not found error on my machine.
did I do something wrong?
I want to run a scheduler configuration file from the  tutorial in K3s, currently only for testing
but that ApiVersion is probably also incompatible with my K3s version

That command throws an --kube-scheduler-arg=config=: command not found error on my machine.

That's not a command to run, it's an option to add to the K3s command-line arguments.

@brandond Thanks for the fast response!
So this e.g. is an installation with a configured scheduler, correct?
curl -sfL https://get.k3s.io | K3S_KUBECONFIG_MODE="644" sh -s - k3s server --kube-scheduler-arg=config=/var/lib/scheduler/scheduler-config.yaml
I'm getting this output:
[INFO]  Finding release for channel stable
[INFO]  Using v1.23.6+k3s1 as release
[INFO]  Downloading hash https://github.com/k3s-io/k3s/releases/download/v1.23.6+k3s1/sha256sum-amd64.txt
[INFO]  Downloading binary https://github.com/k3s-io/k3s/releases/download/v1.23.6+k3s1/k3s
[INFO]  Verifying binary download
[INFO]  Installing k3s to /usr/local/bin/k3s
[INFO]  Skipping installation of SELinux RPM
[INFO]  Creating /usr/local/bin/kubectl symlink to k3s
[INFO]  Creating /usr/local/bin/crictl symlink to k3s
[INFO]  Creating /usr/local/bin/ctr symlink to k3s
[INFO]  Creating killall script /usr/local/bin/k3s-killall.sh
[INFO]  Creating uninstall script /usr/local/bin/k3s-k3s-uninstall.sh
[INFO]  env: Creating environment file /etc/systemd/system/k3s-k3s.service.env
[INFO]  systemd: Creating service file /etc/systemd/system/k3s-k3s.service
[INFO]  systemd: Enabling k3s-k3s unit
Created symlink /etc/systemd/system/multi-user.target.wants/k3s-k3s.service → /etc/systemd/system/k3s-k3s.service.
[INFO]  systemd: Starting k3s-k3s

Yes, that looks like the output of the installer. Is there some problem with it?


MountableFile path resolving on Windows buggy

Added new test into MountableFileTest.java
@Test
public void forHostFileOnWindows() throws Exception {
    final Path file = createTempFile("somepath");
    final MountableFile mountableFile = MountableFile.forHostPath(file.toString());
    /* taken from performChecks() */
    final String mountablePath = mountableFile.getResolvedPath();
    /* fails here */
    assertTrue("The resolved path can be found", new File(mountablePath).exists());
}

On Windows resolved path becomes mingw-fied as //c/something/filename.ext and File API does not understand it.
That results in files being copied with null-length i.e. when packing tar archive with files to copy to container. Header for file is created, but content is not written.
What's the idea behind mingw-fying file path?

PR #445 please review.

Thanks @glebsts, and sorry for not responding sooner. Re scope of testing on Windows, it's unfortunately not something we run automated tests on yet. As far as we're aware there are no cloud testing options that support use of docker on a windows host, so the only option seems to be physical hardware - which takes quite a bit more effort to set up. I need to give this some attention, though.
Will review.

Thank you @rnorth . Yes, I seem to be the only freak here with "alternative operating system", but I enjoy being a rebel. So far I have to use local patched version, as quite many changes were done :). Will make new PRs. Still not able to use docker-compose with testcontainers.
Just randomly found that in google. Not sure if free, or suits you at all, probably you've seen that.
https://stefanscherer.github.io/setup-windows-docker-ci-appveyor/

Thanks @glebsts. I'll have a look - I'm aware of at least two attempts to use Appveyor for testcontainers, but based on that post maybe they've changed their support for Docker!

@rnorth I really hope that will get some review and approval, as I would like to add another PR (about #446 ) on top of that.

@rnorth Please find the time to review this, I'm waiting for this fix too.


Get current docker IP via command line

Let's say I don't want to open a new terminal via Kitematic, but rather set the DOCKER_HOST in my current terminal.
I looked into the code but didn't found a way of doing this. Is it possible? Can we add such feature?

Just use docker-machine env dev to display the settings of your current Docker Host. Or you can use directly in your bash shell the command $(docker-machine env dev) and all settings will be done for you.
Here is a short example:
docker-machine env dev
export DOCKER_TLS_VERIFY=1
export DOCKER_CERT_PATH=/Users/dieter/.docker/machine/machines/dev
export DOCKER_HOST=tcp://<IP_ADDRESS>:2376

Nice!
Many thanks @DieterReuter !


[Guide] ES6: Compare var and let scopes. Fixes and enhancements.

Remove phantom image
Remove notes for contributors
Change explanation and hints so they don't provide the solution too early.
Links to markdown + 2 more resources

[x] I have read freeCodeCamp's contribution guidelines.
[x] My pull request has a descriptive title (not a vague title like Update index.md)
[x] My pull request targets the master branch of freeCodeCamp.
[x] None of my changes are plagiarized from another source without proper attribution.
[x] My article does not contain shortened URLs or affiliate links.

@Manish-Giri @RandellDawson How can we get this PRs ahead and merge it. Seems nothing is happening right now

@AdrianSkar @Manish-Giri @RandellDawson - This needs to get resolved since there's some larger PR's waiting on it - so lets figure out what we want. Perhaps adding a third part to the hints would help....
The log output should read...
'Block scope i is:  block scope'
'Function scope i is:  function scope'

In my opinion, it was fine how it originally was, or it's fine with these changes, I don't think there's a wrong way here... @AdrianSkar is right that, the original hints sort of gave away the solution. If I don't hear back in the near future from someone - I will probably just merge this so we can move on.

Thank you for your contribution to the page! 👍
We're happy to accept these changes, and look forward to future contributions. 🎉


Extract extension contexts to keyword

Completes #234 and task CIMPL-1163.
When processing an Extension definition, set the contexts directly on the ExportableExtension object. When doing so, convert paths back to FSH-style paths. When extracting caret value rules on an Extension definition, do not extract any rules for contexts.
Add optimizer plugin to resolve URLs in contexts.
This ended up being a bit simpler than I had anticipated in regards to differentiating between core and non-core resources. Since SUSHI does the hard work of actually resolving contexts, GoFSH gets to use the name of anything that it can fish up, no matter where it came from.

@jafeltra Thank you for the review!

That change in USCore is a SUSHI bug, similar to the one recently found for Instances of Identifier: if you look at StructureDefinitionExporter.setContext, you'll see that it fishes for extensions before any other type.
It looks like that's the default context that SUSHI sets when no Context keyword is used. There's a corresponding RemoveDefaultExtensionContextRulesOptimizer in GoFSH. This optimizer probably doesn't do much of anything anymore. So, the optimizer should change to instead check the context on each extension, and if the only context is the default, remove it so that no Context keyword is used.