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

C/C++ Build scripts #513

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

C/C++ Build scripts #513

wants to merge 1 commit into from

Conversation

saipriya-m
Copy link

No description provided.

Signed-off-by: Saipriya M <[email protected]>
@dennis-behm
Copy link
Member

@saipriya-m Thank you for the contribution. I will review the updates this week.

Please make sure the DCO is in place, and that you have sent an email to Dan Bruce as outlined in :
https://github.com/IBM/dbb-zappbuild/blob/main/CONTRIBUTIONS.md

* createCParms - Builds up the c compiler parameter list from build and file properties
*/
def createCParms(String buildFile, LogicalFile logicalFile) {
def parms = props.getFileProperty('cc_compileParms', buildFile) ?: ""
Copy link
Member

Choose a reason for hiding this comment

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

@saipriya-m Your PR is missing a C.properties file that defines the Compile and link options the language script that is typically included in the application-conf directory.

Copy link
Author

Choose a reason for hiding this comment

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

@dennis-behm is there any different repo where i can push the application-conf/C.properties file?

Copy link
Member

Choose a reason for hiding this comment

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

Hi, within the zAppBuild samples/application-conf dir at https://github.com/IBM/dbb-zappbuild/tree/a719842f8b9738133e375eaade8497289c30fc05/samples/application-conf, we maintain all application-cons samples, including the README of the parameters.

Copy link
Member

@dennis-behm dennis-behm left a comment

Choose a reason for hiding this comment

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

Hi @saipriya-m

Thank you for the contribution and sharing your language script with the community.
I have added a couple of comments, that should be addressed before we can go ahead and merge it.

Please let me know if you need a hand.

Thanks
@dennis-behm

@Field def buildUtils= loadScript(new File("${props.zAppBuildDir}/utilities/BuildUtilities.groovy"))
@Field def impactUtils= loadScript(new File("${props.zAppBuildDir}/utilities/ImpactUtilities.groovy"))
@Field def bindUtils= loadScript(new File("${props.zAppBuildDir}/utilities/BindUtilities.groovy"))
@Field def resolverUtils = loadScript(new File("${props.zAppBuildDir}/utilities/ResolverUtilities.groovy"))
Copy link
Member

Choose a reason for hiding this comment

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

The resolverUtils are no longer part of zAppBuild


// iterate through build list
sortedList.each { buildFile ->
println "*** Building file $buildFile"
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add the logic for the Progress Indicator, that is part of the other language scripts?

sortedList.each { buildFile ->
println "*** Building file $buildFile"

// Check if this a testcase
Copy link
Member

Choose a reason for hiding this comment

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

IDZ zUnit does not provide support for C/C++ today. I think we can remove the conditions for zUnit from the language script.

if (logFile.exists())
logFile.delete()
MVSExec compile = createCompileCommand(buildFile, logicalFile, member, logFile)
MVSExec linkEdit = createLinkEditCommand(buildFile, logicalFile, member, logFile)
Copy link
Member

Choose a reason for hiding this comment

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

It's better to only construct the linkEdit MVSExec if the build file needsLinking. Let's move the construction of the linkEdit step into the condition, please.

if (buildUtils.isSQL(logicalFile))
parms = "$parms,$sql"

if (props.errPrefix)
Copy link
Member

Choose a reason for hiding this comment

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

Remote Error Feedback is only available for COBOL, PLI and Assembler. C/C++ is unfortunately not supported yet. Let's clean up the sections that leverage error feedback.

String linkEditStream = props.getFileProperty('cc_linkEditStream', buildFile)
String linkDebugExit = props.getFileProperty('cc_linkDebugExit', buildFile)

if (props.debug && linkDebugExit && doLinkEdit.toBoolean()){
Copy link
Member

Choose a reason for hiding this comment

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

In a previous zAppBuild release, we have simplified the management of the OBJs, and always allocate a dataset, that is referenced in the Link step instead of using a temp dataset. The contribution was made in #245

It is preferred to follow this structure for C/C++ as well.

String ssi = buildUtils.getShortGitHash(buildFile)
if (ssi != null) parms = parms + ",SSI=$ssi"
}

Copy link
Member

Choose a reason for hiding this comment

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

The link phase process has been updated with previous zAppBuild releases.

It is preferred to also make this consistent and add:

  • Ability to add IDENTIFY statement - see Use IDENTIFY to sign load modules #369
  • Reference the OBJ dataset, instead of temp dataset allocation - see previous comment
  • Leverage additional runtime flags, such as IMS

@saipriya-m
Copy link
Author

@dennis-behm
I will work on the requested changes and push the code again.

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.

2 participants