Skip to content
This repository was archived by the owner on Oct 12, 2022. It is now read-only.
/ druntime Public archive

Revert "core.thread.refactor Part 1"#2822

Closed
wilzbach wants to merge 1 commit intomasterfrom
revert-2821-osthread
Closed

Revert "core.thread.refactor Part 1"#2822
wilzbach wants to merge 1 commit intomasterfrom
revert-2821-osthread

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Oct 8, 2019

Reverts #2821

@rainers
Copy link
Member

rainers commented Oct 8, 2019

I suspect the merge on Azure fails because the PR is from the dlang repo. Should we still force-merge this PR?

@rainers
Copy link
Member

rainers commented Oct 9, 2019

Removed auto-merge, doesn't seem to be the only problem. #2825 runs the Azure tests with the revert, but still fails.

@wilzbach
Copy link
Contributor Author

wilzbach commented Oct 9, 2019

@rainers the error happens when trying to clone DMD:

Cloning into '../dmd'...
fatal: Remote branch revert-2821-osthread not found in upstream origin
##[error]Cmd.exe exited with code '128'.

The problem is here: https://github.com/dlang/druntime/blob/master/azure-pipelines.yml#L43 :

  DMD_BRANCH:
    Parsing expression: <coalesce(variables['System.PullRequest.TargetBranch'], variables['Build.SourceBranchName'], 'master')>
    Evaluating: coalesce(variables['System.PullRequest.TargetBranch'], variables['Build.SourceBranchName'], 'master')
    Expanded: coalesce(Null, 'revert-2821-osthread', 'master')
    Result: 'revert-2821-osthread'

Maybe the environment variable SYSTEM_PULLREQUEST_TARGETBRANCH (see e.g. https://github.com/YakDriver/azure-pipelines-environment-variables#system_pullrequest_targetbranch) is defined?

@wilzbach
Copy link
Contributor Author

wilzbach commented Oct 9, 2019

It's weird that variables['System.PullRequest.TargetBranch'] is Null though:

The branch that is the target of a pull request. For example: refs/heads/master. This variable is initialized only if the build ran because of a Git PR affected by a branch policy.

https://docs.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml#system-variables

Maybe it's null, because it was re-triggered manually?

@rainers
Copy link
Member

rainers commented Oct 9, 2019

I suspect it makes a difference whether the same repo is used for the PR and the target and then some variables behave differently, We should add printing the environment variables to the yml as in the dmd version.

The revert alone didn't help, though, see #2825 . So resolving this merge issue could be in another PR.

@rainers
Copy link
Member

rainers commented Oct 10, 2019

So resolving this merge issue could be in another PR.

It seems the branch isn't built as a PR, but a separate branch. I guess we should configure only master and stable as "triggers": https://docs.microsoft.com/en-us/azure/devops/pipelines/yaml-schema?view=azure-devops&tabs=schema#triggers

@codecov
Copy link

codecov bot commented Nov 8, 2019

Codecov Report

Merging #2822 into master will decrease coverage by <.01%.
The diff coverage is 92.51%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2822      +/-   ##
==========================================
- Coverage   74.56%   74.55%   -0.01%     
==========================================
  Files         161      160       -1     
  Lines       17147    17147              
==========================================
- Hits        12785    12784       -1     
- Misses       4362     4363       +1
Impacted Files Coverage Δ
src/core/internal/spinlock.d 77.77% <ø> (ø) ⬆️
src/core/thread.d 82.97% <92.51%> (ø)
src/test_runner.d 69.69% <0%> (-3.04%) ⬇️
src/core/sync/semaphore.d 87.34% <0%> (-1.27%) ⬇️
src/rt/aaA.d 88.01% <0%> (-0.28%) ⬇️
src/gc/impl/conservative/gc.d 79.22% <0%> (-0.07%) ⬇️
src/core/sync/rwmutex.d 91.22% <0%> (+0.58%) ⬆️
src/core/sync/condition.d 93.91% <0%> (+1.73%) ⬆️

Continue to review full report at Codecov.

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

@Geod24 Geod24 deleted the revert-2821-osthread branch June 2, 2021 03:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants