GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015

Merged
kou merged 2 commits into
apache:mainfrom
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged

GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom
kou:cpp-orc-static-protobuf

Conversation

@kou

@koukou commented Feb 9, 2024•
edited by github-actions Bot
Loading

Copy link
Copy Markdown
Member

Rationale for this change

We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.

If we don't link system libprotobuf.a, some symbols are missing.

What changes are included in this PR?

Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes.

@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou

kou commented Feb 9, 2024

Copy link
Copy Markdown
MemberAuthor

@github-actions crossbow submit java-jars

@github-actions

github-actionsBot commented Feb 9, 2024

Copy link
Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.

@github-actions

github-actionsBot commented Feb 9, 2024

Copy link
Copy Markdown

Revision: 62c750d

Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d

TaskStatus
java-jarsGitHub Actions

@kou

kou commented Feb 9, 2024

Copy link
Copy Markdown
MemberAuthor

Worked: #39919 (comment)

@koukou mentioned this pull request Feb 9, 2024
Closed
raulcd
raulcd reviewed Feb 9, 2024
View reviewed changes

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?

@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Feb 9, 2024
@kou

kou commented Feb 9, 2024

Copy link
Copy Markdown
MemberAuthor

My idea: #39919 (comment)

raulcd
raulcd approved these changes Feb 9, 2024
View reviewed changes

@raulcdraulcd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018

@github-actionsgithub-actionsBot added awaiting merge Awaiting merge and removed awaiting changes Awaiting changes labels Feb 9, 2024
danepitkin
danepitkin approved these changes Feb 9, 2024
View reviewed changes

@danepitkindanepitkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, @kou !

@vibhatha

vibhatha commented Feb 10, 2024

Copy link
Copy Markdown
Contributor

Thanks @kou for working on this 👍

@vibhathavibhatha mentioned this pull request Feb 10, 2024
Merged
kou
kou commented Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot added awaiting changes Awaiting changes and removed awaiting merge Awaiting merge labels Feb 10, 2024
@kou
kou merged commit f7dc37f into apache:mainFeb 10, 2024
@kou
kou deleted the cpp-orc-static-protobuf branch February 10, 2024 12:09
@koukou removed the awaiting changes Awaiting changes label Feb 10, 2024
@conbench-apache-arrow

conbench-apache-arrowBot commented Feb 10, 2024

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.

There were 2 benchmark results with an error:

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.

@davisusanibardavisusanibar mentioned this pull request Feb 12, 2024
Merged
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit that referenced this pull request Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he mentioned this pull request Jun 19, 2024
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes
+1 more reviewer
@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency - #40015
Merged
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Feb 10, 2024
Merged
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency#40015
kou merged 2 commits into
apache:mainfrom 
kou:cpp-orc-static-protobuf
Conversation
@kou
@koukou
commented
Feb 9, 2024•

edited by github-actions
Bot
Loading

Copy link


Copy Markdown

Member
Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
What changes are included in this PR?
Add missing ${ARROW_PROTOBUF_LIBPROTOBUF} to ARROW_STATIC_INSTALL_INTERFACE_LIBS.
Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.
@kou
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
62c750d
…dency
We need to link system libprotobuf.a too when we link libarrow.a, ORC
is enabled and system Protobuf is used.
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
@github-actions crossbow submit java-jars
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

⚠️ GitHub issue #39919has been automatically assigned in GitHub to PR creator.
@github-actions
github-actionsBot
commented
Feb 9, 2024

Copy link


Copy Markdown

Revision: 62c750d
Submitted crossbow builds: ursacomputing/crossbow @ actions-f7a2fd829d
TaskStatus
java-jarsGitHub Actions
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
Worked: #39919 (comment)
@koukou
mentioned this pull request
Feb 9, 2024

Closed

raulcd
raulcd
reviewed
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

I am good with the change but (maybe on a different PR/issue) is there any way we could have found the issue on the java-jars job or on other Java CI testing?
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting committer review
Awaiting committer review
labels
Feb 9, 2024
@kou
kou
commented
Feb 9, 2024

Copy link


Copy Markdown

MemberAuthor
My idea: #39919 (comment)
raulcd
raulcd
approved these changes
Feb 9, 2024
View reviewed changes
@raulcdraulcd
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks @kou ! I've create a new issue to track it here: #40018
@github-actionsgithub-actionsBot
added

awaiting merge
Awaiting merge
and removed

awaiting changes
Awaiting changes
labels
Feb 9, 2024
danepitkin
danepitkin
approved these changes
Feb 9, 2024
View reviewed changes
@danepitkindanepitkin
left a comment


Copy link


Copy Markdown

Member
Choose a reason for hiding this comment

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

Thanks, @kou !
@vibhatha
vibhatha
commented
Feb 10, 2024

Copy link


Copy Markdown

Contributor
Thanks @kou for working on this 👍
@vibhathavibhatha
mentioned this pull request
Feb 10, 2024
 Merged

kou
kou
commented
Feb 10, 2024
View reviewed changes
Comment threadcpp/CMakeLists.txt
@github-actionsgithub-actionsBot
added

awaiting changes
Awaiting changes
and removed

awaiting merge
Awaiting merge
labels
Feb 10, 2024
@kou
kou
merged commit f7dc37f
into
apache:mainFeb 10, 2024
@kou
kou
deleted the

cpp-orc-static-protobuf

branch
February 10, 2024 12:09
@koukou
removed
the 
awaiting changes
Awaiting changes
label
Feb 10, 2024
@conbench-apache-arrow
conbench-apache-arrowBot
commented
Feb 10, 2024

Copy link


Copy Markdown

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit f7dc37f.
There were 2 benchmark results with an error:
There were no benchmark performance regressions. 🎉
The full Conbench report has more details. It also includes information about 8 possible false positives for unstable benchmarks that are known to sometimes produce them.
@davisusanibardavisusanibar
mentioned this pull request
Feb 12, 2024
 Merged

dgreiss pushed a commit
to dgreiss/arrow
that referenced
this pull request
Feb 19, 2024
@kou@dgreiss
apacheGH-39919: [C++][Dataset] Add missing Protobuf static link depen…
86a514d
…dency (apache#40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: apache#39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
raulcd pushed a commit
that referenced
this pull request
Mar 12, 2024
@kou@raulcd
GH-39919: [C++][Dataset] Add missing Protobuf static link dependency (#…
ac4a643
…40015)
### Rationale for this change
We need to link system libprotobuf.a too when we link libarrow.a, ORC is enabled and system Protobuf is used.
If we don't link system libprotobuf.a, some symbols are missing.
### What changes are included in this PR?
Add missing `${ARROW_PROTOBUF_LIBPROTOBUF}` to `ARROW_STATIC_INSTALL_INTERFACE_LIBS`.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
* Closes: #39919
Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@philo-hephilo-he
mentioned this pull request
Jun 19, 2024
 Merged

Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@felipecrvfelipecrvfelipecrv approved these changes
@raulcdraulcdraulcd approved these changes
@danepitkindanepitkindanepitkin approved these changes

+1 more reviewer

@vibhathavibhathavibhatha approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees


No one assigned


Labels

Component: C++

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

[Java][Dataset] JNI Error when reading parquet file 

5 participants

@kou@vibhatha@felipecrv@raulcd@danepitkin