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

generate_stubs.pl generated files put the comments from the .h file instead of identifying the stub file name #1192

Closed
asgibson opened this issue Nov 3, 2021 · 6 comments
Labels

Comments

@asgibson
Copy link
Contributor

asgibson commented Nov 3, 2021

Is your feature request related to a problem? Please describe.
The top commenting from the header file being put into the stub file is stating that the generated stub file is the .h, because that is what the header states.

Describe the solution you'd like
Have the generated file identify itself as the stub file and if the .h comments are to be kept, have it identify them as such.

Describe alternatives you've considered
Manually fixing it on every generated file

Additional context
N/A

Requester Info
Alan Gibson NASA/GSFC 587

@jphickey
Copy link
Contributor

jphickey commented Nov 9, 2021

The script is expecting that the first block comment in the source/header file is a legal/license boilerplate, as it is in all CFE/OSAL source files. This says whether the file is Apache open source or if its restricted/unreleased source, and this info should be preserved exactly as-is in the generated derivative file as the same permissions/restrictions should be given to that file as was on the original.

File-specific info should occur in a separate doxygen-style block comment that follows the legal block. The script should insert its own doxygen block with a @file tag that indicates it is an auto-generated file and refer back to the original source.

This sounds like maybe the source file is not following the pattern, and it has additional info in the first comment, not just legal/boilerplate stuff?

@skliper
Copy link
Contributor

skliper commented Nov 9, 2021

first block comment in the source/header file is a legal/license boilerplate...

Definitely second this approach. Managing that legal/license boilerplate header block is much easier if it is stand-alone. I converted the majority of the framework back in Aquila and it makes that info much easier to update/manage.

@asgibson
Copy link
Contributor Author

asgibson commented Nov 9, 2021

Understood.

I will amend my solution then:
Have the script output when the header is not found to be in the proper format.

@skliper
Copy link
Contributor

skliper commented Nov 9, 2021

"Proper format" can be subjective and can vary across projects in terms of what that content actually is. For those who want to apply a specific legal/license boilerplate header block or verify a file's compliance I recommend tools specific to that task.

@asgibson
Copy link
Contributor Author

asgibson commented Nov 10, 2021

The script just takes whatever is in the header file and puts it at the top of the stub file. It does add the @file tag and information, that was not in question. I was just surprised to see that the header for my new stub file was just a copy of the header, but the documentation for this tool does state this is what it does.

I was updating an old app (which is what this tool is meant to do?) which does not have the current standard of header licensing information. However, your comments have given me the correct flow:
Manually update (i.e. automate myself) the header information on the files to current licensing standard.
Then run the tool to generate stub files.

Therefore, problem solved.

@jphickey
Copy link
Contributor

Worth noting, it also should not be too hard to add some sort of --no-boilerplate command line option to have the script not copy the first comment block verbatim into the output. Someone with spare time could perhaps implement this, maybe put it on the nice-to-have list for the next release... IMO the better solution is to update the code to have a separate boilerplate block, as you did, but such an option could be helpful for legacy apps.

jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
…fined

Fix nasa#1192, Initialize TotalMsgSize in CFE_SB_GetUserDataLength
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants