Skip to content

Fix the license header of hkdf #196

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

Merged
merged 1 commit into from
Aug 16, 2019

Conversation

RonEld
Copy link
Contributor

@RonEld RonEld commented Jul 31, 2019

Change the license header of hkdf.h to a format the that script
apache_to_gpl.pl knows how to parse.

@RonEld RonEld added bug Something isn't working needs: review The pull request is ready for review. This generally means that it has no known issues. labels Jul 31, 2019
@RonEld RonEld requested a review from Patater July 31, 2019 12:22
Patater
Patater previously approved these changes Jul 31, 2019
@Patater Patater added needs: ci Needs a passing full CI run and removed needs: review The pull request is ready for review. This generally means that it has no known issues. labels Jul 31, 2019
@jainvikas8 jainvikas8 self-requested a review August 9, 2019 11:17
@jainvikas8
Copy link
Contributor

Please could you let me know how to reproduce this issue? (invoke the script - apache_to_gpl.pl)
Also there seems to be failure in TLS testing

test_no_use_psa_crypto_full_cmake_asan: test: ssl-opt.sh (full minus >MBEDTLS_USE_PSA_CRYPTO): tests/ssl-opt.sh -> 1
Is this an known issue?

The copyright year information, Is that allowed?

@RonEld
Copy link
Contributor Author

RonEld commented Aug 11, 2019

@jainvikas8
The script that changes the header to GPL is in our web site repository.
It is generate_license.pl , however internally, apache_to_gpl.pl is called, which does the parsing of the header.

The copyright year information, Is that allowed?

Well, AFAIK, in Apache, every time a file is modified, its copyright year should be modified as well. Although I don't consider this change significant enough to merit an update in copyright year, this file has been changed during 2019 in other commits, so I modified the year as well. In fact, I am not sure the starting year should be 2016, as this file was introduced on 2018.

@RonEld
Copy link
Contributor Author

RonEld commented Aug 13, 2019

steps to reproduce:

  1. cp hkdf.h and another header file to a temporary location
  2. Run the apache_to_gpl.pl script:
./apache_to_gpl.pl . <hkdf.h file path> 

The license header will not change
3. . Run the same script on the other header file, and you will see the license header has changed

@jainvikas8
Copy link
Contributor

steps to reproduce:

1. cp `hkdf.h` and another header (fixed) file to a temporary location

2. Run the `apache_to_gpl.pl` script:
./apache_to_gpl.pl . <hkdf.h file path> 

The license header will not change
3. . Run the same script on the other header (fixed) file, and you will see the license header has changed

Additional note : This issue is seen on Mbed TLS 2.16.2 Apache.

@RonEld
Copy link
Contributor Author

RonEld commented Aug 13, 2019

test_no_use_psa_crypto_full_cmake_asan: test: ssl-opt.sh (full minus >MBEDTLS_USE_PSA_CRYPTO): tests/ssl-opt.sh -> 1
Is this an known issue?

Yes, this is a known flaky test issue with Gnu TLS DTLS client

Change the license header of `hkdf.h` to a format the that script
`apache_to_gpl.pl` knows how to parse.
@RonEld
Copy link
Contributor Author

RonEld commented Aug 13, 2019

I updated the copyright year to reflect the correct years that this file has been modified.
@jainvikas8 @Patater please review

@jainvikas8
Copy link
Contributor

Looks fine to me, able to reproduce the issue. @Patater please comment on the licensing year. Thanks.

@RonEld
Copy link
Contributor Author

RonEld commented Aug 15, 2019

@jainvikas8 As answered by legal, the copyright year should reflect every year the file was modified. This means that in this hkdf.h file, it should be 2018-2019 as modified in this PR

@Patater Patater removed the needs: ci Needs a passing full CI run label Aug 16, 2019
@Patater Patater merged commit c26591a into ARMmbed:development Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants