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

Introduce IDE-FIPS directory #8126

Closed
wants to merge 1 commit into from
Closed

Conversation

gojimmypi
Copy link
Contributor

Description

Upon reconsideration after the merge of #8090, I think it would be best to move all project files to the respective IDE/<environment> directory, out of the respective source directories such as wolfcrypt/benchmark.

Additionally, it may be best to isolate FIPS and non-FIPS environments.

This PR proposes a new IDE-FIPS for all the FIPS-related builds in preparation for the moves.

Fixes zd# n/a

Testing

Not tested.

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@dgarske dgarske removed their request for review October 30, 2024 17:56
@dgarske dgarske removed their assignment Oct 30, 2024
@gojimmypi
Copy link
Contributor Author

Jenkins retest this please

Copy link
Contributor

@kaleb-himes kaleb-himes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent prep work on getting the transition space ready @gojimmypi, thank you!

Copy link
Contributor

@douzzer douzzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it may be an unforced error to separate FIPS from non-FIPS artifacts for a given IDE.

My thinking is that the big variable here is the IDE, not FIPSness, and we go to a lot of trouble to assure that things work the same API-wise, FIPS vs non-FIPS, modulo the reduced set of available crypto algorithms.

I could be convinced otherwise, but currently I find it more intuitive to have side-by-side FIPS and non-FIPS project files grouped under the same IDE.

@douzzer douzzer assigned gojimmypi and unassigned wolfSSL-Bot Dec 17, 2024
@douzzer douzzer removed the request for review from bandi13 December 17, 2024 05:26
@gojimmypi
Copy link
Contributor Author

My thinking is that the big variable here is the IDE, not FIPSness,

Those are both big variables. The project files are duplicated and suffixed with -FIPS in the same directory. I'm proposing we just clarify which, exact files are needed for FIPS and non-FIPS with no overlap.

we go to a lot of trouble to assure that things work the same API-wise, FIPS vs non-FIPS, modulo the reduced set of available crypto algorithms.

I agree with all points. When I am making non-FIPS changes, for instance, I see the files in IDE/WIN and wonder which are FIPS or not; which may be included, perhaps inadvertently for FIPS and have undesired results? Is test.vcxproj FIPS, non-FIPS, or both?

I see the value in having a common 'user_settings.h`; that's the only key thing to share between FIPS and non-FIPS, eh? Maybe that should have a home of its own and shared between projects?

I think it may be an unforced error to separate FIPS from non-FIPS artifacts for a given IDE.

Curious. Do you have an example? My thoughts here are explicitly stand-alone environments in a clean, exclusive directory. Either it is FIPS or is not for a given IDE.

This is all in the context of my work on Visual Studio project files - in particular my plans to introduce more of them for various different configurations, and remove the others from being embedded in the source.

I could imagine an entire set of various project files all referring to a single, common user_settings.h file location, but not having all the project files cluttering the same directory.

For example: see my WIP wolfSSL Visual Studio cmake project file (wolfssl-VS2022-cmake.vcxproj) for Realm in my osp/realm/VS2022 dev branch and the related wolfssl-GlobalProperties.props file. Should that exact global properties file be referenced by both FIPS and non-FIPS project? I'd say no.

@kaleb-himes @lealem47 what do you think?

@JacobBarthelmeh
Copy link
Contributor

Reviewed these changes. My thinking is more aligned with @douzzer on this. I also think that in most IDE projects there should not be enough of a difference between a FIPS and non-FIPS build to merit a new IDE project. Just build configuration options, possibly linker file changes and handling a couple of FIPS API calls on initialization.

@gojimmypi
Copy link
Contributor Author

After feedback, I'm closing this unmerged. We can revisit later if there are compelling reasons for separation of IDE project files.

@gojimmypi gojimmypi closed this Dec 30, 2024
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.

7 participants