Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[threadpool] Add new port #11650

Merged
merged 5 commits into from
Jul 8, 2020
Merged

[threadpool] Add new port #11650

merged 5 commits into from
Jul 8, 2020

Conversation

jesseschalken
Copy link
Contributor

Adds a port for the threadpool library.

Fixes #9735

@msftclas
Copy link

msftclas commented May 29, 2020

CLA assistant check
All CLA requirements met.

@jesseschalken
Copy link
Contributor Author

jesseschalken commented May 29, 2020

What is the actual failure from CI? The only information it says is:

|x86-windows|result|features|notes
|-----------|----|--------|-----
|threadpool |Fail|core|Test passes in baseline but fails in current run. If expected update ci.baseline.txt with 'threadpool:x86-windows=fail'

The following build failures were ignored: openmama x264 3fd

Uncompressing archives\fail\3d\3d131b502c7b4949811c898bbc27864ce567f359.zip to C:\agent\_work\1\a\failureLogs\x86-windows\
##[error]REGRESSION: threadpool:x86-windows. If expected, add threadpool:x86-windows=fail to C:\agent\_work\1\s\scripts\ci.baseline.txt.
##[section]Finishing: Analyze results and prepare test logs

But what is the actual failure? It doesn't say.

I've tried using this library on the x86-windows triplet and it works fine.

@Neumann-A
Copy link
Contributor

-- Downloading https://downloads.sourceforge.net/project/threadpool/threadpool/0.2.5%20%28Stable%29/threadpool-0_2_5-src.zip...
-- Downloading https://downloads.sourceforge.net/project/threadpool/threadpool/0.2.5%20%28Stable%29/threadpool-0_2_5-src.zip... Failed. Status: 7;"Couldn't connect to server"
CMake Error at scripts/cmake/vcpkg_download_distfile.cmake:175 (message):
      
      Failed to download file.
      If you use a proxy, please set the HTTPS_PROXY and HTTP_PROXY environment
      variables to "https://user:password@your-proxy-ip-address:port/".
      Otherwise, please submit an issue at https://github.com/Microsoft/vcpkg/issues

Call Stack (most recent call first):
  ports/threadpool/portfile.cmake:2 (vcpkg_download_distfile)
  scripts/ports.cmake:90 (include)

@Neumann-A
Copy link
Contributor

Neumann-A commented May 29, 2020

Probably should print a warning that the this implementation is not NUMA aware ;) and will only support <64 logical cores on windows

@jesseschalken
Copy link
Contributor Author

@Neumann-A Thanks. I force pushed an identical commit to retrigger the CI because I thought it might be an intermittent network failure.

And still, the file downloads fine for all the triplets except x86-windows. It's literally the same URL. I don't understand how that's possible.

The file downloads fine here via the browser and via vcpkg install threadpool --triplet x86-windows.

How do I even debug this?

@NancyLi1013 NancyLi1013 self-assigned this Jun 1, 2020
@NancyLi1013 NancyLi1013 added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Jun 1, 2020
Copy link
Contributor

@NancyLi1013 NancyLi1013 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a header-only library? It seems just some headers are added.

ports/threadpool/portfile.cmake Outdated Show resolved Hide resolved
Co-authored-by: NancyLi1013 <46708020+NancyLi1013@users.noreply.github.com>
@jesseschalken
Copy link
Contributor Author

@NancyLi1013 Yep, this library is header-only.

@jesseschalken
Copy link
Contributor Author

So how do I mark the requested review changes as accepted and remove the requires:author-response label?

ports/threadpool/portfile.cmake Outdated Show resolved Hide resolved
@NancyLi1013
Copy link
Contributor

@jesseschalken

Sorry for the late response.

The labels can only be removed by our team members. We will remove it if there are no any changes needed to be done.

@jesseschalken
Copy link
Contributor Author

@NancyLi1013 Okay, thanks.

@NancyLi1013
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@NancyLi1013 NancyLi1013 added the info:reviewed Pull Request changes follow basic guidelines label Jun 15, 2020
@ras0219-msft ras0219-msft merged commit 6d4606f into microsoft:master Jul 8, 2020
@ras0219-msft
Copy link
Contributor

Thanks!

@jesseschalken jesseschalken deleted the threadpool branch July 10, 2020 11:05
@jesseschalken
Copy link
Contributor Author

I don't need this anymore personally because the code that used this library turned out to be unreachable. 😣 Maybe someone else will find it useful though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:new-port The issue is requesting a new library to be added; consider making a PR! info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[New Port Request] threadpool
5 participants