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

[bond] updated version + added bond-over-grpc integration as feature #10319

Merged
merged 13 commits into from
Jun 2, 2020

Conversation

eitanhs
Copy link
Contributor

@eitanhs eitanhs commented Mar 5, 2020

Describe the pull request

  • What does your PR fix?
    Fixes bond - please support building with DBOND_ENABLE_GRPC=TRUE #10316

  • Which triplets are supported/not supported? Have you updated the CI baseline?
    currently tested internally against windows-x64 but should work with other triplets without modifications. Didn't update the CI baseline.

  • Does your PR follow the maintainer guide?
    As much as I could, I'm not a C-Make pro so I'm not sure if and what I could have done better.

* added bond-over-grpc integration as feature
@eitanhs eitanhs closed this Mar 5, 2020
@msftclas
Copy link

msftclas commented Mar 5, 2020

CLA assistant check
All CLA requirements met.

@eitanhs eitanhs deleted the feature/bond_with_grpc branch March 5, 2020 22:47
@eitanhs eitanhs restored the feature/bond_with_grpc branch March 5, 2020 22:49
@eitanhs eitanhs reopened this Mar 15, 2020
@ras0219-msft
Copy link
Contributor

/azp run

1 similar comment
@dan-shaw
Copy link
Contributor

/azp run

@eitanhs eitanhs changed the title * updated bond version [bond] updated version + added bond-over-grpc integration as feature Mar 21, 2020
ports/bond/portfile.cmake Show resolved Hide resolved
ports/bond/portfile.cmake Outdated Show resolved Hide resolved
@@ -0,0 +1,11 @@
diff --git a/thirdparty/CMakeLists.txt b/thirdparty/CMakeLists.txt
Copy link
Member

Choose a reason for hiding this comment

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

We would be happy to take this functionality of skipping gRPC compilation upstream were you to contribute it behind a CMake variable.

@LilyWangL
Copy link
Contributor

@eitanhs Now this port build failed on Linux with the following error:

CMake Error at stack_build.cmake:21 (message):
  Version 2.3.1, Git revision de2a7b694f07de7e6cf17f8c92338c16286b2878 (8103
  commits) x86_64 hpack-0.33.0

  2020-05-15 11:23:37.473592: [debug] Checking for project config at:
  /mnt/_work/1/s/buildtrees/bond/src/8.2.0-6f74198761/compiler/stack.yaml

  2020-05-15 11:23:37.479173: [debug] Loading project config file stack.yaml

  2020-05-15 11:23:37.499447: [error] Preventing creation of stack root
  '/home/root/.stack/'.  Parent directory '/home/root/' is owned by someone
  else.

@chwarr
Copy link
Member

chwarr commented May 18, 2020

In the Bond CI builds themselves, we have to use cmake -DBOND_STACK_OPTIONS="--allow-different-user" ...

@eitanhs
Copy link
Contributor Author

eitanhs commented May 18, 2020

In the Bond CI builds themselves, we have to use cmake -DBOND_STACK_OPTIONS="--allow-different-user" ...

can I turn this flag regardless of the compiling platform? meaning, just add it to the options list in the portfile?

@chwarr
Copy link
Member

chwarr commented May 18, 2020

In the Bond CI builds themselves, we have to use cmake -DBOND_STACK_OPTIONS="--allow-different-user" ...

can I turn this flag regardless of the compiling platform? meaning, just add it to the options list in the portfile?

Seems to work fine on Windows. I just tested it with stack version 2.1.3 on Windows 10 (cd compiler; stack compile --allow-different-user) and everything built fine.

The documentation for --allow-different-user says "POSIX systems only." I guess that means that on non-POSIX systems or just doesn't do anything rather than causing a failure.

@eitanhs
Copy link
Contributor Author

eitanhs commented May 19, 2020

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 10319 in repo microsoft/vcpkg

@eitanhs
Copy link
Contributor Author

eitanhs commented May 19, 2020

@eitanhs Now this port build failed on Linux with the following error:
...

@LilyWangL, I thought I fixed it by applying the relevant flag @chwarr suggested.
Unfortunatley the build still fails on Azure Pipelines.
I did not see where is the actual error messages in the build? it shows only summary,
Can you post a link please so I can address the issues?

Thanks.

@LilyWangL
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LilyWangL
Copy link
Contributor

LilyWangL commented May 21, 2020

@eitanhs The Linux error:

CMake Error at stack_build.cmake:21 (message):
  Invalid argument `"--allow-different-user"'

  Auxiliary command not found in path `stack-"--allow-different-user"'

  File does not exist or is not a regular file `"--allow-different-user"'

  Usage: stack [--help] [--version] [--numeric-version]
  [--hpack-numeric-version]

               [--docker*] [--nix*] ([--verbosity VERBOSITY] | [-v|--verbose] |
               [--silent]) [--[no-]time-in-log] [--stack-root STACK-ROOT]
               [--work-dir WORK-DIR] [--[no-]system-ghc] [--[no-]install-ghc]
               [--arch ARCH] [--ghc-variant VARIANT] [--ghc-build BUILD]
               [-j|--jobs JOBS] [--extra-include-dirs DIR] [--extra-lib-dirs DIR]
               [--with-gcc PATH-TO-GCC] [--with-hpack HPACK]
               [--[no-]skip-ghc-check] [--[no-]skip-msys] [--local-bin-path DIR]
               [--setup-info-yaml URL] [--[no-]modify-code-page]
               [--[no-]allow-different-user] [--[no-]dump-logs]
               [--color|--colour WHEN] [--resolver RESOLVER] [--compiler COMPILER]
               [--[no-]terminal] [--stack-colors|--stack-colours STYLES]
               [--terminal-width INT] [--stack-yaml STACK-YAML] [--lock-file ARG]
               COMMAND|FILE
ninja: build stopped: subcommand failed.

You could get the error log from here: https://dev.azure.com/vcpkg/public/_build/results?buildId=37392&view=logs&jobId=f79cfdd7-47a8-597f-8f57-dc3e21a8f2ad

@eitanhs
Copy link
Contributor Author

eitanhs commented May 21, 2020

Thanks, I'll try to setup a Linux machine so I'll be able to test it locally before pushing...

+ using gbc@0.12.0.1 tag that includes relevant build fixes
@eitanhs
Copy link
Contributor Author

eitanhs commented May 21, 2020

I'm unable to re-produce the same error the Azure Piplines get.
I guess I hit this isse: commercialhaskell/stack#2187 (since the pipleines are running inside docker containers(?))
Not sure how to tackle this :(

@chwarr
Copy link
Member

chwarr commented May 21, 2020

@LilyWangL, what version of stack is on the vcpkg VMs? I think it's old enough that it doesn't have the --allow-different-user switch.

Below is what I see when I pass an invalid switch to stack Version 2.3.1, Git revision de2a7b694f07de7e6cf17f8c92338c16286b2878. Notice that it has [--[no-]allow-different-user]. The error that LilyWangL shared doesn't include that in the help text.

The stack documentation says this switch was added in 1.0.1.

We also know that Bond has trouble with anything below stack 1.5.1. If the stack version is old, is there a way to get it updated?

2020-05-21 9:28:00 AM (2020-05-21 16:28:00Z) :: chwarr@CHWARR-SL2 :: C:\src
  > stack --not-a-valid-switch
Invalid option `--not-a-valid-switch'

Usage: stack.exe [--help] [--version] [--numeric-version]
                 [--hpack-numeric-version] [--docker*] [--nix*]
                 ([--verbosity VERBOSITY] | [-v|--verbose] | [--silent])
                 [--[no-]time-in-log] [--stack-root STACK-ROOT]
                 [--work-dir WORK-DIR] [--[no-]system-ghc] [--[no-]install-ghc]
                 [--arch ARCH] [--ghc-variant VARIANT] [--ghc-build BUILD]
                 [-j|--jobs JOBS] [--extra-include-dirs DIR]
                 [--extra-lib-dirs DIR] [--with-gcc PATH-TO-GCC]
                 [--with-hpack HPACK] [--[no-]skip-ghc-check] [--[no-]skip-msys]
                 [--local-bin-path DIR] [--setup-info-yaml URL]
                 [--[no-]modify-code-page] [--[no-]allow-different-user]
                 [--[no-]dump-logs] [--color|--colour WHEN]
                 [--resolver RESOLVER] [--compiler COMPILER] [--[no-]terminal]
                 [--stack-colors|--stack-colours STYLES] [--terminal-width INT]
                 [--stack-yaml STACK-YAML] [--lock-file ARG] COMMAND|FILE

@BillyONeal
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@BillyONeal
Copy link
Member

#11569 See also, Bond's generated files are ill formed and don't work on VS2019 version 16.6.

@chwarr
Copy link
Member

chwarr commented May 26, 2020

This is fixed in Bond's master branch. I'm publishing a 9.0 release with this fix today.

@eitanhs
Copy link
Contributor Author

eitanhs commented May 27, 2020

This is fixed in Bond's master branch. I'm publishing a 9.0 release with this fix today.

Thanks @chwarr !
After upgrading to the latest release all tests seems to pass!

@eitanhs
Copy link
Contributor Author

eitanhs commented May 30, 2020

@LilyWangL - I think I addressed your comment. Can you please re-review?

@LilyWangL LilyWangL added the info:reviewed Pull Request changes follow basic guidelines label Jun 1, 2020
@BillyONeal BillyONeal merged commit a41c53c into microsoft:master Jun 2, 2020
@eitanhs eitanhs deleted the feature/bond_with_grpc branch June 3, 2020 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bond - please support building with DBOND_ENABLE_GRPC=TRUE
7 participants