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

Add flag at prep stage to OMIT_DEPRECATED #355

Closed
skliper opened this issue Sep 30, 2019 · 7 comments · Fixed by #490
Closed

Add flag at prep stage to OMIT_DEPRECATED #355

skliper opened this issue Sep 30, 2019 · 7 comments · Fixed by #490

Comments

@skliper
Copy link
Contributor

skliper commented Sep 30, 2019

Need an easy way for users (and the CI system) to omit all deprecated elements. Something like

make OMIT_DEPRECATED=true prep

which would then internally add all the -DCFE_OMIT_DEPRECATED -DOSAL_OMIT_DEPRECATED and so on flags when building everything.

@skliper skliper added this to the 6.7.1 milestone Sep 30, 2019
@skliper skliper self-assigned this Sep 30, 2019
@skliper skliper added the bug label Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Sep 30, 2019

Imported from trac issue 324. Created by jhageman on 2019-08-23T17:07:17, last modified: 2019-08-23T17:07:17

@skliper skliper removed their assignment Sep 30, 2019
@skliper skliper modified the milestones: 6.7.1, 6.7.0, 6.8.0 Sep 30, 2019
@skliper
Copy link
Contributor Author

skliper commented Nov 1, 2019

@jphickey do you have a suggestion for this one? I quickly jammed in:

  # Omit deprecated elements if set
  if (OMIT_DEPRECATED)
    add_definitions(-DCFE_OMIT_DEPRECATED_6_6 -DOSAL_OMIT_DEPRECATED)
  endif (OMIT_DEPRECATED)

to the prepare function of mission_build.cmake so I could move forward with CI improvements locally. Didn't cache but easy enough to add.

@jphickey
Copy link
Contributor

jphickey commented Nov 1, 2019

I recommend just setting OSAL_USER_C_FLAGS when doing the initial prep, which allows one to add your own flags.

i.e. make SIMULATION=native OSAL_USER_C_FLAGS='-DCFE_OMIT_DEPRECATED_6_6 -DOSAL_OMIT_DEPRECATED -Wall -Werror -std=c99 -pedantic' prep

Shouldn't need to edit files. This is how I generally test it out. This is also cached so it is retained as part of the build tree.

@skliper
Copy link
Contributor Author

skliper commented Nov 1, 2019

I am really not a fan of doing it that way. OSAL_USER_C_FLAGS is not consistently applied everywhere, I think I've either emailed you or commented somewhere about previous issues with this technique. I'd much rather from a top level add these definitions explicitly, and not require the user or CI to know what the actual define values are.

@skliper skliper added enhancement and removed bug labels Nov 4, 2019
@skliper
Copy link
Contributor Author

skliper commented Nov 4, 2019

See #24 for some other issues with OSAL_USER_CFLAGS. Not a fan of make SIMULATION=native OSAL_USER_C_FLAGS='-DCFE_OMIT_DEPRECATED_6_6 -DOSAL_OMIT_DEPRECATED -Wall -Werror -std=c99 -pedantic' prep since a layer of abstraction (like "OMIT_DEPRECATED") means I wouldn't have to retrain users whenever a flag changes or we decide to add or remove from the warning set. Also allows us to work towards compliance (define an initial set, and add as needed) with only requiring to update in one spot... we've discussed strict-protoypes and cast-align in the past.

@jphickey
Copy link
Contributor

jphickey commented Nov 5, 2019

Yes, the top-level wrapper makefile is another place that we could put logic like this. However it was never required that projects use this wrapper, it is just a convenient way to build with a single "make" and not be concerned with the specific CMake invocation to generate your makefiles.

That being said, we probably do need to have some sort of document (or at least a paragraph in the README) that indicates the name and meaning of the various configure-time options in the build system for that version of code.

@skliper
Copy link
Contributor Author

skliper commented Nov 7, 2019

CCB 20191106 - discussed as high priority so simple CI improvements can move forward (matrix jobs to build both with and without deprecated elements) without hard coding defines in yml scripts

skliper added a commit to skliper/cFE that referenced this issue Jan 7, 2020
Adds STRICT_NO_WARNINGS and OMIT_DEPRECATED
prep options for CI and as example build
skliper added a commit to skliper/cFE that referenced this issue Jan 8, 2020
Adds STRICT_NO_WARNINGS and OMIT_DEPRECATED
prep options for CI and as example build
jphickey added a commit to jphickey/cFE that referenced this issue Jan 8, 2020
Adds a "global_build_options.cmake" file akin to the existing
arch_build/mission_build option files.  Include an example of
this file that optionally does add_definitions() to omit the
deprected elements for build testing.
skliper added a commit that referenced this issue Jan 21, 2020
Fix #355, Add global scope option to omit deprecated items
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants