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

Refactor: Add namespace, enable sanitize builds #4692

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

talregev
Copy link
Contributor

@talregev talregev commented Dec 7, 2024

  • Add namespace
  • Enable sanitize builds
  • Refactor platform unittest external cmake

@talregev talregev requested a review from a team as a code owner December 7, 2024 14:47
CMakeLists.txt Outdated Show resolved Hide resolved
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.83%. Comparing base (d22ec11) to head (302abc8).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4692      +/-   ##
==========================================
+ Coverage   85.87%   86.83%   +0.95%     
==========================================
  Files          56       56              
  Lines       17361    17361              
==========================================
+ Hits        14909    15075     +166     
+ Misses       2452     2286     -166     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@talregev
Copy link
Contributor Author

talregev commented Dec 7, 2024

Errors when remove line:
install(FILES ${QUIC_BUILD_DIR}/inc/MsQuicEtw.h DESTINATION include)
https://github.com/microsoft/msquic/blob/main/src/inc/quic_trace.h#L255

C:\Program Files\msquic\include\quic_trace.h(255,10): error C1083: Cannot open include file: 'MsQuicEtw.h': No such file or directory [D:\a\msquic\msquic\build_external\msquicplatformtest.vcxproj]
  (compiling source file '../src/platform/unittest/PlatformTest.cpp')
  
C:\Program Files\msquic\include\quic_trace.h(255,10): error C1083: Cannot open include file: 'MsQuicEtw.h': No such file or directory [D:\a\msquic\msquic\build_external\msquicplatformtest.vcxproj]
  (compiling source file '../src/platform/unittest/CryptTest.cpp')
  
C:\Program Files\msquic\include\quic_trace.h(255,10): error C1083: Cannot open include file: 'MsQuicEtw.h': No such file or directory [D:\a\msquic\msquic\build_external\msquicplatformtest.vcxproj]
  (compiling source file '../src/platform/unittest/main.cpp')
  
C:\Program Files\msquic\include\quic_trace.h(255,10): error C1083: Cannot open include file: 'MsQuicEtw.h': No such file or directory [D:\a\msquic\msquic\build_external\msquicplatformtest.vcxproj]
  (compiling source file '../src/platform/unittest/DataPathTest.cpp')
  
  TlsTest.cpp
C:\Program Files\msquic\include\quic_trace.h(255,10): error C1083: Cannot open include file: 'MsQuicEtw.h': No such file or directory [D:\a\msquic\msquic\build_external\msquicplatformtest.vcxproj]
  (compiling source file '../src/platform/unittest/TlsTest.cpp')

@talregev
Copy link
Contributor Author

talregev commented Dec 8, 2024

@nibanks From msquic api respective:
Did external user / library can define QUIC_EVENTS_STDOUT for msquic in their project? If yes, we should keep MsQuicEtw.h in the external include lib.
If not, I will remove the symbol QUIC_EVENTS_STDOUT and add QUIC_EVENTS_STUB manually, and remove MsQuicEtw.h from the external include lib.

Let me know what do you think.

- Add namespace
- Enable sanitize builds
- Refactor platform unittest external cmake
@talregev
Copy link
Contributor Author

talregev commented Dec 9, 2024

@nibanks I remove the MsQuicEtw.h from external header folder. Please review again. Thank you!

@talregev talregev requested a review from nibanks December 9, 2024 20:30
@talregev
Copy link
Contributor Author

@nibanks Can you approve the ci? Thank you for your time and effort for review my PR.

@nibanks nibanks merged commit 0d8e7df into microsoft:main Dec 10, 2024
481 checks passed
@talregev
Copy link
Contributor Author

@nibanks
Thank you! Can you also release this with 2.4.*?

@talregev talregev deleted the TalR/namespace branch December 10, 2024 13:58
@nibanks
Copy link
Member

nibanks commented Dec 10, 2024

@nibanks Thank you! Can you also release this with 2.4.*?

Please create a cherry-pick PR for it, and run .\scripts\update-version.ps1 -Part Patch.

@talregev
Copy link
Contributor Author

I run the script with errors. I am not sure why. I will create a PR without run this script.

@talregev
Copy link
Contributor Author

PS C:\src\msquic> .\scripts\update-version.ps1 -Part Patch
Join-Path : A positional parameter cannot be found that accepts argument 'inc'. This failure might be caused by
applying the default parameter binding. You can disable the default parameter binding in $PSDefaultParameterValues by
setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following default parameter was successfully
bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:27 char:22
+ $MsQuicVerFilePath = Join-Path $RootDir "src" "inc" "msquic.ver"
+                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Join-Path : A positional parameter cannot be found that accepts argument 'OneBranch.Package.yml'. This failure might
be caused by applying the default parameter binding. You can disable the default parameter binding in
$PSDefaultParameterValues by setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following
default parameter was successfully bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:28 char:24
+ ... teVPackFilePath = Join-Path $RootDir ".azure" "OneBranch.Package.yml"
+                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Join-Path : A positional parameter cannot be found that accepts argument 'package-nuget.ps1'. This failure might be
caused by applying the default parameter binding. You can disable the default parameter binding in
$PSDefaultParameterValues by setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following
default parameter was successfully bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:29 char:21
+ $NugetPackageFile = Join-Path $RootDir "scripts" "package-nuget.ps1"
+                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Join-Path : A positional parameter cannot be found that accepts argument 'package-distribution.ps1'. This failure
might be caused by applying the default parameter binding. You can disable the default parameter binding in
$PSDefaultParameterValues by setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following
default parameter was successfully bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:30 char:21
+ ... ibutionFile = Join-Path $RootDir "scripts" "package-distribution.ps1"
+                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Join-Path : A positional parameter cannot be found that accepts argument 'distribution'. This failure might be caused
by applying the default parameter binding. You can disable the default parameter binding in $PSDefaultParameterValues
by setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following default parameter was
successfully bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:31 char:22
+ ... meworkInfoFile = Join-Path $RootDir "src" "distribution" "Info.plist"
+                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Join-Path : A positional parameter cannot be found that accepts argument 'write-versions.ps1'. This failure might be
caused by applying the default parameter binding. You can disable the default parameter binding in
$PSDefaultParameterValues by setting $PSDefaultParameterValues["Disabled"] to be $true, and retry. The following
default parameter was successfully bound for this cmdlet when the error occurred: -ErrorAction
At C:\src\msquic\scripts\update-version.ps1:33 char:22
+ ... VersionsWriteFile = Join-Path $RootDir "scripts" "write-versions.ps1"
+                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (:) [Join-Path], ParameterBindingException
    + FullyQualifiedErrorId : PositionalParameterNotFound,Microsoft.PowerShell.Commands.JoinPathCommand

Select-String : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:38 char:34
+ $VerMajor = (Select-String -Path $MsQuicVerFilePath "#define VER_MAJO ...
+                                  ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Select-String], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.SelectStrin
   gCommand

Select-String : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:39 char:34
+ $VerMinor = (Select-String -Path $MsQuicVerFilePath "#define VER_MINO ...
+                                  ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Select-String], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.SelectStrin
   gCommand

Select-String : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:40 char:34
+ $VerPatch = (Select-String -Path $MsQuicVerFilePath "#define VER_PATC ...
+                                  ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Select-String], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.SelectStrin
   gCommand

Current version: ..
    New version: 0.0.1
Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:56 char:14
+ (Get-Content $MsQuicVerFilePath) `
+              ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:61 char:14
+ (Get-Content $CreateVPackFilePath) `
+              ~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:70 char:14
+ (Get-Content $NugetPackageFile) `
+              ~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:73 char:14
+ (Get-Content $FrameworkInfoFile) `
+              ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:76 char:14
+ (Get-Content $DistributionFile) `
+              ~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

Get-Content : Cannot bind argument to parameter 'Path' because it is null.
At C:\src\msquic\scripts\update-version.ps1:79 char:14
+ (Get-Content $VersionsWriteFile) `
+              ~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidData: (:) [Get-Content], ParameterBindingValidationException
    + FullyQualifiedErrorId : ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.GetContentC
   ommand

talregev added a commit to talregev/msquic that referenced this pull request Dec 10, 2024
- Add namespace
- Enable sanitize builds
- Refactor platform unittest external cmake
@nibanks
Copy link
Member

nibanks commented Dec 10, 2024

Make sure you're using a newer version of powershell (7+).

@talregev
Copy link
Contributor Author

Make sure you're using a newer version of powershell (7+).

@nibanks
I upgrade to poweshell 7, and run the script, and create a PR:
#4701

@talregev talregev mentioned this pull request Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants