workflow with "when" stuck in running after all tasks completed

Summary
What happened/what you expected to happen?
I ran the attached workflow with when.
I expect the workflow to finish successfully after all tasks are completed, but the workflow itself is stuck in running status.
The original workflow is generated by Kubeflow Pipelines so it's a little convoluted, see downstream issue https://github.com/kubeflow/pipelines/issues/5937.
I've tried my best to simplify the reproducing example and make it readable. If I remove any tasks from the dag, the problem will no longer be reproduced.
Diagnostics
👀 Yes! We need all of your diagnostics, please make sure you add it all, otherwise we'll go around in circles asking you for it:
What Kubernetes provider are you using?
GKE
What version of Argo Workflows are you running?
v3.1.0
What executor are you running? Docker/K8SAPI/Kubelet/PNS/Emissary
Docker
Did this work in a previous version? I.e. is it a regression?
Yes, this worked in v2.12.9.
Workflow with status:
apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  creationTimestamp: "2021-06-30T02:36:28Z"
  generateName: condition-
  generation: 4
  labels:
    workflows.argoproj.io/phase: Running
  name: condition-8hhhs
  namespace: kubeflow
  resourceVersion: "31048360"
  selfLink: /apis/argoproj.io/v1alpha1/namespaces/kubeflow/workflows/condition-8hhhs
  uid: 5dfc75e2-c942-449d-a718-ff9dab6db4e5
spec:
  arguments: {}
  entrypoint: single-condition-pipeline
  serviceAccountName: pipeline-runner
  templates:
  - dag:
      tasks:
      - arguments:
          parameters:
          - name: text
            value: '{{inputs.parameters.flip_result}}'
        name: print
        template: print
    inputs:
      parameters:
      - name: flip_result
    metadata: {}
    name: condition
    outputs: {}
  - container:
      args:
      - |
        mkdir -p "$(dirname $1)"
        echo $0 > $1
      - heads
      - /tmp/outputs/flip_result/data
      command:
      - sh
      - -c
      image: alpine
      name: ""
      resources: {}
    inputs: {}
    metadata: {}
    name: flip
    outputs:
      artifacts:
      - name: result
        path: /tmp/outputs/flip_result/data
      parameters:
      - name: result
        valueFrom:
          path: /tmp/outputs/flip_result/data
  - container:
      args:
      - echo $0
      - '{{inputs.parameters.text}}'
      command:
      - sh
      - -c
      image: alpine
      name: ""
      resources: {}
    inputs:
      parameters:
      - name: text
    metadata: {}
    name: print
    outputs: {}
  - dag:
      tasks:
      - arguments:
          parameters:
          - name: flip_result
            value: '{{tasks.flip.outputs.parameters.result}}'
        dependencies:
        - flip
        name: condition
        template: condition
        when: '"{{tasks.flip.outputs.parameters.result}}" == "heads"'
      - arguments: {}
        name: flip
        template: flip
      - arguments:
          parameters:
          - name: text
            value: '{{tasks.flip.outputs.parameters.result}}'
        dependencies:
        - flip
        name: print
        template: print
    inputs: {}
    metadata: {}
    name: single-condition-pipeline
    outputs: {}
status:
  artifactRepositoryRef:
    default: true
  conditions:
  - status: "False"
    type: PodRunning
  finishedAt: null
  nodes:
    condition-8hhhs:
      children:
      - condition-8hhhs-3281584603
      displayName: condition-8hhhs
      finishedAt: null
      id: condition-8hhhs
      name: condition-8hhhs
      phase: Running
      progress: 2/2
      startedAt: "2021-06-30T02:36:28Z"
      templateName: single-condition-pipeline
      templateScope: local/condition-8hhhs
      type: DAG
    condition-8hhhs-1189854215:
      boundaryID: condition-8hhhs
      displayName: print
      finishedAt: "2021-06-30T02:36:41Z"
      id: condition-8hhhs-1189854215
      inputs:
        parameters:
        - name: text
          value: heads
      name: condition-8hhhs.print
      outputs:
        artifacts:
        - name: main-logs
          s3:
            key: artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-1189854215/main.log
        exitCode: "0"
      phase: Succeeded
      progress: 1/1
      resourcesDuration:
        cpu: 2
        memory: 0
      startedAt: "2021-06-30T02:36:38Z"
      templateName: print
      templateScope: local/condition-8hhhs
      type: Pod
    condition-8hhhs-1221011148:
      boundaryID: condition-8hhhs-2253232213
      displayName: print
      finishedAt: "2021-06-30T02:36:41Z"
      id: condition-8hhhs-1221011148
      inputs:
        parameters:
        - name: text
          value: heads
      name: condition-8hhhs.condition.print
      outputs:
        artifacts:
        - name: main-logs
          s3:
            key: artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-1221011148/main.log
        exitCode: "0"
      phase: Succeeded
      progress: 1/1
      resourcesDuration:
        cpu: 2
        memory: 0
      startedAt: "2021-06-30T02:36:38Z"
      templateName: print
      templateScope: local/condition-8hhhs
      type: Pod
    condition-8hhhs-2253232213:
      boundaryID: condition-8hhhs
      children:
      - condition-8hhhs-1221011148
      displayName: condition
      finishedAt: "2021-06-30T02:36:48Z"
      id: condition-8hhhs-2253232213
      inputs:
        parameters:
        - name: flip_result
          value: heads
      name: condition-8hhhs.condition
      outboundNodes:
      - condition-8hhhs-1221011148
      phase: Succeeded
      progress: 1/1
      resourcesDuration:
        cpu: 2
        memory: 0
      startedAt: "2021-06-30T02:36:38Z"
      templateName: condition
      templateScope: local/condition-8hhhs
      type: DAG
    condition-8hhhs-3281584603:
      boundaryID: condition-8hhhs
      children:
      - condition-8hhhs-2253232213
      displayName: flip
      finishedAt: "2021-06-30T02:36:31Z"
      id: condition-8hhhs-3281584603
      name: condition-8hhhs.flip
      outputs:
        artifacts:
        - name: result
          path: /tmp/outputs/flip_result/data
          s3:
            key: artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-3281584603/result.tgz
        - name: main-logs
          s3:
            key: artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-3281584603/main.log
        exitCode: "0"
        parameters:
        - name: result
          value: heads
          valueFrom:
            path: /tmp/outputs/flip_result/data
      phase: Succeeded
      progress: 1/1
      resourcesDuration:
        cpu: 2
        memory: 0
      startedAt: "2021-06-30T02:36:28Z"
      templateName: flip
      templateScope: local/condition-8hhhs
      type: Pod
  phase: Running
  progress: 3/3
  resourcesDuration:
    cpu: 6
    memory: 0
  startedAt: "2021-06-30T02:36:28Z"

Workflow controller logs:
time="2021-06-30T02:36:28.233Z" level=info msg="Processing workflow" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.240Z" level=info msg="Updated phase  -> Running" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.240Z" level=info msg="DAG node condition-8hhhs initialized Running" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.240Z" level=info msg="All of node condition-8hhhs.flip dependencies [] completed" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.240Z" level=info msg="Pod node condition-8hhhs-3281584603 initialized Pending" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.264Z" level=info msg="Created pod: condition-8hhhs.flip (condition-8hhhs-3281584603)" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:28.286Z" level=info msg="Workflow update successful" namespace=kubeflow phase=Running resourceVersion=31048210 workflow=condition-8hhhs
time="2021-06-30T02:36:38.267Z" level=info msg="Processing workflow" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.267Z" level=info msg="Updating node condition-8hhhs-3281584603 exit code 0" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.267Z" level=info msg="Setting node condition-8hhhs-3281584603 outputs: {\"parameters\":[{\"name\":\"result\",\"value\":\"heads\",\"valueFrom\":{\"path\":\"/tmp/outputs/flip_result/data\"}}],\"artifacts\":[{\"name\":\"result\",\"path\":\"/tmp/outputs/flip_result/data\",\"s3\":{\"key\":\"artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-3281584603/result.tgz\"}},{\"name\":\"main-logs\",\"s3\":{\"key\":\"artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-3281584603/main.log\"}}]}" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.267Z" level=info msg="Updating node condition-8hhhs-3281584603 status Pending -> Succeeded" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.268Z" level=info msg="All of node condition-8hhhs.condition dependencies [flip] completed" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.268Z" level=info msg="DAG node condition-8hhhs-2253232213 initialized Running" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.268Z" level=info msg="All of node condition-8hhhs.condition.print dependencies [] completed" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.268Z" level=info msg="Pod node condition-8hhhs-1221011148 initialized Pending" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.285Z" level=info msg="Created pod: condition-8hhhs.condition.print (condition-8hhhs-1221011148)" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.285Z" level=info msg="All of node condition-8hhhs.print dependencies [flip] completed" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.285Z" level=info msg="Pod node condition-8hhhs-1189854215 initialized Pending" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.302Z" level=info msg="Created pod: condition-8hhhs.print (condition-8hhhs-1189854215)" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:38.329Z" level=info msg="Workflow update successful" namespace=kubeflow phase=Running resourceVersion=31048285 workflow=condition-8hhhs
time="2021-06-30T02:36:38.336Z" level=info msg="cleaning up pod" action=labelPodCompleted key=kubeflow/condition-8hhhs-3281584603/labelPodCompleted
time="2021-06-30T02:36:48.284Z" level=info msg="Processing workflow" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Updating node condition-8hhhs-1221011148 exit code 0" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Setting node condition-8hhhs-1221011148 outputs: {\"artifacts\":[{\"name\":\"main-logs\",\"s3\":{\"key\":\"artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-1221011148/main.log\"}}]}" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Updating node condition-8hhhs-1221011148 status Pending -> Succeeded" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Updating node condition-8hhhs-1189854215 exit code 0" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Setting node condition-8hhhs-1189854215 outputs: {\"artifacts\":[{\"name\":\"main-logs\",\"s3\":{\"key\":\"artifacts/condition-8hhhs/2021/06/30/condition-8hhhs-1189854215/main.log\"}}]}" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.285Z" level=info msg="Updating node condition-8hhhs-1189854215 status Pending -> Succeeded" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.286Z" level=info msg="Outbound nodes of condition-8hhhs-2253232213 set to [condition-8hhhs-1221011148]" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.286Z" level=info msg="node condition-8hhhs-2253232213 phase Running -> Succeeded" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.286Z" level=info msg="node condition-8hhhs-2253232213 finished: 2021-06-30 02:36:48.286139773 +0000 UTC" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.286Z" level=info msg="Checking daemoned children of condition-8hhhs-2253232213" namespace=kubeflow workflow=condition-8hhhs
time="2021-06-30T02:36:48.320Z" level=info msg="Workflow update successful" namespace=kubeflow phase=Running resourceVersion=31048360 workflow=condition-8hhhs
time="2021-06-30T02:36:48.328Z" level=info msg="cleaning up pod" action=labelPodCompleted key=kubeflow/condition-8hhhs-1221011148/labelPodCompleted
time="2021-06-30T02:36:48.328Z" level=info msg="cleaning up pod" action=labelPodCompleted key=kubeflow/condition-8hhhs-1189854215/labelPodCompleted
time="2021-06-30T02:36:58.322Z" level=info msg="Processing workflow" namespace=kubeflow workflow=condition-8hhhs

Message from the maintainers:
Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

Verified this has been fixed in v3.1.1


Remove 3rd party URT license

Removing broken link to 3rd party license for URT. This was an internal component that briefly lived in github but has since moved internal.

Thanks @kkrum. I went ahead and merged this.


File Seller Plugin downloading txt file instead of actual file

I set up this POS app to make my 3D-files available for Sats. When the download link which is generated after the payment is clicked, the download begins, but not with the intended file name or extension.
Depending on the browser, different things happen:
Chrome based browsers download a .txt file which's name is the internal File Id, not the actual file name.
Firefox, opens prompt to save a file which's name is the file id without a file extension.
Example:
This link: https://pay.fomozon.com/LocalStoragetmp/a12b65fc-37f2-468f-9569-6a5752c1a285?download should download "NerdMiner_modified_GoBrrr.zip", but it downloads a12b65fc-37f2-468f-9569-6a5752c1a285.txt in chrome.

Thank you!!


Inconsistent Display of attributes in CA Insights workbook.

In the Conditional Access Insights and reporting workbook, sometimes the UPN is shown as a GUID, other times it shows the actual user name, it should never show the GUID

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: eb2155a4-d8b2-a542-1aba-387183c3f2e3
Version Independent ID: 11565414-4093-d4cf-2665-1343b45a732a
Content: Azure Monitor workbooks for reports
Content Source: articles/active-directory/reports-monitoring/howto-use-azure-monitor-workbooks.md
Service: active-directory
Sub-service: report-monitor
GitHub Login: @MarkusVi
Microsoft Alias: markvi

@DeanGross Thank you for your feedback. We will investigate and update the thread further.

@DeanGross  Azure Monitor just displays the information that is logged in the sign-in activities. This depends on the source/application sending the information which gets logged into the sign-in activities.


Configure Azure Pipelines

This pipeline configuration builds the application and produces a build for the Release configuration.

Done merged


Fix product availability logic for discounted products

Current coverage is 63.63% (diff: 100%)

Merging #677 into master will not change coverage

@@             master       #677   diff @@
==========================================
  Files            91         91
  Lines          4295       4295
  Methods           0          0
  Messages          0          0
  Branches        462        462
==========================================
  Hits           2733       2733
  Misses         1468       1468
  Partials         94         94

Powered by Codecov. Last update c4e3c9b...eaaa330


Vaadin 24 update

Update to latest Vaadin 24.4. there is Vaadin 24.5.13 just coming out soon and I can update to that when it comes out of beta.

Hey @ amithkoujalgi, do you think these updates are useful? I have some further contributions I can do, if you see them helping.

Hey @samie. Apologies for the delay in my response. Thanks a lot for the PR and yes, your contributions are incredibly helpful, and I truly appreciate them. Please feel free to continue contributing.
Thank you very much!


fix: aws proxy missing subnet

Describe your changes
resource for aws autostopping proxy is missing subnet_id. this is needed to place the proxy in a specific subnet. seems to choose at random otherwise.
test:
  # harness_autostopping_aws_proxy.snyder_lab will be created
  + resource "harness_autostopping_aws_proxy" "snyder_lab" {
      + allocate_static_ip                = true
      + api_key                           = (sensitive value)
      + cloud_connector_id                = "rileyharnessccm"
      + delete_cloud_resources_on_destroy = true
      + host_name                         = "<IP_ADDRESS>.nip.io"
      + id                                = (known after apply)
      + identifier                        = (known after apply)
      + keypair                           = "riley"
      + machine_type                      = "t3.micro"
      + name                              = "snyder-lab"
      + region                            = "us-west-2"
      + security_groups                   = [
          + "sg-040bd76d805fb4e2e",
        ]
      + subnet_id                         = "subnet-0d91195f6cc504f8e"
      + vpc                               = "vpc-0e2b5e811d1ce6767"
    }

Comment Triggers

PR Check triggers

Build: trigger build
Sub Category Field Check: trigger subcategoryfieldcheck
gitleaks: trigger gitleaks

 Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

This pull request has been automatically closed due to 30 days of inactivity. If you believe this was done in error, please feel free to reopen the PR or reach out to the BT team for further assistance. Thank you for your contributions!


ui-select for angularjs does not wrap

Hi
We are using the muiltiple select option. We are able to select multiple options. However once the entries reach the end of the line the selected entries overflow the right border. I read on stack overflow that setting the white-space: normal in custom.css should cause the entries to wrap. However this did not work.
How should I do to get ui-select to wrap entries. Is there a directive that I can use ? Please let me know
what I should do

Dupe #1278


[Bug]: Unresponsive Interests navigation button

Is there an existing issue for this?

[X] I have searched the existing issues

Is there a StackOverflow question about this issue?

[X] I have searched StackOverflow

What happened?
When navigating to either a AuthorDestination or TopicDestination, the NavigationBar's NavigationBarItem corresponding to InterestsDestination becomes unselected and clicking on it does nothing.
It seems that the NavDestination's hierarchy is not properly configured.
https://user-images.githubusercontent.com/1921278/168175101-d68877b5-3fa5-4f38-803e-5269c381d381.mp4
Relevant logcat output
No response
Code of Conduct

[X] I agree to follow this project's Code of Conduct

Can I raise a PR for this Issue?

Our designs specify that the bottom bar should always show, also when navigating to a topic or author. Can you try to fix the bug of keeping the right tab selected instead?

@JolandaVerhoef I got it, I would edit the PR with the changes as suggested by you. Thank You!

Our designs specify that the bottom bar should always show, also when navigating to a topic or author. Can you try to fix the bug of keeping the right tab selected instead?

@JolandaVerhoef I want to work on this issue. should I create a new issue about that or you can reopen this issue?

This was fixed in the latest set of changes, thanks for raising it!


Mcflugen/fix rtfd builds

@mcflugen this is great! I'm not sure exactly how you fixed the problems... other than the  sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir)))? and the two style-lines you commented out, I had some combination of your changes at some point of time.
If you have any ideas, let me know since I'd like to understand how it actually works...
Importantly the autodoc_mock_imports still works, so perhaps that will work for landlab.

I should make sure I'm using the same build settings in RTD as you are:
I have Python 3.x
Give the virtualenv access to global site packages
Install project inside of virutalenv
and use requirements.txt


Returning the latest resource from the Delete() call

Related issue: https://github.com/aws-controllers-k8s/community/issues/836
This PR contains code changes to patch custom resource with latest details during long running delete.
It does so if resource manager's Delete function return requeue related errors.
The expectation is that resource manager's ReadOne method would set appropriate conditions (example: ACK.ResourceSynced to false with message) and provide latest details about the resource which is being deleted.
Conditions can also be set at some common place where common conditions logic should reside.
Testing:
make test passed for runtime.
Alternate approach that was considered, but was not taken up as it involved non-intrusive changes to resource manager's APIs. Following are the details:
Currently, resource manager's Delete API returns only one data field of type error, it does not return resource.
Also, the sdk.go::sdkDelete() logic does not parse the return result output of service delete API.
It can be updated such that sdk.go::sdkDelete() logic parses the output response, and allows a sdk_delete_post_set_output hook for service controllers to have custom logic (as needed). The resultant resource reference would then be returned from this method.
In case, requeue is required then the return values from this method will have both resource, error (requeue type) as not nil.
And the reconciler:cleanup() logic here, would patch the current resource with the returned resource from rm.Delete() call and return the error (if any).
Though this change is similar to the approach taken for resource manager ReadOne here, this is more intrusive as it changes the Delete API's return type.
However, it does have advantage that there is no need for another ReadOne call from reconciler, and also provides flexibility to service controller's to implement custom hook specific to delete scenario.
[EDIT]: the alternate approach that is mentioned above, is the final approach taken per the discussion/comments in this PR.
Related PR in code-generator: https://github.com/aws-controllers-k8s/code-generator/pull/114

@kumargauravsharma if a call to DeleteReplicationGroup returns success, can the ReplicationGroup ever end up in any state other than deleted?

@jaypipes no error from DeleteReplicationGroup API call set the replication group state as 'deleting'. It may take some time for it to get deleted, meanwhile the resource describe api does return the resource details as delete progresses.

@jaypipes no error from DeleteReplicationGroup API call set the replication group state as 'deleting'. It may take some time for it to get deleted, meanwhile the resource describe api does return the resource details as delete progresses.

(apologies for the late reply, I'm on PTO this week...)
@kumargauravsharma ok, thanks for the info. I just want to make sure I'm understanding your use cases clearly...you want this functionality in order to match the Kubernetes API experience to the Elasticache API experience: that the replication group resource continues to appear in a DescribeReplicationGroup API call -- and the ReplicationGroup CR will continue to appear in calls to kubectl describe replicationgroups/{name} with a Status.Status == deleting -- even after a DeleteReplicationGroup API call returns no error and the replication group will only ever transition to a deleted state (and not appear in a DescribeReplicationGroup API call). This is about allowing the ReplicationGroup CR to exist for that period of time while it is in progress of being deleted, yes?

@jaypipes no error from DeleteReplicationGroup API call set the replication group state as 'deleting'. It may take some time for it to get deleted, meanwhile the resource describe api does return the resource details as delete progresses.

(apologies for the late reply, I'm on PTO this week...)
@kumargauravsharma ok, thanks for the info. I just want to make sure I'm understanding your use cases clearly...you want this functionality in order to match the Kubernetes API experience to the Elasticache API experience: that the replication group resource continues to appear in a DescribeReplicationGroup API call -- and the ReplicationGroup CR will continue to appear in calls to kubectl describe replicationgroups/{name} with a Status.Status == deleting -- even after a DeleteReplicationGroup API call returns no error and the replication group will only ever transition to a deleted state (and not appear in a DescribeReplicationGroup API call). This is about allowing the ReplicationGroup CR to exist for that period of time while it is in progress of being deleted, yes?

The PR: https://github.com/aws-controllers-k8s/elasticache-controller/pull/34 takes care of ensuring that the ReplicationGroup CR exists for the period of time while delete is in progress.
However, the ReplicationGroup CR becomes out of sync from its latest state on service side. Thus, this PR #21 was to fix that behavior of reconciler.
nit: once the ReplicationGroup gets deleted on service side, the sdkFind returns not found exception, as there is no ' deleted' status.

/lgtm


WIP: Add documentation.

A preview is available by visiting the following address:
https://icefall.readthedocs.io/en/latest/

Screenshots of the documentation:

That looks great!
The colab notebook is a nice touch.

That looks great!
The colab notebook is a nice touch.

Shall we choose a dedicated place to save such notebooks, say, a GitHub repo?

Regarding the colab notebooks: I'm OK with that as long as it doesn't make it harder to get things done... but maybe not a short-term/urgent priority?

Regarding the colab notebooks: I'm OK with that as long as it doesn't make it harder to get things done... but maybe not a short-term/urgent priority?

Yeah, it's not urgent. If later we have lots of notebooks, we can start to find a place to put them.

Merging so that @pkufool can also work on this.


[Gradle] Add Support for init.gradle Configuration

Issue:
In some cases, it is necessary to configure repositories, including mirrors or internal enterprise sources. Typically, an init.gradle file is created in the project's root directory to handle these configurations. However, I've noticed that the current support for this configuration in cdxgen is not very robust.
Steps to Reproduce:

Use the project link: https://github.com/pbadenski/call-hierarchy-printer

Review the build.gradle file of the project, which contains the following repository settings:

repositories {
    maven { url "https://repo.maven.apache.org/maven2" }
}

The above link is no longer valid and can be considered as a private source to simulate this scenario I mentioned. Creating an init.gradle file and set up a new source within it. Now, it is normal to generate the dependency tree using the following command:
gradlew -I init.gradle dependencies

The command above successfully generates the dependency tree. However, when attempting to generate it using cdxgen, an error is encountered. The error screenshot is attached below.

Expected Outcome:
I expect cdxgen to fully support the custom source configurations defined in the init.gradle file. This would enable accurate generation of dependency reports.

@Moujuruo did you try setting the environment variables such as GRADLE_ARGS?

@Moujuruo, if the init.gradle file gets copied to the $HOME/.gradle directory, it is getting picked up by gradle automatically, thus requiring no workaround in cdxgen. However, I see a different error related to the use of --console argument, which is not supported by the version of the gradle wrapper used in this repo. Since it's quite old, I will treat this as won't fix unless you think otherwise.


Issue with "reading choices"

Cannot read properties of null (reading 'choices').
Using GPT model 4o


[BACKPORT #1939 #2042 #2061 #2204] smp/irq fixes and improvements

Summary
This backports 4 patches:

#1939 -- Add cxd56xx intstack for SMP  (required to cleanly apply other patches)
#2042 -- Fix interrupt stack handling for ARM SMP
#2061 -- Refactor arm interrupt stack related code
#2204 -- sched: irq: Fix enter_critical_section() in an irq handler for SMP

@masayuki2009 I'll backport #2309 after we have these merged in so that the backport patch will apply cleanly.


same name with factory method

Hi,
I'm using a factory method that should return a function with different names for each call. Do you know why this mocha test case doesn't run as expected ?
var set = require('function-name');
var assert = require('assert');

describe('function-name', function () {

	it(" should set different names on factory", function(done){

		var f1 = null;
		var f2 = null;

		var factory = function(){
			var Y = function test(){};
			set(Y, Math.random());
			return Y;
		}

		f1 = factory();
		f2 = factory();

		assert.notEqual(f1.name, f2.name);
		done();
	});

});

// Result : AssertionError: "0.743471089284867" != "0.743471089284867"

It works fine with Function.create but I'm not sure if it's enough secure as node-function-name. Any comment will be appreciated.
Thanks!

This is caused by a what we could call a "bug" in V8 function cache.
Function.prototype.name is defined as follow(simplified):
v8::String v8::Function::GetName() {
    return reinterpret_cast<i::JSFunction>(Utils::OpenHandle(this))->shared()->name();
}

When multiple functions with the same body are found, they share the same structure in V8 heap. The problem is that the function name is within that structure(SharedFunctionInfo) and not with the JSFunction itself.
Basically, when you change one function name you end up changing all functions that shares the same code name.
I could assume that this is a won't fix until Harmony function name draft is implemented(http://wiki.ecmascript.org/doku.php?id=harmony:function_name_property).

I ended up figuring out a pretty nasty workaround for this:
var crypto = require('crypto');
var assert = require('assert');
var set = require('function-name');

function c (p) {
  var name;
  do {
    name = crypto.randomBytes(100).toString('hex');
  } while (/\d/.test(name[0]));

  return new Function('p',
    'return function ' + name + '(){' +
      'return p.apply(this,arguments);' +
    '};')(p);
}

var f1 = null;
var f2 = null;

var factory = function(){
  var Y = c(function test(){});
  set(Y, Math.random());
  return Y;
}

f1 = factory();
f2 = factory();

assert.notEqual(f1.name, f2.name);

The trick is that we have to generate a proxy function with a unique named function. I'm using crypto.randomBytes(100).toString('hex') here to create a "unique" name, but there are better ways of doing this.
Again, this is extremely nasty, and doesn't really make a lot of sense. But it works if you're in a pinch and really, really NEED to create name properties that otherwise would be syntax errors. Otherwise Function.create() almost does the job perfectly.


Memcached expire cannot exceed 30 days

According to PHP manual Memcached expire cannot be more than 30 days. (http://php.net/manual/en/memcache.set.php).
If it does, the value is not set but Memcached ext. returns true!
I suggest in the \yii\caching\MemCache @ setValue(...) to limit the duration to 30 days (or throw some exception or at least a warning in the logs).

Does it apply to memcache only or to memcached as well? These are two separate extensions.

According to this http://php.net/manual/ru/memcache.set.php for Memcache we have the same thing.

expire
Expiration time of the item. If it's equal to zero, the item will never expire. You can also use Unix timestamp or a number of seconds starting from current time, but in the latter case the number of seconds may not exceed 2592000 (30 days).

OK. Makes sense to throw an exception or at least to write something to logs.

What do you think is better?

Perhaps it is better to truncate expiration value to 30 days silently.
Cache is an unreliable storage by definition, so I supose such solution is acceptable.

Probably yes since it is very unlikely that exception will be handled.

The only thing that would not break backward compatibility would be to silently override it and set it to 30 days.
And if we add appropriate comments there then any developer not familiar with the limitation would very easily discover why his cache is not really set.
Ideally however, a warning in the logs in debug mode would be the best - makes it easier to spot on staging environments.

Done as you discussed. Looking forward for feedback :wink:


Write failure and 'characteristic not found' on android

Describe the bug
I have written an ionic framework app that uses capacitor-community/bluetooth-le to read and write from a device.  It works fine on ios. On android it will read all characteristics successfully.  However, some writes will fail, and then after that the same reads that were previously working fail with the error 'Characteristic not found'.  I know the characteristic exists and works because they previously work, and work fine on ios.  Once in this state, the only thing I've found that can resolve is to "Forget device" in android BT settings, then re-pair and try again.
To Reproduce
Reproducing the behavior will be difficult as the device is a custom device.  I'm hoping the logs and code below will provide enough context to help identify the issue.  When looking at the logs, note there is an authorization step where the user may be prompted to enter a password to a characteristic on the device before it lets them write to the protected characteristics.
Expected behavior
Expected behavior is for Android app to work as well as the ios app.  I need to know why the write is failing on android, when the same code works as expected on ios, AND how to prevent it from getting stuck in the 'Characteristic not found' state in the event of future failed writes.
Screenshots
NA
Plugin version:

@capacitor-community/bluetooth-le: 1.8.2

Smartphone (please complete the following information):

Device: Nexus 6
OS: Android 7.1.1
Browser: na/chrome
Version 7.1.1

Additional context
bleissue.txt

Thank you for using this plugin.
Unfortunately, I don't understand what's going on from the logs.
Have you tried with several Android phones?
If there is an authorization step involved, your can try using the explicit methods createBond and isBonded (on Android only).

After testing with a newer device, I'm pretty sure this was caused by flaky bluetooth on the Nexus 6.  Same code works fine on Pixel 5 with newer android.

Good to know. Thanks for reporting back.


WIP

For #

[ ] Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR).
[ ] Title summarizes what is changing.
[ ] Has a news entry file (remember to thank yourself!).
[ ] Appropriate comments and documentation strings in the code.
[ ] Has sufficient logging.
[ ] Has telemetry for enhancements.
[ ] Unit tests & system/integration tests are added/updated.
[ ] Test plan is updated as appropriate.
[ ] package-lock.json has been regenerated by running npm install (if dependencies have changed).

Codecov Report

Merging #8522 (7f52da7) into main (9fc3c2e) will decrease coverage by 0%.
The diff coverage is n/a.

:exclamation: Current head 7f52da7 differs from pull request most recent head 5b8c993. Consider uploading reports for the commit 5b8c993 to get more accurate results

@@          Coverage Diff          @@
##            main   #8522   +/-   ##
=====================================
- Coverage     72%     71%   -1%
=====================================
  Files        378     378
  Lines      23985   23985
  Branches    3692    3692
=====================================
- Hits       17277   17260   -17
- Misses      5216    5230   +14
- Partials    1492    1495    +3

Impacted Files
Coverage Δ

...ience/variablesView/variableViewMessageListener.ts
77% <0%> (-23%)
:arrow_down:

...ent/common/application/webviewViews/webviewView.ts
67% <0%> (-11%)
:arrow_down:

src/client/datascience/dataScienceSurveyBanner.ts
54% <0%> (-9%)
:arrow_down:

...client/datascience/notebook/cellLanguageService.ts
71% <0%> (-5%)
:arrow_down:

...tascience/jupyter/kernels/kernelCommandListener.ts
67% <0%> (-4%)
:arrow_down:

...lient/datascience/variablesView/notebookWatcher.ts
90% <0%> (-3%)
:arrow_down:

src/client/datascience/raw-kernel/rawSocket.ts
82% <0%> (-2%)
:arrow_down:

...t/datascience/notebook/vscodeNotebookController.ts
78% <0%> (-1%)
:arrow_down:

src/client/debugger/jupyter/kernelDebugAdapter.ts
77% <0%> (+<1%)
:arrow_up:

src/client/common/utils/decorators.ts
80% <0%> (+1%)
:arrow_up:

... and 2 more


Create tasks_taxonomy

wip..
taxonomy definition

We differentiate open QA from multiple choice but not abstractive and extractive and not regular from multihop? (This makes sense to me, as the templates might be the same. So the distinction just helps finding the task but have little practical value) Also closed book must have their own sub category. What about no answer? It is probably shown in the templates, but should it define the category somehow? Same for summarization abstractive and extractive? Summarization as you know is just very ill-defined but a lot could be changed there as well (headline generation simplification etc.). what unifies the different text generations? What is MT and summarization not under it?

thanks @borgr , will try to address some of the issues you raised..

indeed "no answer" is somehow encoded in some of the templates. If you think it deserves a category by itself, we should consider.
for summarization i think that extractive is (almost) no longer in the game... and thus wonder if we should create such a sub-category
MT- machine translation
about text generation.. i agree that another taxonomy could be NLG, NLU and then to put under NLG summarization.. but again we need to stick to something..

i did few more changes. @borgr please take a looks and feel free to modify

Can we close this?

Closing as I assume it's no longer needed.


Extend PlaceholderAPI and Arguments support.

Add PlaceholderAPI placeholders and DeluxeMenus arguments support to other options:

[ ] Slots
[ ] to be determined

It's missing in CustomModelData too

It's missing in CustomModelData too

It was added in release 1.14.0

I didn't see that, perfect


Not listed on Cocoapods.org?

I can't see this on Cocoapods.org search results. http://cocoapods.org/?q=foursquarekit I don't know why. Would you please take a look? I'd love to use this.

Please do!

+1 to the above. Would love to use this.

+1

+1000, please add it to cocoapods master repo.
Now I'm using this way of installing FourSquareKit, but I wish i were able to use it without setting path to git repo.
pod 'FourSquareKit', :git => 'https://github.com/rexstjohn/FourSquareKit.git', :tag => '0.0.4'


Read build_tools from .app data and show more metadata

Read from the build_tools key, if it's present, or default to
[<<"rebar3">>] if it's not.
Print the description, maintainers, licenses, links, and build tools
when prompting the user about publishing the package.
Closes #36

I added the printing because I recently published a package (https://hex.pm/packages/dogstatsde) and it doesn't have any build tools listed. I can see, though, that <<"rebar3">> is being sent, so I'm not sure if that's a Hex problem.

Great, thanks!


Only calculate directions for changed blocks && followers

Fixes https://github.com/facebook/draft-js/issues/146, a significant lag when editing large documents, caused by every keystroke triggering a full re-calculation of the direction of every block.
It now only calculates direction for blocks that have changed or are right after a block whose direction changed.

Hey, just wanted to check whether you still plan to update this. :)

Also interested on the outcome of this PR.

Sorry @hellendag ! I've busy with other stuff and didn't see your comment. I do hope to get something back in about 2 weeks.

It looks like this pull request has been abandoned so I am going to close it. Please feel free to reopen if you are still planning on working on this!


Provide new authentication features: Environment variable authentication

Is your feature request related to a problem? Please describe.
We are currently building more powerful authentication features, but we have to keep the ability to store user names and passwords in clear text to not break existing pipelines running these tests.
Describe the solution you'd like
It should still work as it does today, but the diagnostics generated should preferably contain a message dissuading people from using this option.
Describe alternatives you've considered
The alternative is not supporting this at all. But since we have it, we should wire it in for now, and build a plan for subsequent obsolescence.
Additional context?
This is not safe, since clear text user names and passwords need to be known by users of the tool.

2 days


Issue running Finder

Hello, bellow i post the output of the run with finder pipeline. It looks like some alignment files that should be produced by Finder are not produced, STAR aligner is installed of course and working, I also installed psiclass, although i do not know if it was necessary, genemark is in etc... But of course, it doesnt go as far, this are the initial steps.
So why this happens?
cp: '/home/orestis/../x_soft_all.fasta' and '/home/orestis/../iquitos_soft_all.fasta' are the same file
cat: /home/orestis/../alignments/RNA_all_round1_SJ.out.tab: No such file or directory
cat: /home/orestis/../alignments/cacao_RNA_all_round2_SJ.out.tab: No such file or directory
mv: cannot stat '/home/orestis/../alignments/RNA_all_final_Log.final.out': No such file or directory
cat: /home/orestis/../alignments/RNA_all_round3_SJ.out.tab: No such file or directory
samtools index: "/home/orestis/../alignments/RNA_all_final.sortedByCoord.out.bam" is in a format that cannot be usefully indexed
samtools index: "/home/orestis/../alignments/RNA_all_final.sortedByCoord.out.bam" is in a format that cannot be usefully indexed
[bam_header_read] EOF marker is absent. The input is probably truncated.
[bam_header_read] invalid BAM binary header (this is not a BAM file).
[bam_header_read] EOF marker is absent. The input is probably truncated.
[bam_header_read] invalid BAM binary header (this is not a BAM file).
Can not open /home/orestis/../alignments/RNA_all_final.sortedByCoord.out.bam.
[main_samview] fail to read the header from "/home/orestis/../alignments/RNA_all_final.sortedByCoord.out.bam".
[main_samview] fail to read the header from "/home/orestis/../alignments/RNA_all_for_psiclass.sam".
mv: cannot stat '/home/orestis/../assemblies_psiclass_modified/combined/psiclass_output_sample_0.gtf': No such file or directory
mv: cannot stat '/home/orestis/../assemblies_psiclass_modified/combined/psiclass_output_vote.gtf': No such file or directory
Traceback (most recent call last):
File "/home/orestis/FINDER/finder_v1.1.0/finder", line 688, in
main()
File "/home/orestis/FINDER/finder_v1.1.0/finder", line 649, in main
orchestrateGeneModelPrediction( options, logger_proxy, logging_mutex )
File "/home/orestis/FINDER/finder_v1.1.0/finder", line 461, in orchestrateGeneModelPrediction
findTranscriptsInEachSampleNotReportedInCombinedAnnotations( options, logger_proxy, logging_mutex )
File "/home/orestis/FINDER/finder_v1.1.0/scripts/findTranscriptsInEachSampleNotReportedInCombinedAnnotations.py", line 17, in findTranscriptsInEachSampleNotReportedInCombinedAnnotations
combined_transcript_info = readAllTranscriptsFromGTFFileInParallel( [combined_gtf_filename, "combined", "combined"] )[0]
File "/home/orestis/FINDER/finder_v1.1.0/scripts/fileReadWriteOperations.py", line 290, in readAllTranscriptsFromGTFFileInParallel
fhr = open( gtf_filename, "r" )
FileNotFoundError: [Errno 2] No such file or directory: '/home/orestis/../assemblies_psiclass_modified/combined/combined.gtf'

Hello @nousiaso,
Thank you so much for your interest in finder. Are you running it within the docker container? Also, could you please provide me with your metadata file? That will help me zero in on the problem. You can email it to me at<EMAIL_ADDRESS>if you do not wish to post it here.
Thank you.

Hello @sagnikbanerjee15
Thank you for the quick reply, I am not running docker, I am running a local installation. I will email you the metadata file.
Best

I appear to be having a similar issue using the Docker container. The STAR alignment step is generating empty files and it's not clear why (I don't believe the input files are actually being mapped/read - the process is taking seconds despite what the output below suggests ...).  Indexing of the genome seems successful. Greatly appreciate any assistance in troubleshooting this.
The input file locations/names seem to be correctly referred to from the metadata?
e.g:
BioProject	SRA Accession	Tissues	Description	Date	Read Length (bp)	Ended	RNA Seq	process	Location
Oppia	R2100233S1	Control	PCR;Illumina NextSeq 550;ribodepleted;soil-OECD	na	100	PE	1	1	/RAID/data/Analysis/Data/MitesWork/onitens-assembly-2022/fastq/ribodepletion
Oppia	R2100234S2	Control	PCR;Illumina NextSeq 550;ribodepleted;soil-2	na	100	PE	1	1	/RAID/data/Analysis/Data/MitesWork/onitens-assembly-2022/fastq/ribodepletion
Files are labeled as R2100234S2_1.fastq and R2100234S2_2.fastq for PE reads (gunzipped files in case compression was an issue as the input filenames were missing the .gz extensions when they were compressed).
The error log refers to expected files that were not generated:
cat: /RAID/data/Analysis/Data/MitesWork/onitens-assembly-2022/Finder/Finder-finder_v1.1.0/Finder_Oppia/alignments/R2100233S1_round1_SJ.out.tab: No such file or directory
Section of the progress.log file:
2022-05-06 01:07:23,037 - finder -   INFO - STAR Round2 run for R2100225S15 completed
2022-05-06 01:07:23,038 - finder -   INFO - Mapping of reads for round2 completed for Control
2022-05-06 01:07:23,043 - finder -   INFO - Selecting high confidence junctions after round2 mapping completed for Control
2022-05-06 01:07:23,043 - finder -   INFO - Mapping rate in round2 Control R2100233S1 0.0
2022-05-06 01:07:23,043 - finder -   INFO - Mapping rate in round2 Control R2100234S2 0.0
2022-05-06 01:07:23,044 - finder -   INFO - Mapping rate in round2 Control R2100235S3 0.0
2022-05-06 01:07:23,044 - finder -   INFO - Mapping rate in round2 Control R2100237S5 0.0
2022-05-06 01:07:23,044 - finder -   INFO - Mapping rate in round2 Control R2100238S6 0.0
2022-05-06 01:07:23,044 - finder -   INFO - Mapping rate in round2 Control R2100240S8 0.0
2022-05-06 01:07:23,045 - finder -   INFO - Mapping rate in round2 Control R2100246S14 0.0
2022-05-06 01:07:23,045 - finder -   INFO - Mapping rate in round2 Control R2100247S15 0.0
2022-05-06 01:07:23,045 - finder -   INFO - Mapping rate in round2 Control R2100211S1 0.0
2022-05-06 01:07:23,045 - finder -   INFO - Mapping rate in round2 Control R2100212S2 0.0
2022-05-06 01:07:23,046 - finder -   INFO - Mapping rate in round2 Control R2100213S3 0.0
2022-05-06 01:07:23,046 - finder -   INFO - Mapping rate in round2 Control R2100215S5 0.0
2022-05-06 01:07:23,046 - finder -   INFO - Mapping rate in round2 Control R2100216S6 0.0
2022-05-06 01:07:23,046 - finder -   INFO - Mapping rate in round2 Control R2100218S8 0.0
2022-05-06 01:07:23,047 - finder -   INFO - Mapping rate in round2 Control R2100224S14 0.0
2022-05-06 01:07:23,047 - finder -   INFO - Mapping rate in round2 Control R2100225S15 0.0
2022-05-06 01:07:23,047 - finder -   INFO - Resorting to alignment with relaxed parameters for these runs due to poor mapping R2100233S1,R2100234S2,R2100235S3,R2100237S5,R2100238S6,R2100240S8,R2100246S14,R2100247S15,R2100211S1,R2100212S2,R2100213S3,R2100215S5,R2100216S6,R2100218S8,R2100224S14,R2100225S15
2022-05-06 01:07:23,058 - finder -   INFO - STAR relaxed alignment run for R2100233S1 completed
2022-05-06 01:07:23,068 - finder -   INFO - STAR relaxed alignment run for R2100234S2 completed
2022-05-06 01:07:23,078 - finder -   INFO - STAR relaxed alignment run for R2100235S3 completed
2022-05-06 01:07:23,088 - finder -   INFO - STAR relaxed alignment run for R2100237S5 completed
2022-05-06 01:07:23,097 - finder -   INFO - STAR relaxed alignment run for R2100238S6 completed
2022-05-06 01:07:23,107 - finder -   INFO - STAR relaxed alignment run for R2100240S8 completed
2022-05-06 01:07:23,117 - finder -   INFO - STAR relaxed alignment run for R2100246S14 completed
2022-05-06 01:07:23,128 - finder -   INFO - STAR relaxed alignment run for R2100247S15 completed
2022-05-06 01:07:23,138 - finder -   INFO - STAR relaxed alignment run for R2100211S1 completed
2022-05-06 01:07:23,147 - finder -   INFO - STAR relaxed alignment run for R2100212S2 completed
2022-05-06 01:07:23,157 - finder -   INFO - STAR relaxed alignment run for R2100213S3 completed
2022-05-06 01:07:23,166 - finder -   INFO - STAR relaxed alignment run for R2100215S5 completed
2022-05-06 01:07:23,177 - finder -   INFO - STAR relaxed alignment run for R2100216S6 completed
2022-05-06 01:07:23,186 - finder -   INFO - STAR relaxed alignment run for R2100218S8 completed
2022-05-06 01:07:23,195 - finder -   INFO - STAR relaxed alignment run for R2100224S14 completed
2022-05-06 01:07:23,204 - finder -   INFO - STAR relaxed alignment run for R2100225S15 completed
2022-05-06 01:07:23,204 - finder -   INFO - Mapping of reads for round3 completed for Control

Hi @SeqSmith,
Thank you reporting this issue. Currently, I am working on an improved version of Finder making major changes to some of the steps. I will let you know once that is available.
Thank you.

Hello, I have the same issue. Do you have a solution for it? I'm not running it with docker as I use HPC cluster. Please let me know if you have any questions. Thanks!


Log tile ids

Addressing and maybe fixing https://github.com/CesiumGS/cesium-native/issues/179 :
There now is a function to create a std::string from a TileID. The warning/error messages from RasterOverlayTileProvider::doLoad now contain these tile IDs. (If the underlying implementation uses loadTileImageFromUrl, then all warnings/errors will also contain the URL, but that was already done in another PR).
The current structure is (depending on the implementation, in pseudocode)
void doLoad(Tile tile) {
    Result result = loadTileImage(tile.getId());
    if (result.error) {
        // Log something here...
    }
}
virtual Result  loadTileImage(TileID tileID) { // Depends on implementation
    String url = createUrlFrom(tileId);
    return loadTileImageFromUrl(url);
}
Result loadTileImageFromUrl(String url) {
    Response response = request(url);
    return readImageData(response.data);
}
Result readImageData(vector<byte> data) {
    return stb_load(data);
}

So the question of "What should be logged where?" could be answered:

Networking errors (404s) are reported in loadTileImageFromUrl, which does the request/response handling, including the URL
Loading errors (image could not be read) are reported in doLoad, including the the tile ID
In order to "transport" low-level errors from stb, the loadedImage.errors can be used. Since this is implemented locally in a lambda, only for the loadTileImageFromUrl case, the URL can be added there directly.

In any case: If there is a problem with a tile, it now prints
LogCesium: Warning: [2021-04-26 17:52:21.040] [warning] [RasterOverlayTileProvider.cpp:552] Warnings while loading image for tile L8-X463-Y199:
- ...

Regarding the use of this and the extracted method: You made a similar point in another PR. I'm aware of the difficulties of race conditions. The lack of clarity and documentation add another layer of difficulty (i.e. it is not clear which objects are "thread safe" in which way, or which methods may or may not be called on which thread). And as much as I'd like to avoid repeating myself, and trying to say that as diplomatically as possible: Throwing hundreds of lines of code into several nested layers of undocumented lambdas, capturing and passing on random parts of the surrounding environment (of which it is not clear whether they are thread-safe, FWIW) is just not good. We have to find a better solution for that.
Here, it's only two things that are required from this, namely the IPrepareRendererResources, and, as always, the logger. (Why had the tileRectangle and the projection also been captured in the lambda originally? Well *shrugs* ... why not?). So I think that converting this into a static method is OK.

The lack of clarity and documentation add another layer of difficulty (i.e. it is not clear which objects are "thread safe" in which way, or which methods may or may not be called on which thread).

You keep saying this, but it's not true. The rules are fairly clear and totally the norm across C++ code bases. Documentation should tell you when something is explicitly thread safe or explicitly not thread safe, but when it doesn't, you may assume:

You can use different instances of a class simultaneously from different threads all you want.
You can share a single instance between threads safely so long as none of the threads are modifying it. If even one thread is or may be modifying it, it is not safe to even call const methods or access simple fields.
Instances that are potentially modified in one thread should not be used - and ideally shouldn't even be available to - any other threads.

If it helps to write this down somewhere, please do. But the above is what we should expect from any C++ library (and most other languages as well); it's not anything special about cesium-native.
To be clear, RasterOverlayTIleProvider is firmly in camp 3.

Why had the tileRectangle and the projection also been captured in the lambda originally? Well shrugs ... why not?

They were used by the old rectangle cutout system that we removed before 1.0, and we forgot to remove them. It's a shame none of the compilers flagged them as unused.

Throwing hundreds of lines of code into several nested layers of undocumented lambdas, capturing and passing on random parts of the surrounding environment (of which it is not clear whether they are thread-safe, FWIW) is just not good.

I don't think I agree with anything in this statement.

It's not hundreds of lines.
They're not "undocumented lambdas", they're continuations; in other environments with an async/await type feature we could write this code completely linearly without an extra lambda.
We're not capturing "random parts of the environment", we're capturing what we need to do the job; by reference for things that can be safely shared, by value for things that can't due to lifetime or thread safety.
Any time we pass something to a lambda, we need to think about its lifetime to ensure that the lambda can't outlive it. If that lambda is to be executed in a worker thread, we also need to think about its thread safety. Hopefully my previous comment makes it clearer what thread safety assumptions we can make about the things we potentially want to use from a worker.

Documentation should tell you when something is explicitly thread safe or explicitly not thread safe, but when it doesn't, you may assume:

I know that. That's the standard. But it's subtle. For example, consider https://github.com/CesiumGS/cesium-native/pull/232 . The problem was: This lambda did something in the worker thread. Some other class did something in the main thread. And these "somethings" interfered, causing random crashes.
The fix was was essentially a change from thenInWorkerThread to thenInMainThread. But maybe there is "something else" done there now. And and some other class does "something else" on the worker thread with the same object. And that may cause new random crashes. I did not verify that this was not the case, because it is not possible in practice.
And that's a problem.
More broadly: You cannot claim that we do not have an issue with concurrency and thread safety when we experience random crashes, flickering of tiles, or other (vague) ~"unexpected random behavior".

I don't think I agree with anything in this statement.

The severity of this problem varies. And I neither want to argue about whether it's "hundreds" or "dozens" of lines, or about whether we call it "continuation" or "lambda" (and I'll leave out anecdotes about all the conferences about parallel programming that I visited, and where I never saw even remotely similar constructs). But to re-phrase my statements in a form that will not allow you to vaguely disagree:

There are many lines of code in lambdas, and some of them are even nested
They are undocumented, period. There is no documentation whatsoever about what the lambdas are doing. In order to figure that out, one has to read them, line by line.
They are capturing parts of the environment, and there is no documentation whatsoever about 1. what these captures are used for or 2. how the lambda may affect these captures.

Of course, things like https://github.com/CesiumGS/cesium-native/blob/main/CesiumIonClient/src/Connection.cpp#L101 are extreme cases. Bringing that into a shape that is easier to read, maintain, and test could be a lot of effort (and have a bad effort-to-benefit ratio in this particular case, because it's unlikely (fingers crossed) that this code will ever have to be touched by anyone again). But for the "core" of cesium-native, maintainability, readability and testability should be priorized higher than they are right now.

The problem was: This lambda did something in the worker thread. Some other class did something in the main thread. And these "somethings" interfered, causing random crashes.

Please don't make it sound so mysterious. The previous code in #232 was passing a pointer to a CreditSystem instance to a worker. CreditSystem was not documented to be thread safe. Therefore, sharing it with the worker thread was a violation of the rules. And it caused a bug. We could have fixed this by either making CreditSystem thread safe or by making sure it's not shared between threads. We initially thought we should go with the first, but then realized the cost would be too high and decided to go with the second instead.
Yes, this sort of thing is easy to miss. If you have ideas how to make it less error-prone, I'm all ears. The purpose of the patterns we're using (AsyncSystem, the policy of not capturing this in lambdas passed to .thenInWorkerThread) is to make it so that, to be confident our code is thread-safe, we only need to look at the list of things that are captured in the .thenInWorkerThread lambda and confirm that they're all either copied or thread safe. But if we don't do that, or if we miss something, we'll have a bug.
It's clearly more error prone than other concurrency models like JavaScript's share-nothing Web Workers or an Actor model. But it has some significant advantages (mostly performance) and doesn't seem totally out of control to me.
But if you know of a better model for our needs (from attending many conferences on parallel programming or otherwise), then pelase tell me.

They are undocumented, period. There is no documentation whatsoever about what the lambdas are doing. In order to figure that out, one has to read them, line by line.

Frankly I'm a little exasperated here. The lambdas are part of a method that is documented to do a certain job asynchronously. Because some of that work needs to happen in a worker thread, and some needs to happen in the main thread, we need to use continuations. There is no reason those continuations need to be "documented", any more than a random dozen lines or whatever in a synchronous method need to be documented. If it's unclear what those dozen lines do, comments and method extraction can help explain. But there's no particular documentation needed just because we're using lambdas.

They are capturing parts of the environment, and there is no documentation whatsoever about 1. what these captures are used for or 2. how the lambda may affect these captures.

I'm not sure what you mean by "how the lambda may affect these captures."

Hey let me chime in (sorry for the intrusion).
From my experience lambdas can help readability or hurt readability. I agree with @javagl that if a lambda is long enough it really can hurt readability and can make you to lose the big picture pretty easy. For that matters sometimes I find more readable to use private (non-friend non-member) functions inside of an unnamed namespace.
Say that, a private function certainly does not need to be documented for the user (as they won't see it, the same as lambdas) but definitely can have some dev comments here and there (and to be honest while scrubbing some parts of cesium native I miss some dev comments). I usually forget what I wrote two days after, so it's more a help for my future me than anything else :sweat_smile: .
Lambdas can equally have this kind of comments, and I would say extracting a lambda to a function is a common sense judgement. Also there are cases when lambdas cannot easily be changed to a private function, when capturing local variables for instance.
Anyway I guess it's not a great deal to change those functions that are not very readable now  (either extracting or adding more comments).
All in all what I would love to see is more developer comments all over the place, but maybe is a personal preference as I tend to forget how things work very easily. Also when reading others people code, especially if there are some clever/smart tricks involved (template magic, ultramodern c++30 features, niche C++ features, or whatever).
Sorry for the disruption again, and keep up the good work!

Uh oh, if @javagl and @jtorresfabra mostly agree, and I'm on the other side, I must really be off my rocker! 😆
Thanks for the thoughts, though, @jtorresfabra. It all sounds reasonable to me, and I don't think I disagree with any of it.
I think maybe the thing that's missing when it comes to AsyncSystem is some higher-level documentation of what it's all about, how to use it, and what to watch out for. What should you capture in the lambdas and why? I don't think it's appropriate (or at least, I'm way too lazy) to try to document that at the (many) places it's used, but we should have it clearly laid out somewhere. I've been working under the assumption that the analogy to JS promises would make it inuitive to everyone without much documentation, and that the implications of thread safety on what you pass to the lambdas (and how) would be clear, but it's probably not true.
Meanwhile, I do stand by my original comment that started this discussion:

IMO this is an example of an "extract method" refactoring make the code less clear and harder to follow. When looking at doLoad, it's less clear what processing a loaded image actually entails, and in particular how it relates to the work that happens next in the main thread. And while looking at processLoadedImage, we're missing the vital context that it's invoked as a continuation in a worker thread. So, for example, that use of this may look fine in code review when actually it's pretty sus.

Marco has since renamed processLoadedImage to createLoadResultFromLoadedImage, and the new name actually helps a lot. If the code were written this way originally, I might suggest putting the extracted method in an anonymous namespace to get it out of the header, but otherwise wouldn't complain. But I don't think it's necessary or even an improvement to go back and refactor all our code like this to immediately call an anonymous-namespace method from inside every .thenX.
The way I think about a method like this doLoad here is that it's a straightforward, linear process that just happens to be asynchronous. The asynchronicity unfortunately makes the code look a bit more awkward, but once you get used to the style and see it for the linear process it is, it's not hard to read. Habitually extracting methods for each lambda actually obfuscates the nature of this code and makes it a little bit more error prone.
I want to emphasize that I'm not arguing against method extraction in general. For example, I think what you did here @javagl is great:
https://github.com/CesiumGS/cesium-native/pull/232/files
If I can try to articulate the difference...  collectCredits in #232 is a clearly distinct part of the process of creating a BingMapsTileProvider, with succinct inputs and a single output. createLoadResultFromLoadedImage is the entire threadable part of the doLoad, it has a mess of inputs, and it has an output that is interesting only because it's what the non-threadable part of doLoad expects. Prior to the refactoring in this PR, LoadResult was a private struct of doLoad. In other words, the collectCredits refactoring improves the readability of the code by moving self-contained and less important details out of the main method. The createLoadResultFromLoadedImage refactoring, on the other hand, makes the code less cohesive, with little apparent benefit other than avoiding a kinda-long lambda.
I guess what I'm saying is that I'm 👍 with extracting methods from AsyncSystem continuations using the same heuristics you'd use to decide when to extract a method from any method: improve reuse, improve testability, to remove less important details, etc. I'm less ok with extracting an entire continuation into a separate function. Maybe I'm splitting hairs here. As I mentioned before, I wouldn't complain much if the code in this PR were written originally like this, rather than a refactoring of existing code. But Marco has been keen to refactor the code for readability, and if we're going to do that we should try to be clear about what it means. Hence all the bike shedding.
And as an aside, I really dislike what our current clang-format config does with lambdas. If we were actually able to format our C++ code the way Prettier formats javascript, the code would be less indented and would do a much better job of revealing the programmatic structure with the source structure. But I don't know how to fix it with clang-format's limitations.

The crucial point is that they are not used in a thenInMainThread anywhere else. And that's hard to figure out.

No! It has almost nothing to do with thenInMainThread.
It's not safe to capture a pointer or reference for a thenInWorkerThread continuation unless the object pointed to or referenced is documented to be thread safe. End of story! Don't even look at how it's used. We shouldn't pass a non-thread-safe CreditSystem to a worker continuation no matter how we use it. Just like we should never pass this for a RasterOverlayTileProvider or anything else.

and 120 lines further down the road, there is the critical call to pCreditSystem->createCredit. That's far easier to miss than a function that carries a /** To be called on thread X, @param The thing that requires thread X */ comment.

Just to reiterate, capturing a CreditSystem by pointer or reference in a thenInWorkerThread lambda should be a red flag, whether that CreditSystem is used on the next line, 120 lines later, or not at all. Because it means we're sharing (or risking sharing) a non-thread-safe object between threads.

I do not know any coding guideline that does not explicitly state a limit on the "number of lines of code per method".

Hmm, well, I do.
The C++ Core Guidelines say nothing about it.
The LLVM Coding Standard says nothing about it.
The Google Coding Standard says to try to write short functions, but "We recognize that long functions are sometimes appropriate, so no hard limit is placed on functions length."

Of course, "LOC" is not a sensible measure, but the ideas of "having a method do one thing (and only that)" and "limiting the complexity of a method" are clear. And "one method doing things on different threads" is a complexity that should be viewn with scrutiny

Of course having a method do one thing and limiting method complexity are good goals, no argument there. And of course doing things across threads is complexity. That complexity will unfortunately exist in doLoad whether we break its continuations into separate functions or not. The question is whether extracting each continuation into a separate lambda reduces complexity. I say no. The complexity is all still there, but now spread out into multiple places in such a way that it's harder to follow.

e.g. for cases where no mutating (i.e. non-const) function is called on the object. This const should be added wherever it can be added.

Yeah that makes sense to me.
Keep in mind, though, that sharing an object across threads is safe only if all threads (including the main one) treat it as const. It's not good enough for the worker thread to access it via a pointer to const if there's any possibility of another thread modifying it. Since this is usually very hard to determine, and even harder to ensure stays true as the code evolves in the future, we basically shouldn't ever do it.

There are many levels on which the further discussion here could take place. I'll omit questions that could be considered as some sort of... handbrake-turn on the highway. (An example for such a question could be: Why do we only want one worker thread on a 6-core CPU?). So trying to narrow it down:

I think maybe the thing that's missing when it comes to AsyncSystem is some higher-level documentation of what it's all about, how to use it, and what to watch out for. What should you capture in the lambdas and why?
...
It's not safe to capture a pointer or reference for a thenInWorkerThread continuation unless the object pointed to or referenced is documented to be thread safe. End of story! Don't even look at how it's used.

It would be perfectly safe to use anything in the worker thread, if everything that is done with this object happened on the worker thread. End of story. The fact that there is ~"a collection of classes/objects that are only supposed to be used (!) on the main thread" is implicit knowledge. You "know" that because you created a mental model of the whole thing and implemented it. But deriving a certain mental model from code is hard or impossible.
(Or to put it that way: It's not something that could sensibly be documented in the AsyncSystem, but only contextually, as a description of how exactly the AsyncSystem is supposed to be used in cesium-native in general. Even then, I don't have a clear idea about how to reduce the risk of accidentally using something on the wrong thread, be it technically or on the level of documentation).
But my main point originally was not so much about the AsyncSystem itself, but more about the level of granularity and documentation in general. Looking at some examples, like the Connection one that I already linked to, highly critical (but far too complex) functions like Tile::loadContent, or something like https://github.com/CesiumGS/cesium-native/blob/main/Cesium3DTiles/src/TileMapServiceRasterOverlay.cpp#L121 , I'd strongly encourage you to try and take a step back, put yourself into the shoes of someone who has no clue what this is all about, and just has this one, simple question: What is happening there?.
For the last example: Sure, it's called createTileProvider, so .... it will probably create a tile provider. But there are >100 LOC, and even though some of the ">" can be attributed to the clang formatting here, the only way of making an educated guess to answer that simple question is to read the code. Line by line.
Iff this function was a bit closer to pseudocode like
LoadingContext loadingContext = { owner, accesor, whatnot };
struct TileDims { int width, height, minLevel, maxLevel };

return pAssetAccessor->requestAsset(asyncSystem, xmlUrl, this->_headers)
  .thenInWorkerThread([loadingContext](Request request) {
       JsonDocument document;
       if (!parseResponse(request->response, document)) return nullptr;

       TileDims tileDims = parseTileDims(document);
       Rectangle rectangle = parseRectangle(document, options);

       return std::make_unique<TileMapServiceTileProvider>(
           loadingContext, tileDims, rectangle);
  })
})

then answering the question of "What is happening there?" would involve reading 5 lines of code: It converts the response into a document, obtains the tile dimension and the coverage rectangle, and returns the tile provider. If it was necessary to know more details e.g. about what "computing the coverage rectangle" means, then one could read the doxygen documentation of parseRectangle, which would include the hint ~"to be called on worker thread".

The C++ Core Guidelines say nothing about it.

The C++ Guidelines and LLVM are targeting a very low level (and one reason of the difficulty of coming up with sensible coding standards, e.g. on a company level, is that "important" rules are buried under rules that dictate where to put a   space or not...).
Fortunately, these guidelines have already moved from the "non-sensical" LOC measure to others. But the first and most important points of the "Functions" category already try to encourage the right style, and https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-single still explicitly talks about "one-to-five-line functions" (which would be in line with the refactoring pseudocode that I sketched above).
Some of the https://clang.llvm.org/extra/clang-tidy/checks/list.html try to establish measures for "cognitive complexity" or a "function size" that can refer to LOC, but also to the number of statements (with the obvious caveat of some SPDLOG_LOGGER_ERROR macro that might well expand to hundreds or thousands of LOC/statements).
(BTW: Now this starts to become related to https://github.com/CesiumGS/cesium-native/issues/203 - I'm not a CI expert and probably couldn't set this up without admin rights somewhere, but will read a bit more the CLang toos in general - the https://clangpowertools.com/ seem to be a good starter here).
Coming back to one point

But I don't think it's necessary or even an improvement to go back and refactor all our code like this to immediately call an anonymous-namespace method

There's a certain amount of subjectivity involved. The C++ Guidelines stay vague here as well:

Flag functions that do not “fit on a screen.” How big is a screen? Try 60 lines by 140 characters; that’s roughly the maximum that’s comfortable for a book page.
Flag functions that are too complex. How complex is too complex? You could use cyclomatic complexity. Try “more than 10 logical path through.” Count a simple switch as one path.

Refactoring existing code to reduce the complexity of functions (particularly, functions that contain then... calls with large lambdas) is never "necessary", so to speak. I think that a general refactoring to break long functions and lambdas into shorter, documented ones would be worthwhile. But I know that there's always a trade off for that (roughly: it cannot be announced as a new feature - unless "reducing technical debt" is considered to be a "feature"...). But if this discussion leads to a state where

... people follow the "Boy Scout Rule" and break/document long functions ~"while they are working on that code part anyhow" and
... people don't introduce long functions in the first place

then the (my) main goal has been achieved, because I'm convinced that it will pay out, in better maintainability, testability, and fewer bugs.

I haven't processed all that, but one really quick correction... There isn't one worker thread. Continuations registered with .thenInWorkerThread are dispatched with ITaskProcessor::startTask to "a background thread." Not necessarily the same one each time it's called. In Cesium for Unreal, they're dispatched to Unreal's task system, which uses a thread pool. I don't know offhand how many threads are in UE's pool by default, but it's definitely more than 1.

The fact that there is ~"a collection of classes/objects that are only supposed to be used (!) on the main thread" is implicit knowledge. You "know" that because you created a mental model of the whole thing and implemented it. But deriving a certain mental model from code is hard or impossible.

I don't think there's any special knowledge required, beyond what could be easily documented in a "how to use AsyncSystem" tutorial. Any time you call .thenInWorkerThread, you are allowing - requesting, in fact! - that the continuation be invoked in some random other thread. Even if you're already in a worker thread, that continuation can and usually will be invoked in a different worker thread. Therefore it is never ok to give that lambda a pointer or reference to a non-thread-safe object. There's no implicit knowledge required to determine this, either. Look at the documentation for the object you're considering passing to the continuation. Does it explicitly say it's thread safe? No? Then you can't pass it by pointer or reference.

@javagl all this discussion is interesting and valuable, but let's get this PR merged. Unless you're opposed (or it doesn't work for some reason that isn't clear to me), please move createLoadResultFromLoadedImage and LoadResult out of the header and into the anonymous namespace in the .cpp. And then I'll merge this.

Well, details are important: I assumed (and that assumption may have been naive, and it is definitely wrong, but ... not totally unreasonably, IMHO) that there was only one, dedicated worker thread - similar to the "HTTP Thread" or the "Game Thread" or the "IO Thread" or whatnot that exist in Unreal. So when thenInWorkerThread is rather thenInAnyWorkerThread (aka throwThisIntoSomeThreadPool), some of my statements make no sense.
But again: I'm not dissecting the AsyncSystem here, or arguing about idioms in C++ vs. JS - the main points have been about keeping functions small, and lambdas even smaller, giving them names, and documenting them and their inputs (parameters/captures).

move createLoadResultFromLoadedImage and LoadResult out of the header and into the anonymous namespace

Done in the last commits. More generally, we could (and IMHO should) try to aim for some consistency here: There are many static functions and helper structs right now that are not in an anonymous (or impl) namespace, although they could (and therefore IMHO should) be.
I already ... mentioned earlier that I ... already mentioned this earlier ... 😳 at the bottom of https://github.com/CesiumGS/cesium-native/pull/94#discussion_r547997889  - if you don't shout "STOP!", I'd open an issue for that later today.

Thanks @javagl!


Add Mach-O magic bytes to the bundled binary detector

The bundled binary detector currently detects ELF (Unix) and exe (Windows) files, but it does not account for the magic numbers used to identify Mach-O (Mac) files. This could lead to malicious packages targeting Macs to go unnoticed.
For reference:

See https://en.wikipedia.org/wiki/Mach-O#Mach-O_header for more info.

Let me know if I can work on this :)

Hello @ocku , the case you point out seems valid as well
Please, send the PR well look into it.
Thanks again!


android: self-view native example does not show camera

This is with latest openwebrtc from master. With 0.3 release it works fine on the same device.
It seems like the camera itself works and captures but the output is not rendered correctly. The following is printed for both versions in adb logcat
04-13 13:31:18.157   210 13208 I mm-camera-sensor: Setting YUV preview to 15fps
04-13 13:31:18.158   210 13248 E mm-camera: demux_set_params: param_id 6, is not supported in this module
04-13 13:31:18.158   210 13248 E mm-camera: demux_set_params: param_id 8, is not supported in this module
04-13 13:31:18.158   210 13208 E mm-camera-sensor: module_sensor_event_control_set_parm:1084 CAM_INTF_PARM_LED_MODE 0
04-13 13:31:18.158   210 13208 E mm-camera-sensor:
04-13 13:31:18.158   210 13208 E mm-camera: isp_util_set_param_crop_region, No change in crop info, nothing to update

nativecall can show the self-view, not sure what is different with that one

https://bugzilla.gnome.org/show_bug.cgi?id=765266 is the problem


Make pixi work without internet connection

When I am not connected to the internet, I get:
$ pixi r build
ERROR error=error sending request for url (https://conda.anaconda.org/conda-forge/osx-arm64/repodata.json.zst)
ERROR error=error sending request for url (https://conda.anaconda.org/conda-forge/osx-arm64/repodata.json.zst)
  ⠁ build:win-64         [00:00:00] loading repodata
  ⠁ build:linux-64       [00:00:00] loading repodata
  ⠁ host:osx-arm64       [00:00:00] loading repodata
  ⠁ host:win-64          [00:00:00] loading repodata
  ⠁ host:linux-64        [00:00:00] loading repodata
  ⠁ default:osx-arm64    [00:00:00] loading repodata
  ⠁ default:linux-64     [00:00:00] loading repodata
  ⠁ default:win-64       [00:00:00] loading repodata                              × error sending request for url (https://conda.anaconda.org/conda-forge/osx-
  │ arm64/repodata.json.zst)
  ├─▶ client error (Connect)
  ├─▶ dns error: failed to lookup address information: nodename nor servname
  │   provided, or not known
  ╰─▶ failed to lookup address information: nodename nor servname provided, or
      not known

This is in contrast to conda where I just do "conda activate lf" and I can work without an internet.
Part of this issue is that pixi stores all the environments in .pixi in the current directory, so if I do "git clean -dfx" I lose it.
However, it would be great if pixi could simply used the already downloaded packages from ~/.pixi and rebuild the environment without the internet. Then I can easily work with pixi and different environments and creating environments while offline.

I just tried the following:
pixi init test-offline
cd test-offline
pixi add python
pixi run python
# cut internet connection
rm -rf .pixi
pixi run python

This just worked for me.
Can you please provide a reproducer for your issue?

@Hofer-Julian I think this happens when the .pixi folder is removed.

That's what I did :)

I don't have a lock file. Here is what I did yesterday:
$ git clone https://github.com/lfortran/lfortran
$ cd lfortran
$ pixi r build

Then disconnect from the internet:
$ git clean -dfx
$ pixi r build

However there was some time between the two parts. I just did this again in succession and it works!
But above I posted what happened when I tried to build yesterday, and it clearly fails.
When does pixi requires internet?

(The following Pixi task clean = "git clean -dfx -e '.pixi'" takes care of cleaning up my local repository except .pixi, which greatly speeds up builds.)

There is https://github.com/quantco/pixi-pack


Input Map: cannot map joypads with device numbers above zero

Operating system or device - Godot version:
Arch Linux, godot 3.0 f919be936b366f23388a8bcd99941d1fcd7cba73
Issue description:
Device numbers are reset to zero when saving joypad input mappings with device numbers above zero.  This is because:

InputEvents are serialized using the new generic Variant::Object serialization approach, which writes an object's member variables in a json-ish format (see variant_parser.cpp)
The device number is not written or read because it is set and retrieved using functions (InputEvent.get_device() and InputEvent.set_device(device))

I would fix this by making InputEvent.device a public variable, but if I did that I would probably also want to remove get_device and set_device because they are redundant, which would be a breaking change.
We should discuss the best way to fix this case, but we should probably also talk about how we will generically solve the "serializing/deserializing getters and setters" class of problems.
Semi-related question: Are there guidelines for when member variables should be hidden behind getters and setters in the godot codebase?  In the case of "device", get_device() just reads InputEvent.device and set_device(device) just writes to InputEvent.device.  Why does InputEvent.device need getters and setters, but public member variables like InputEvent.pressed don't?
Steps to reproduce:

map non-zero joypad in "Input Map" screen
save project
reopen project
observe that the mapping is saved, but the device number is set to zero

Can confirm with 00e5ba314393ce2cc4df883bc1742306007ed117 (yesterday build, I think).

Looks like @bojidar-bg discovered that  the problem is that "device" is not registered as a property, not that it isnt a public member variable.  I should have a looked at variant_parser.cpp a little closer.
@bojidar-bg: sorry if i lead you astray 😄.  It looks like you've got this!

@mcanders Don't worry, I ignored what you wrote the moment I read it 😃

That is usually a wise choice :)


Change 2D navigation ProjectSettings from integers to floats

Changes 2D navigation ProjectSettings from integers to floats.
Related issue https://github.com/godotengine/godot/issues/79474.
That was some arbitrary limitation added just for the ProjectSettings considering that those properties are floats everywhere else.

Will the type and default in doc/classes/ProjectSettings.xml be somehow synced with this update or is this missing in this PR?

Forgot to build the ProjectSettings xml.
Will only work for new projects or projects that have not changed the defaults. If an existing project has already changed the ProjectSettings the value will need to be reset first.

Will only work for new projects or projects that have not changed the defaults. If an existing project has already changed the ProjectSettings the value will need to be reset first.

This doesn't make it a good candidate for cherry-picking then.

Thanks!


[Feature request] Guide user to write DockerFile

Context
There is no GUI helping user to write a Dockerfile. source
but there are tons of written tutorials one example
BioShaDock is a GUI app (license: Apache 2.0)
but the DockerFile is to be written by hand.
Proposition
In page "new docker", replace the field "Dockerfile"
by a set of fields composing a dockerfile.
Use the javascript library BootstrapTour to guide the user to fill the fields of the dockerfile, giving information in popus.
A hack enabling to not only give an overview of an interface but wait for the user to input a field can be found on stackoverflow

Note
BioShaDock webpage html markup uses ids. Cf use of GWT in Alvis.

## Note
dockerfiles are not straight forward, so a tree of possibilities should be considered to achieve the simplicity of the "tour".


No control over backlight

I’m on ASUS Zenbook UX430UNR, OpenCore 0.6.2 from ur repo, and i’ve tried many times but with this kext my backlight doesn’t work. I see the image on screen of the backlight when i press the buttons but the screen remains at the same light level.

Send me output from https://github.com/hieplpvip/OC-gen-debug

http://www.mediafire.com/file/kqxylrxc3fa0fuc/debug_14561.zip/file
Thanks.

Hi, I see that you're injecting wrong AAPL,ig-platform-id and device-id.
AAPL,ig-platform-id should be 0000C087, while device-id should be 16590000, as done here.
If backlight still doesn't work after fixing id, please use DEBUG version and generate new debugging files.
Thanks.

Tried it yesterday, and the results are the same, anyway I'll try it another time and send file.

Here it is
thanks

Hi, I need some output from DEBUG version. Please add boot-arg -applbklsmoothdbg and try this one, then create new debugging files.
AppleBacklightSmoother-1.0.0-DEBUG.zip
In addition, can you try sleeping and see if backlight works after sleep? I know this sounds weird, but I had this problem before, so we should try everything possible.
Thank you a lot!

I'll try asap, Thanks to you.

Hey,
thanks for your effort! It doesn't work for me either on Kaby Lake Refresh / UHD 620 / DeviceId 5917 on Big Sur. Used your suggested framebuffer-config and the Debug-version of the kext and hot the following output. There are two obvious problems:

System is KabyLake and not CoffeeLake
The denied cache read

I don't have lilucpu-boot-arg or similar in place. Used platform-id is macbookpro14,1. If I can provide any more info, please let me know.
[   11.597468]: AppleBacklightSmoother  smoother: @ (DBG) Successfully routed hwSetBacklight
[   12.412805]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: system initialized BXT_BLC_PWM_FREQ1 = 0x4240161
[   12.412818]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: system initialized BXT_BLC_PWM_CTL1 = 0x80000000
[   12.412826]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   12.412917]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   12.412959]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   12.412971]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.507002]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.535636]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.535650]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.535662]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.535673]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.535688]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.569032]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.569045]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.569057]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.569069]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.569086]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.587073]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.587087]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.587098]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.587109]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.587123]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.782610]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   13.822815]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   15.843885]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   15.893827]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   22.925979]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   22.927415]: Sandbox: imagent(354) deny(1) mach-lookup com.apple.system.opendirectoryd.apiAppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   22.974783]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   22.991939]: AppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   23.339852]: Sandbox: routined(298) deny(1) mach-lookup com.apple.Maps.MapsSync.storeSandbox: nsurlsessiond(189) deny(1) file-write-create /private/var/db/nsurlsessiond/LibraryAppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161
[   24.639980]: Sandbox: AssetCacheLocato(434) deny(1) user-preference-read kCFPreferencesAnyApplicationSandbox: 1 duplicate report for AssetCacheLocato deny(1) user-preference-read kCFPreferencesAnyApplicationSandbox: distnoted(435) deny(1) file-read-metadata /usr/sbin/distnotedSandbox: distnoted(435) deny(1) file-read-metadata /usrSandbox: parsecd(371) deny(1) mach-lookup com.apple.securityd.xpcAppleBacklightSmoother  smoother: @ (DBG) wrapCflFakeWriteRegister32: write PWM_DUTY1 0x4f5/0x56c, rescaled to 0x3c9203f/0x4240161

Two obvio

Seems like CPU is incorrectly recognized as Coffee Lake. Can you try boot-arg lilucpu=9?

I suspect that Lilu is returning incorrect CPU generation. Please post output from this tool. It's based on Lilu CPUID parsing code.
cpuidparser.zip

Yeah, you are right:
CPU vendor: Intel
CPU max level: 0x16
CPU max level (ext): 0x80000008
CPU brand: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
CPU family: 6
CPU model: 142
CPU stepping: 0xa
CPU generation: 10 (Coffee Lake)

Very interesting. will pin to kaby lake and test again later.

with the argument lilucpu=9 works, for me

And it seems to be the intended behavior. See https://github.com/acidanthera/Lilu/blob/3b302dc0d972b8cf89c19569dfbea75a16b3c51a/Lilu/Sources/kern_cpu.cpp#L107
Didn't knew that...

@benbender Yeah, I have seen that. I'm opening a bug report now. For now please use lilucpu=9

A fine, want to do the same. Btw, old Bug about the same topic for reference:
https://github.com/acidanthera/bugtracker/issues/290
Thanks again!

Can confirm that it works with lilucpu=9. Issue can be closed I think if a note is added for the moment.

I'm thinking about doing the CPUID check on our side to determine if it's Kaby Lake-R or Coffee Lake, as it seems from https://github.com/acidanthera/bugtracker/issues/290 that this "bug" is intentional behavior.
It seems that stepping 0x9 is Kaby Lake, 0xa is Kaby Lake-R, 0xb is Coffee Lake.

Yes, I think so. And that might be ok for the "cpu side" of things, but for lilu and its cpu-detection, on which other projects build upon, its at least a documentation issue. There should be some notes in the code I think to avoid such problems like this one in the first place.

one more thing, since my brightness works fine without (maybe I don't notice difference) , I don't understand the meaning of this kext, can u explain it to me ?

It gives you smooth backlight transitions by splitting big change in brightness into smaller changes.

I can see the more "smooth" transition clearly. For me its working as intended, I think.

Another Idea: It would be also nice to have the option to simply force the use of a specific backlight-implementation via boot-arg. That would be even simpler I think as to redo the cpu-detection?
Use case: 5917 is troublesome on Hackintoshes in different ways and until now the backlight never worked with CFL-framebuffers. One could start experimenting if the backlight could be simply fixed with this kext.

I don't think it's necessary. The only case that it's useful is UHD620, which can be solved easily with a CPUID check.

@benbender Can you test this? No need to use lilucpu=9. In addition, backlight should also work with CFL framebuffer.
AppleBacklightSmoother-1.0.1-DEBUG.zip

Seems to be working. Thank you very much!


PhantomJS crashes. Error message doesn't explain much

I just opened another issue & PR related to phantomJS, but this one is actually unrelated
Regardless of using a global phantomjs installation or a local one, I get this error:
  1) user can login (Utrust.UserListTest)
     test/features/login_test.exs:6
     ** (Wallaby.JSError) There was an uncaught javascript error:

     [WDS] Disconnected!
       error (:0)

     code: |> visit("/")
     stacktrace:
       (wallaby) lib/wallaby/phantom/logger.ex:12: Wallaby.Phantom.Logger.parse_log/1
       (elixir) lib/enum.ex:675: Enum."-each/2-lists^foreach/1-0-"/2
       (elixir) lib/enum.ex:675: Enum.each/2
       (wallaby) lib/wallaby/phantom/driver.ex:497: Wallaby.Phantom.Driver.check_logs!/2
       (wallaby) lib/wallaby/browser.ex:707: Wallaby.Browser.visit/2
       test/features/login_test.exs:8: (test)

I can't really figure out what to do here.
In the meantime, switching to use headless chrome seems to work, but since support for that is only experimental, I didn't want to rely on it so much

Whats happening is that your page is throwing a JS error, wallaby is capturing that js error, and its failing your test. This is actually working as intended. Wallaby fails tests when it detects a JS error so that you don't accidentally ship code with errors that might cause regressions for your users. If you want to disable this you can set js_errors: false in your config. :+1:

Thanks. I actually figured that was the case, but couldn't find any reference to that js_errors: false option
After a quick look a the readme again I realise that it's there, and I just missed it