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

Compilation fix for Windows ( and some small cleanups ) #212

Merged
merged 5 commits into from
Apr 24, 2019

Conversation

tomasjura
Copy link
Contributor

Hi,
I was not able to compile the package on windows, there was a typo. Corrected in commit e56aca1

During the compilation there was some warnings. Corrected in commits 5caa3a0, 5aa6b66, d9980b2

Using fiddler for Win32 calls instead Win32API package. (commit b7beed1).

README.md file update so that the links on github page will be correctly displayed. commit 803793b and 5810488

@kubo
Copy link
Owner

kubo commented Apr 21, 2019

Thanks!

Could you drop some commits by git rebase -i ... and push rewritten commit history forcedly by git push -f?
https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#_changing_multiple

I was not able to compile the package on windows, there was a typo. Corrected in commit e56aca1

I accept this commit. 0f43bf7 broken compilation on Windows. I forgot to check it. Thanks.

During the compilation there was some warnings. Corrected in commits 5caa3a0, 5aa6b66, d9980b2

I accept 5aa6b66 and d9980b2.

What warning do you get without 5caa3a0? What compiler do you use?
NORETURN has been set already in the following header file.

ruby-oci8/ext/oci8/oci8.h

Lines 427 to 431 in 0f43bf7

NORETURN(void oci8_do_env_raise(OCIEnv *envhp, sword status, int free_envhp, const char *file, int line));
NORETURN(void oci8_do_raise_init_error(const char *file, int line));
sb4 oci8_get_error_code(OCIError *errhp);
VALUE oci8_get_error_message(ub4 msgno, const char *default_msg);
NORETURN(void oci8_do_raise_by_msgno(ub4 msgno, const char *default_msg, const char *file, int line));

As far as I remember, 5caa3a0 causes compilation error on some platforms. (old compiler? I forget what platforms they are.)

Using fiddler for Win32 calls instead Win32API package. (commit b7beed1).

I accept this. Thanks.

README.md file update so that the links on github page will be correctly displayed. commit 803793b and 5810488

I don't accept them. They make incorrect links in https://www.rubydoc.info/github/kubo/ruby-oci8.

@tomasjura tomasjura force-pushed the master branch 2 times, most recently from 17b7e5f to 2548130 Compare April 23, 2019 22:15
@tomasjura
Copy link
Contributor Author

Hi I did the rebase and some corrections.

The correct remediation for the commit 5caa3a0 ( NORETURN ) seems to be to add the function prototype to header fille - commit 2548130. (Used compiler is GCC version 7.4.0.)
Actually the function oci8_do_raise seems not to be used anywhere.

zsh>grep 'oci8_do_raise\b' **/*.[ch]
ext/oci8/error.c:void oci8_do_raise(OCIError *errhp, sword status, OCIStmt *stmthp, const char *file, int line)
ext/oci8/oci8.h:#define oci8_raise(err, status, stmt) oci8_do_raise(err, status, stmt, __FILE__, __LINE__)
ext/oci8/oci8.h:NORETURN(void oci8_do_raise(OCIError *errhp, sword status, OCIStmt *stmthp, const char *file, int line));

I need to extend the ruby driver by XA transaction support ( OCITransStart, OCITransPrepare, OCITransForget, ...). Did you noticed the XA transaction support requirement any time before?

@kubo kubo merged commit abeeccc into kubo:master Apr 24, 2019
@kubo
Copy link
Owner

kubo commented Apr 24, 2019

Thanks. It was merged.

I need to extend the ruby driver by XA transaction support ( OCITransStart, OCITransPrepare, OCITransForget, ...). Did you noticed the XA transaction support requirement any time before?

No. I have not used it and I had not been requested to support XA transaction.

@kubo
Copy link
Owner

kubo commented Apr 25, 2019

@tomasjura
Do you start extending the ruby driver? If no, I could do it next weekend.
Well, I don't know real-world use cases about XA transaction. So it will be straight mapping from OCI functions to ruby methods.

@tomasjura
Copy link
Contributor Author

I started to study the xa resource manager interface ( methods in xa_switch_t structure ) and their relation to OCITrans* methods. I'm on the beginning, the relation are not very clear to me. Especially the recovery after an (unexpected) machine/process restart.
Let me check the other participants for the XA transaction ( https://github.com/reidmorrison/rubywmq ).
When the feasibility study will be done, I will know if I can go in my project with ruby XA or have to use Java XA. To be continued...

@kubo
Copy link
Owner

kubo commented Apr 26, 2019

I started to study the xa resource manager interface ( methods in xa_switch_t structure ) and their relation to OCITrans* methods.

I guess that they are similar but unrelated. OCITrans* functions looks available without XA.

I found an example using xa_switch_t in old document. It is documented as Oracle XA. In the document it is stated that "Oracle Call Interface applications that use the Oracle XA library must not call OCISessionBegin to log on to the resource manager."

On the other hand, examples about OCITransStart use OCISessionBegin, which could not be called with the Oracle XA library.

I won't support Oracle XA in ruby-oci8 because it will need many modifications. On the other hand supporting OCITrans* functions will be relatively easy.

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