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

Test more windows targets #452

Merged
merged 2 commits into from
Jul 16, 2024
Merged

Test more windows targets #452

merged 2 commits into from
Jul 16, 2024

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Jun 27, 2024

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@justsmth justsmth requested a review from a team as a code owner June 27, 2024 13:16
@codecov-commenter
Copy link

codecov-commenter commented Jun 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.83%. Comparing base (c358484) to head (eff8249).
Report is 37 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #452      +/-   ##
==========================================
- Coverage   95.80%   92.83%   -2.97%     
==========================================
  Files          61       62       +1     
  Lines        8143     8626     +483     
  Branches        0     8626    +8626     
==========================================
+ Hits         7801     8008     +207     
- Misses        342      362      +20     
- Partials        0      256     +256     

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

@justsmth justsmth force-pushed the more-windows-tests branch 9 times, most recently from 7022c42 to 0107064 Compare June 28, 2024 13:03
@justsmth justsmth force-pushed the more-windows-tests branch 3 times, most recently from 37884d0 to 21cc839 Compare July 1, 2024 14:14
.github/workflows/cross.yml Outdated Show resolved Hide resolved
@justsmth justsmth force-pushed the more-windows-tests branch 12 times, most recently from e37c74e to e41924b Compare July 1, 2024 18:32
@justsmth
Copy link
Contributor Author

It appears that the symbols provided by the assembly code are not getting prefixed:

% dumpbin //EXPORTS //SYMBOLS  ./target/aarch64-pc-windows-msvc/debug/build/aws-lc-sys-bdeeea1c963418e6/out/build/artifacts/aws_lc_0_19_0_crypto.lib | grep External | grep SECT1 | egrep -v aws_lc | grep 'bn_'
006 00000000 SECT1  notype       External     | bn_add_words
00A 00000050 SECT1  notype       External     | bn_sub_words
006 00000000 SECT1  notype ()    External     | bn_mul_mont
% dumpbin //EXPORTS //SYMBOLS  ./target/aarch64-pc-windows-msvc/debug/build/aws-lc-sys-bdeeea1c963418e6/out/build/artifacts/aws_lc_0_19_0_crypto.lib | grep External | grep SECT1 | grep 'aws_lc' | head -n 5
047 00000000 SECT13 notype ()    External     | aws_lc_0_19_0_aes_nohw_ctr32_encrypt_blocks
04C 00000000 SECT14 notype ()    External     | aws_lc_0_19_0_aes_nohw_cbc_encrypt
051 00000000 SECT15 notype ()    External     | aws_lc_0_19_0_AES_wrap_key
056 00000000 SECT16 notype ()    External     | aws_lc_0_19_0_AES_unwrap_key
05B 00000000 SECT17 notype ()    External     | aws_lc_0_19_0_AES_wrap_key_padded

@justsmth justsmth force-pushed the more-windows-tests branch 2 times, most recently from c297c02 to fc8178a Compare July 11, 2024 12:50
@justsmth
Copy link
Contributor Author

It appears that the symbols provided by the assembly code are not getting prefixed:

I was missing the required include directory when building the assembly files. I think I'm closing in on the solution now.

justsmth added a commit to aws/aws-lc that referenced this pull request Jul 15, 2024
### Issues:
* Addresses: aws/aws-lc-rs#453

### Description of changes: 
* When building for Windows/ARM64 with the "Visual Studio" generator
(instead of Ninja) the "*.S" assembly source files for libraries are
ignored.
* This change provides a custom command to assemble the require objects
files and adds them to the list of sources for the library.

### Call-outs:
* See related aws-lc-rs PR: aws/aws-lc-rs#452

### Testing
* I've added CI tests to build for Windows using the "Visual Studio"
generator.


By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
@justsmth justsmth marked this pull request as ready for review July 15, 2024 19:34
@justsmth justsmth force-pushed the more-windows-tests branch 2 times, most recently from 506eb4e to 433dbc2 Compare July 15, 2024 19:52
.github/workflows/cross.yml Show resolved Hide resolved
@justsmth justsmth merged commit eeac93b into aws:main Jul 16, 2024
189 of 195 checks passed
@justsmth justsmth deleted the more-windows-tests branch July 16, 2024 18:59
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.

5 participants