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

Build (windows) with old MSVC compilers (was available before OSS… #24209

Closed
wants to merge 0 commits into from

Conversation

yjh-styx
Copy link

@yjh-styx yjh-styx commented Apr 19, 2024

…L v3.3)

1. VC++ 2008 or earlier x86 compilers do not have inline implementation
    not only for InterlockedOr64, also for InterlockedAdd64 and
    InterlockedAnd64 (used in OSSL v3.3+)
2. VC++ 2008 or earlier x86 compilers not map _InterlockedOr to
    InterlockedOr
3. VC++ 2015 or earlier compilers not support 'inline' in C (nor C++) mode -
    only __inline

fix all this and remove legacy (non-trivial) variant of InterlockedOr64

@t8m t8m added hold: cla required The contributor needs to submit a license agreement cla: trivial One of the commits is marked as 'CLA: trivial' branch: master Merge to master branch triaged: feature The issue/pr requests/adds a feature labels Apr 19, 2024
@t8m
Copy link
Member

t8m commented Apr 19, 2024

This is definitely not within CLA: trivial limits. Please submit a regular CLA and remove t the CLA: trivial annotation from the commit message.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Apr 19, 2024
@yjh-styx
Copy link
Author

pdf sended to legal@opensslfoundation.org

@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Apr 19, 2024
@t8m
Copy link
Member

t8m commented Apr 25, 2024

@yjh-styx please drop the merge commits. I assume you're adding them because you use a master branch in your forked repo for this pull request. This is not recommended, you should create a separate branch in your forked repository for any pull requests instead.

@github-actions github-actions bot added the severity: ABI change This pull request contains ABI changes label Apr 25, 2024
@t8m
Copy link
Member

t8m commented Apr 25, 2024

Assuming you have your forked repo master branch checked out, you could do something like:

$ git reset --hard HEAD~~
$ git checkout -b old-msvc-builds
$ git push --set-upstream origin old-msvc-builds

Now you can create a new pull request from the old-msvc-builds branch.

You can then also reset the master branch to not diverge from the upstream master.

$ git checkout master
$ git reset --hard HEAD~
$ git push --force

@tom-cosgrove-arm
Copy link
Contributor

@yjh-styx did you mean to close this PR?

@yjh-styx
Copy link
Author

yjh-styx commented Apr 25, 2024

@yjh-styx did you mean to close this PR?

New PR is #24262 (rebuild as described by t8m)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch: master Merge to master branch cla: trivial One of the commits is marked as 'CLA: trivial' severity: ABI change This pull request contains ABI changes severity: fips change The pull request changes FIPS provider sources triaged: feature The issue/pr requests/adds a feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants