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

Changed CMAKE_CXX_FLAGS to c++17, add target links to Abseil library and c_str() addition when logging #64

Merged
merged 26 commits into from
Sep 22, 2023

Conversation

fordth
Copy link
Contributor

@fordth fordth commented Aug 31, 2023

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jamolina
Copy link
Contributor

Thank you for your contribution! We will review your changes and get back to you as soon as possible

fordth and others added 9 commits September 1, 2023 10:33
* Changes in CMakeLists.txt with check for Ubuntu
* Detect location of .NET for Ubuntu
Conditional compilation for Ubuntu
   If libraries are not available, compile grpc and krb5 libs and include
    into credentials-fetcher.
    grpc and krb5 libs are separate CMake targets.

The grpc and krb5 tar files are too large for github, they have to be downloaded.

To build grpc or krb5:
* "cd dependencies && mkdir build && cd build && make build_grpc && make build_krb5"
@smhmhmd
Copy link
Contributor

smhmhmd commented Sep 7, 2023

@fordth I have added one more PR on top of yours - fordth#2

Pull in grpc and krb5 compilation into credentials-fetcher
@smhmhmd
Copy link
Contributor

smhmhmd commented Sep 8, 2023

@saikiranakula-amzn - Please review PR

@fordth
Copy link
Contributor Author

fordth commented Sep 8, 2023

@smhmhmd I just tried running cmake -DCMAKE_CXX_STANDARD=17 ../ on Ubuntu 20.04 and have now since received a number of errors. I will follow up with an output of the errors today ,but performing a cmake on the PR will repo the issue in Ubuntu

@smhmhmd
Copy link
Contributor

smhmhmd commented Sep 8, 2023

@smhmhmd I just tried running cmake -DCMAKE_CXX_STANDARD=17 ../ on Ubuntu 20.04 and have now since received a number of errors. I will follow up with an output of the errors today ,but performing a cmake on the PR will repo the issue in Ubuntu

Hi @fordth ,
You dont need to run cmake -DCMAKE_CXX_STANDARD=17 ../., cmake ../ is sufficient if you already have gRPC installed.
To install gRPC and krb5, you need to run cmake in the dependencies directory.
I have the steps in the comment of the PR - fordth#2

I just retested in my instance.

@smhmhmd
Copy link
Contributor

smhmhmd commented Sep 8, 2023

@fordth, I have some time till the end of the hour if you want to meet or we can meet on Monday.

@smhmhmd
Copy link
Contributor

smhmhmd commented Sep 12, 2023

@fordth
If you are able to test, I will get your PR merged.

@fordth
Copy link
Contributor Author

fordth commented Sep 12, 2023

We plan to test the compilation morning of Wednesday 9/13. Once this PR is then completed, I will create a new PR containing the additional changes we spoke about. I will provide an update tomorrow

smhmhmd and others added 5 commits September 17, 2023 16:38
Changing grpc install to default location from user defined directory
since Cmake module in grpc searches only in default locations
Changing grpc install to default location from user defined directory
since Cmake module in grpc searches only in default locations

This also includes docker changes to build on Ubuntu-20.04
Contains docker file for Ubuntu 20.04 to support the latest CMake refactoring
@fordth
Copy link
Contributor Author

fordth commented Sep 19, 2023 via email

fordth and others added 2 commits September 19, 2023 21:20
…wnloads. Fixed issue with .net now included in Ubuntu 22.04 causing conflicts when installing .Net sdk 6 package.
Copy link
Collaborator

@saikiranakula-amzn saikiranakula-amzn left a comment

Choose a reason for hiding this comment

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

testing on fedora

@smhmhmd smhmhmd merged commit 31ca81e into aws:mainline Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants