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

Support for crtsqlcppi and additional compile options #202

Merged

Conversation

sbestgen
Copy link
Collaborator

No description provided.

Copy link
Member

@edmundreinhardt edmundreinhardt left a comment

Choose a reason for hiding this comment

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

Thanks for the pull request please look at the comments and let's move it forward

src/mk/def_rules.mk Show resolved Hide resolved
@@ -231,6 +246,13 @@ SQLCIMOD_SYSIFCOPT := $(CMOD_SYSIFCOPT)
SQLCIMOD_TERASPACE := *YES *TSIFC
SQLCIMOD_TGTRLS := $(TGTRLS)

SQLCPPIMOD_DBGVIEW := *SOURCE
Copy link
Member

Choose a reason for hiding this comment

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

maybe this should not be hardcoded

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agree.

Copy link
Member

Choose a reason for hiding this comment

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

please commit a change to this in this branch/pull request

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

CURLIB :=
DBGVIEW := *ALL
DBGVIEW :=
Copy link
Member

Choose a reason for hiding this comment

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

do we need to change the default?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For debug view, it makes sense to NOT change the previous behavior. But I am interested in a way to built an entire project either for debug or optimized without needed to override that option at an object level.

Copy link
Member

Choose a reason for hiding this comment

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

Please update this in another commit and open an issue or discussion on how to have the same Rules.mk work for both debug and optimized

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done: Project level flag to build an entire project for a target debug level. #203

src/mk/def_rules.mk Outdated Show resolved Hide resolved
@$(PRESETUP) \
$(SCRIPTSPATH)/launch "$(JOBLOGFILE)" "$(crtcmd)" >> $(LOGFILE) 2>&1 && $(call logSuccess,$@) || $(call logFail,$@)
@($(call EVFEVENT_DOWNLOAD,$(basename $(@F)).evfevent); ret=$$?; rm $(DEPDIR)/$*.Td 2>/dev/null; rm "$<-1252" 2>/dev/null; exit $$ret)
@rm "$<-1252"
Copy link
Member

Choose a reason for hiding this comment

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

I see this file being deleted twice. What is this file for?
So does this work. I.e. the CPP front end is not a PASE executable that is called from an ILE command

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, it should only be deleted once. Good catch. As for the file, I replicated code from the CRTSQLCI path that (I presume) fixed a problem dealing with CCSID and the SQL precompiler. Perhaps this is code that should be revisited? Here is the comment from the code that I replicated: # Temp: Convert UTF-8 to temporary Windows Latin-1, because SQLC pre-compiler doesn't understand UTF-8

Copy link
Member

Choose a reason for hiding this comment

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

Right, the proper solution is to specify TGTCCSID on the compiler so that it converts to a target EBCDIC CCSID before compiling.
This is supported by RPGLE, CLLE, and CBLLE
But I don't know if CRTSQLCI supports it
1252 might not work for all languages, seems fragile to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like CRTSQLCI and CRTSQLCPPI both have this parm : Conversion CCSID (CVTCCSID) - Help

The CCSID used for conversion when a UTF-8 source stream
file is provided on the Source stream file (SRCSTMF)
parameter. Otherwise, this parameter is ignored.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Yes CVTCCSID is perfect
SO the value is derived from .ibmi.json
and is stored in TGTCCSID variable

@edmundreinhardt edmundreinhardt self-requested a review February 23, 2023 22:49
@edmundreinhardt
Copy link
Member

Bob-recursive compiles cleanly for me


Objects:             0 failed 109 succeed 109 total
Build Completed!

@edmundreinhardt edmundreinhardt merged commit d56f0f1 into master Mar 6, 2023
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