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

First attempt at fixing nlohmann doctest compilation error #4499

Merged
merged 3 commits into from
Mar 31, 2023

Conversation

LarryOsterman
Copy link
Member

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

  • C++ Guidelines
  • Doxygen docs
  • Unit tests
  • No unwanted commits/changes
  • Descriptive title/description
    • PR is single purpose
    • Related issue listed
  • Comments in source
  • No typos
  • Update changelog
  • Not work-in-progress
  • External references or docs updated
  • Self review of PR done
  • Any breaking changes?

@LarryOsterman
Copy link
Member Author

/azp run cpp - core

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LarryOsterman
Copy link
Member Author

/azp run cpp - core

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vhvb1989
Copy link
Member

Do we have an issue for this?
What is what we are trying to fix?
We did not need to update this tests (as it is external) in the past. So, wondering why we need to touch those files now...

@vhvb1989
Copy link
Member

might be worth it to consider updating the nlohmann json lib to a newer version.

@danieljurek
Copy link
Member

@vhvb1989 -- This is to fix a complier error we're encountering in this PR: #4442

This is the error we're seeing: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2667957&view=logs&j=ee76c233-a59e-5872-c34c-b9843bcc71ea&t=7df4bab8-9439-5dda-1896-ba94748419f5

@LarryOsterman
Copy link
Member Author

/azp run cpp - core

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@LarryOsterman
Copy link
Member Author

@vhvb1989 -- This is to fix a complier error we're encountering in this PR: #4442

This is the error we're seeing: https://dev.azure.com/azure-sdk/internal/_build/results?buildId=2667957&view=logs&j=ee76c233-a59e-5872-c34c-b9843bcc71ea&t=7df4bab8-9439-5dda-1896-ba94748419f5

Basically, on Ubuntu 2022, the SIGSTKSZ value stopped being a constant and turned into a function call. This change picks up the minimal set of changes necessary to get the doctest.h file to continue to work.

IMHO resyncing nlohmann.json is a great idea, but this is all about fixing our pipelines in front of a monday shutdown of Ubuntu 18 images.

@vhvb1989
Copy link
Member

Just noticed that doctest is a library and we are consuming an auto-generated .h file.

I created this other approach to update the doctest library directly to latest version, instead of patching the old library
#4501

@LarryOsterman
Copy link
Member Author

/check-enforcer bypass

@github-actions
Copy link

For help using check enforcer, see https://aka.ms/azsdk/checkenforcer

Available commands:

  • /check-enforcer evaluate - Re-evaluate existing pipeline statuses for PR
  • /check-enforcer override - Ignore any pipeline missing or failed statuses for PR
  • /check-enforcer help - Add this comment

If you are initializing a new service, follow the new service docs. If no Azure Pipelines are desired, run /check-enforcer override.

@LarryOsterman
Copy link
Member Author

/check-enforcer override

@LarryOsterman LarryOsterman merged commit 6e81075 into Azure:main Mar 31, 2023
LarryOsterman added a commit that referenced this pull request Apr 3, 2023
* Sync eng/common directory with azure-sdk-tools for PR 5608 (#4411)

* ongoing

* Move get-codeowners scripts and tests to their own dirs

* address PR remarks

* Fix CodeOwnerFileLocation path, fix casing, and dedup param defaults

* fix param names

* add todos on needed changes in cpp repo

* Add CodeownersFileLocation to Get-Codeowners and use $null for default param values

* move get-codeowners back to scripts/ and rename -functions to .lib

* fix: use empty string as defaults instead of $nulls, to fix invocation

* fix bug with invocation of Get-Codeowners + add support for passing IncludeNonUserAliases as switch

* fix path iin Metadata-Helpers.ps1

* fix typo

* Update archetype-cpp-release.yml

---------

Co-authored-by: Konrad Jamrozik <[email protected]>

* First attempt at fixing nlohmann doctest compilation error (#4499)

* First attempt at fixing nlohmann doctest compilation error

* fixed compilation issue

* Fixed ApiView generation for azure core

* update doctest lib to 2.4.11 (#4501)

* Migrate Ubuntu 18 to 22 directly in the matricses (#4442)

* First cut at migrating Ubuntu 18 to 22 directly in the matricses

* Try 20.04

* Remove azure.list from apt configuration on 22.04

* Parens

* gpp-8 on Ubuntu 20.04

* Remove g++-5

* Move config changes to steps/

* Remove g++-5 from live tests

* Update eng/pipelines/templates/steps/fix-linux-1es-configs.yml

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Review feedback

* Finish renaming

* Setting line coverage target to 91%

---------

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Initial population of azure-core-amqp

* cspell

* AMQP specific cspell dictionary

* CI pipeline fixes

* azure-uamqp-c cannot work on UWP builds

* Final cspell error; added amqp package to artifacts

* clang-format; added copyright notices to everything

* Added required files

* more CI fixes

* Renamed some files

* Renamed some files

* fixed case of amqp directory

* Renamed models directory

* Renamed network directory

* Most filenames are now lower case

* Hopefully fix CI pipeline failures

* Sort lines in cmakelists.txt

* Fixed name of message_target.cp

* Fixed name of transfer_instance.cpp

* Added preset for AMQP builds; reverted change from cmakesettings.json

* Add AMQP to doxygen generation; fixed clang-format issue

* Case folding fixes

* Correctly set up for DLL builds; removed diagnostic code

* Fixed socket listener to initialize platform

* clang-format

* clang-format

* clang-format; clang compilation fix

* Moved message sender implementation to shared pointer to enable copy and move semantics

* Don't forward declare enums with underlying type

* MOved MessageReceiver into an implementation class

* A couple of clang compiler fixes

* Moved all connection and session code to shared pointers

* Converted network callbacks to use events; restructured network to use shared pointer

* clang-format fixes

* case sensitivity

* clang fixes 4

random port for socket listener echo tests

clang-format

Random port for two more tests

Null event handler before destroying messagereceiver

clang-format

Random port for ReceiverOpenClose

Random port for LinkAttachDetach

Use random port for session tests

gcc warnings again 3

gcc warnings again 2

gcc warnings again

Print errno on socket listener start error

gcc warnings; enabled code coverage for amqp

clang-format

Missed file

clang fixes 8

clang fixes 7

clang fixes 6

clang fixes 5

clang fixes 3

clang fixes 2

clang fixes

* Catch exception thrown from worker thread

* Sample fixes

* Added context to wait for polled result

* clang build fix

clang build fix

* wait until listener thread completes before starting message receiver

* wait until listener thread completes before starting message receiver 2

* more thread cleanup changes

* more linux diagnostics 5

more linux diagnostics 13

more linux diagnostics 12

more linux diagnostics 11

more linux diagnostics 10

more linux diagnostics 9

more linux diagnostics 8

more linux diagnostics 7

more linux diagnostics 6

more linux diagnostics 4

more linux diagnostics 3

more linux diagnostics 2

more linux diagnostics

* Search for ports at 5000, not 0x5000; clang-format changes

* Fixed AV in MessageSenderReceiver when test is cancelled;

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* pull request feedback

* Added cbs test

* Finished snake case rename

* Fixed copyright and license text

* Undid accidental checkin

* Moved common and models types to _internal terminal namespace

* Basic CBS authentication test

* removed test resource name

* Fixed SAS token samples

* Sanitized token writer sample

* CI pipeline fixes

* Clear event callback when destroying message sender; close sender and receiver when done with test thread

* ApiView fixes; clang-format

* Fixed a coupe of CI pipeline issues

* Moved credenial type to a member variable not a virtual method returnign a constant

* Full mock implementation of claims based security

* Renamed ClaimBasedSecurity to ClaimsBasedSecurity; clang-format; clang fixes

* Improved code coverage.

* clang fix

* unit test fix

* Improved code coverage for connection string tests

* Added test for authenticated sender and receiver

* Added message source and target tests

Use better constructor for tokencredential

Use better constructor for tokencredential

crlf at the end of every file; made ClaimBasedSecurity class a _detail class

More code coverage fixes

Improved coe coverage for amqp message

Message->Value conversion 3

Message->Value conversion 2

Try casting message to value

Try casting message to value

Reverted attestation chagnes 2

Added more tests

Fixed doxygen issue

Don't use sender unless sender is set

Removed incorrect test

* More code coverage fixes

* Removed 2 param ctor

* More code coverage

* clang-format

* Hopefully final code coverage fixes

* clang-format

* Moved all amqp types to _internal terminal namespace

* Undid attestation change

* clang-format fixes

* Renamed Azure::Core::Amqp::Value to Azure::Core::Amqp::AmqpValue because Value is too generic a type name when you have 'using namespace' directives

* clang-format

* clang-format2

* turn crash into failure

* Try using async send rather than sync send for responding to CBS operations

* It helps to build the code you write

* Trigger CI

---------

Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: Konrad Jamrozik <[email protected]>
Co-authored-by: Victor Vazquez <[email protected]>
Co-authored-by: Daniel Jurek <[email protected]>
Co-authored-by: Ben Broderick Phillips <[email protected]>
antkmsft pushed a commit that referenced this pull request Apr 5, 2023
* First attempt at fixing nlohmann doctest compilation error

* fixed compilation issue

* Fixed ApiView generation for azure core
antkmsft pushed a commit that referenced this pull request Apr 5, 2023
* First attempt at fixing nlohmann doctest compilation error

* fixed compilation issue

* Fixed ApiView generation for azure core
benbp added a commit that referenced this pull request Apr 6, 2023
* Sync eng/common directory with azure-sdk-tools for PR 5608 (#4411)

* ongoing

* Move get-codeowners scripts and tests to their own dirs

* address PR remarks

* Fix CodeOwnerFileLocation path, fix casing, and dedup param defaults

* fix param names

* add todos on needed changes in cpp repo

* Add CodeownersFileLocation to Get-Codeowners and use $null for default param values

* move get-codeowners back to scripts/ and rename -functions to .lib

* fix: use empty string as defaults instead of $nulls, to fix invocation

* fix bug with invocation of Get-Codeowners + add support for passing IncludeNonUserAliases as switch

* fix path iin Metadata-Helpers.ps1

* fix typo

* Update archetype-cpp-release.yml

---------

Co-authored-by: Konrad Jamrozik <[email protected]>

* First attempt at fixing nlohmann doctest compilation error (#4499)

* First attempt at fixing nlohmann doctest compilation error

* fixed compilation issue

* Fixed ApiView generation for azure core

* update doctest lib to 2.4.11 (#4501)

* Migrate Ubuntu 18 to 22 directly in the matricses (#4442)

* First cut at migrating Ubuntu 18 to 22 directly in the matricses

* Try 20.04

* Remove azure.list from apt configuration on 22.04

* Parens

* gpp-8 on Ubuntu 20.04

* Remove g++-5

* Move config changes to steps/

* Remove g++-5 from live tests

* Update eng/pipelines/templates/steps/fix-linux-1es-configs.yml

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Review feedback

* Finish renaming

* Setting line coverage target to 91%

---------

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Initial population of azure-core-amqp

* cspell

* AMQP specific cspell dictionary

* CI pipeline fixes

* azure-uamqp-c cannot work on UWP builds

* Final cspell error; added amqp package to artifacts

* clang-format; added copyright notices to everything

* Added required files

* more CI fixes

* Renamed some files

* Renamed some files

* fixed case of amqp directory

* Renamed models directory

* Renamed network directory

* Most filenames are now lower case

* Hopefully fix CI pipeline failures

* Sort lines in cmakelists.txt

* Fixed name of message_target.cp

* Fixed name of transfer_instance.cpp

* Added preset for AMQP builds; reverted change from cmakesettings.json

* Add AMQP to doxygen generation; fixed clang-format issue

* Case folding fixes

* Correctly set up for DLL builds; removed diagnostic code

* Fixed socket listener to initialize platform

* clang-format

* clang-format

* clang-format; clang compilation fix

* Moved message sender implementation to shared pointer to enable copy and move semantics

* Don't forward declare enums with underlying type

* MOved MessageReceiver into an implementation class

* A couple of clang compiler fixes

* Moved all connection and session code to shared pointers

* Converted network callbacks to use events; restructured network to use shared pointer

* clang-format fixes

* case sensitivity

* clang fixes 4

random port for socket listener echo tests

clang-format

Random port for two more tests

Null event handler before destroying messagereceiver

clang-format

Random port for ReceiverOpenClose

Random port for LinkAttachDetach

Use random port for session tests

gcc warnings again 3

gcc warnings again 2

gcc warnings again

Print errno on socket listener start error

gcc warnings; enabled code coverage for amqp

clang-format

Missed file

clang fixes 8

clang fixes 7

clang fixes 6

clang fixes 5

clang fixes 3

clang fixes 2

clang fixes

* Catch exception thrown from worker thread

* Sample fixes

* Added context to wait for polled result

* clang build fix

clang build fix

* wait until listener thread completes before starting message receiver

* wait until listener thread completes before starting message receiver 2

* more thread cleanup changes

* more linux diagnostics 5

more linux diagnostics 13

more linux diagnostics 12

more linux diagnostics 11

more linux diagnostics 10

more linux diagnostics 9

more linux diagnostics 8

more linux diagnostics 7

more linux diagnostics 6

more linux diagnostics 4

more linux diagnostics 3

more linux diagnostics 2

more linux diagnostics

* Search for ports at 5000, not 0x5000; clang-format changes

* Fixed AV in MessageSenderReceiver when test is cancelled;

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* pull request feedback

* Added cbs test

* Finished snake case rename

* Fixed copyright and license text

* Undid accidental checkin

* Moved common and models types to _internal terminal namespace

* Basic CBS authentication test

* removed test resource name

* Fixed SAS token samples

* Sanitized token writer sample

* CI pipeline fixes

* Clear event callback when destroying message sender; close sender and receiver when done with test thread

* ApiView fixes; clang-format

* Fixed a coupe of CI pipeline issues

* Moved credenial type to a member variable not a virtual method returnign a constant

* Full mock implementation of claims based security

* Renamed ClaimBasedSecurity to ClaimsBasedSecurity; clang-format; clang fixes

* Improved code coverage.

* clang fix

* unit test fix

* Improved code coverage for connection string tests

* Added test for authenticated sender and receiver

* Added message source and target tests

Use better constructor for tokencredential

Use better constructor for tokencredential

crlf at the end of every file; made ClaimBasedSecurity class a _detail class

More code coverage fixes

Improved coe coverage for amqp message

Message->Value conversion 3

Message->Value conversion 2

Try casting message to value

Try casting message to value

Reverted attestation chagnes 2

Added more tests

Fixed doxygen issue

Don't use sender unless sender is set

Removed incorrect test

* More code coverage fixes

* Removed 2 param ctor

* More code coverage

* clang-format

* Hopefully final code coverage fixes

* clang-format

* Moved all amqp types to _internal terminal namespace

* Undid attestation change

* clang-format fixes

* Renamed Azure::Core::Amqp::Value to Azure::Core::Amqp::AmqpValue because Value is too generic a type name when you have 'using namespace' directives

* clang-format

* clang-format2

* turn crash into failure

* Try using async send rather than sync send for responding to CBS operations

* It helps to build the code you write

* Trigger CI

---------

Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: Konrad Jamrozik <[email protected]>
Co-authored-by: Victor Vazquez <[email protected]>
Co-authored-by: Daniel Jurek <[email protected]>
Co-authored-by: Ben Broderick Phillips <[email protected]>
benbp added a commit that referenced this pull request Apr 6, 2023
* Sync eng/common directory with azure-sdk-tools for PR 5608 (#4411)

* ongoing

* Move get-codeowners scripts and tests to their own dirs

* address PR remarks

* Fix CodeOwnerFileLocation path, fix casing, and dedup param defaults

* fix param names

* add todos on needed changes in cpp repo

* Add CodeownersFileLocation to Get-Codeowners and use $null for default param values

* move get-codeowners back to scripts/ and rename -functions to .lib

* fix: use empty string as defaults instead of $nulls, to fix invocation

* fix bug with invocation of Get-Codeowners + add support for passing IncludeNonUserAliases as switch

* fix path iin Metadata-Helpers.ps1

* fix typo

* Update archetype-cpp-release.yml

---------

Co-authored-by: Konrad Jamrozik <[email protected]>

* First attempt at fixing nlohmann doctest compilation error (#4499)

* First attempt at fixing nlohmann doctest compilation error

* fixed compilation issue

* Fixed ApiView generation for azure core

* update doctest lib to 2.4.11 (#4501)

* Migrate Ubuntu 18 to 22 directly in the matricses (#4442)

* First cut at migrating Ubuntu 18 to 22 directly in the matricses

* Try 20.04

* Remove azure.list from apt configuration on 22.04

* Parens

* gpp-8 on Ubuntu 20.04

* Remove g++-5

* Move config changes to steps/

* Remove g++-5 from live tests

* Update eng/pipelines/templates/steps/fix-linux-1es-configs.yml

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Review feedback

* Finish renaming

* Setting line coverage target to 91%

---------

Co-authored-by: Ben Broderick Phillips <[email protected]>

* Initial population of azure-core-amqp

* cspell

* AMQP specific cspell dictionary

* CI pipeline fixes

* azure-uamqp-c cannot work on UWP builds

* Final cspell error; added amqp package to artifacts

* clang-format; added copyright notices to everything

* Added required files

* more CI fixes

* Renamed some files

* Renamed some files

* fixed case of amqp directory

* Renamed models directory

* Renamed network directory

* Most filenames are now lower case

* Hopefully fix CI pipeline failures

* Sort lines in cmakelists.txt

* Fixed name of message_target.cp

* Fixed name of transfer_instance.cpp

* Added preset for AMQP builds; reverted change from cmakesettings.json

* Add AMQP to doxygen generation; fixed clang-format issue

* Case folding fixes

* Correctly set up for DLL builds; removed diagnostic code

* Fixed socket listener to initialize platform

* clang-format

* clang-format

* clang-format; clang compilation fix

* Moved message sender implementation to shared pointer to enable copy and move semantics

* Don't forward declare enums with underlying type

* MOved MessageReceiver into an implementation class

* A couple of clang compiler fixes

* Moved all connection and session code to shared pointers

* Converted network callbacks to use events; restructured network to use shared pointer

* clang-format fixes

* case sensitivity

* clang fixes 4

random port for socket listener echo tests

clang-format

Random port for two more tests

Null event handler before destroying messagereceiver

clang-format

Random port for ReceiverOpenClose

Random port for LinkAttachDetach

Use random port for session tests

gcc warnings again 3

gcc warnings again 2

gcc warnings again

Print errno on socket listener start error

gcc warnings; enabled code coverage for amqp

clang-format

Missed file

clang fixes 8

clang fixes 7

clang fixes 6

clang fixes 5

clang fixes 3

clang fixes 2

clang fixes

* Catch exception thrown from worker thread

* Sample fixes

* Added context to wait for polled result

* clang build fix

clang build fix

* wait until listener thread completes before starting message receiver

* wait until listener thread completes before starting message receiver 2

* more thread cleanup changes

* more linux diagnostics 5

more linux diagnostics 13

more linux diagnostics 12

more linux diagnostics 11

more linux diagnostics 10

more linux diagnostics 9

more linux diagnostics 8

more linux diagnostics 7

more linux diagnostics 6

more linux diagnostics 4

more linux diagnostics 3

more linux diagnostics 2

more linux diagnostics

* Search for ports at 5000, not 0x5000; clang-format changes

* Fixed AV in MessageSenderReceiver when test is cancelled;

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* Diagnostics for FindAvailableSOcket

* pull request feedback

* Added cbs test

* Finished snake case rename

* Fixed copyright and license text

* Undid accidental checkin

* Moved common and models types to _internal terminal namespace

* Basic CBS authentication test

* removed test resource name

* Fixed SAS token samples

* Sanitized token writer sample

* CI pipeline fixes

* Clear event callback when destroying message sender; close sender and receiver when done with test thread

* ApiView fixes; clang-format

* Fixed a coupe of CI pipeline issues

* Moved credenial type to a member variable not a virtual method returnign a constant

* Full mock implementation of claims based security

* Renamed ClaimBasedSecurity to ClaimsBasedSecurity; clang-format; clang fixes

* Improved code coverage.

* clang fix

* unit test fix

* Improved code coverage for connection string tests

* Added test for authenticated sender and receiver

* Added message source and target tests

Use better constructor for tokencredential

Use better constructor for tokencredential

crlf at the end of every file; made ClaimBasedSecurity class a _detail class

More code coverage fixes

Improved coe coverage for amqp message

Message->Value conversion 3

Message->Value conversion 2

Try casting message to value

Try casting message to value

Reverted attestation chagnes 2

Added more tests

Fixed doxygen issue

Don't use sender unless sender is set

Removed incorrect test

* More code coverage fixes

* Removed 2 param ctor

* More code coverage

* clang-format

* Hopefully final code coverage fixes

* clang-format

* Moved all amqp types to _internal terminal namespace

* Undid attestation change

* clang-format fixes

* Renamed Azure::Core::Amqp::Value to Azure::Core::Amqp::AmqpValue because Value is too generic a type name when you have 'using namespace' directives

* clang-format

* clang-format2

* turn crash into failure

* Try using async send rather than sync send for responding to CBS operations

* It helps to build the code you write

* Trigger CI

---------

Co-authored-by: Azure SDK Bot <[email protected]>
Co-authored-by: Konrad Jamrozik <[email protected]>
Co-authored-by: Victor Vazquez <[email protected]>
Co-authored-by: Daniel Jurek <[email protected]>
Co-authored-by: Ben Broderick Phillips <[email protected]>
@LarryOsterman LarryOsterman deleted the larryo/fixdoctest branch January 24, 2024 21:38
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.

3 participants