Skip to content

ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh #4749

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.

Sign up for GitHub

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

Jump to bottom
Closed
shiro615 wants to merge 9 commits into from
Closed

ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh #4749

shiro615 wants to merge 9 commits into from

Conversation

@shiro615
Copy link
Contributor

@shiro615 shiro615 commented Jun 29, 2019

c_glib/ source archive is generated by make dist because includes configure script.
The current dev/release/02-source.sh build Arrow C++ and Arrow GLib to include the artifacts of GTK-Doc and then run make dist. But it is slow.
So this PR run only c_glib/autogen.sh and then replace c_glib/.

shiro615
shiro615 commented Jun 29, 2019
View reviewed changes
dev/release/source/Dockerfile Outdated
Copy link
Contributor Author

@shiro615 shiro615 Jun 29, 2019

Choose a reason for hiding this comment

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

I've removed the libraries for building Arrow C++, because just runs c_glib/autogen.sh.

shiro615
shiro615 commented Jun 29, 2019
View reviewed changes
ci/travis_release_test.sh Outdated
Copy link
Contributor Author

@shiro615 shiro615 Jun 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I've enabled the test for GLib configure.
But if you don't want to enable this option, I will remove this.

Copy link
Member

@kou kou Jul 3, 2019

Choose a reason for hiding this comment

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

It just increase just 1min. We can always run this test now.
Could you remove omit from 02-source-test.rb.?

Copy link
Contributor Author

@shiro615 shiro615 Jul 3, 2019

Choose a reason for hiding this comment

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

I've removed it.

Copy link
Member

@kou kou Jul 3, 2019

Choose a reason for hiding this comment

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

Could you also remove this change?

Copy link
Contributor Author

@shiro615 shiro615 Jul 4, 2019

Choose a reason for hiding this comment

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

Oh...sorry. I've removed this change.

@codecov-io
Copy link

codecov-io commented Jun 29, 2019 •
edited
Loading

Codecov Report

Merging #4749 into master will decrease coverage by 0.54%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4749      +/-   ##
==========================================
- Coverage   86.44%   85.89%   -0.55%     
==========================================
  Files         992      881     -111     
  Lines      138020   115177   -22843     
  Branches     1418     1418              
==========================================
- Hits       119311    98936   -20375     
+ Misses      18347    15879    -2468     
  Partials      362      362
Impacted Files Coverage Δ
cpp/src/arrow/array/builder_union.cc 0% <0%> (-100%) ⬇️
cpp/src/arrow/csv/reader.h 0% <0%> (-100%) ⬇️
cpp/src/arrow/csv/reader.cc 0% <0%> (-92.9%) ⬇️
cpp/src/arrow/adapters/orc/adapter_util.cc 15.74% <0%> (-64.97%) ⬇️
cpp/src/arrow/array/builder_union.h 0% <0%> (-61.91%) ⬇️
cpp/src/arrow/util/cpu-info.h 0% <0%> (-50%) ⬇️
cpp/src/arrow/csv/options.h 66.66% <0%> (-33.34%) ⬇️
cpp/src/arrow/extension_type.cc 66.66% <0%> (-26.67%) ⬇️
cpp/src/parquet/column_scanner.cc 55% <0%> (-25%) ⬇️
cpp/src/arrow/vendored/xxhash/xxhash.c 52.79% <0%> (-20.82%) ⬇️
... and 179 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d25902...447f83e. Read the comment docs.

@shiro615 shiro615 changed the title ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh WIP ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh Jun 29, 2019
@shiro615 shiro615 changed the title WIP ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/autogen.sh in dev/release/02-source.sh Jun 29, 2019
@wesm
Copy link
Member

wesm commented Jul 3, 2019

Seems like a nice improvement. Will let @kou review

@kou
Copy link
Member

kou commented Jul 3, 2019

I'll take a look this after 0.14.0 RC0. Please wait a while.

kou
kou requested changes Jul 3, 2019
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

Could you check my comments?
I've added a commit to use docker-compose.

dev/release/rat_exclude_files.txt Outdated
Copy link
Member

@kou kou Jul 3, 2019

Choose a reason for hiding this comment

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

Could you remove them after we run autogen.sh instead of adding them to archive?

Copy link
Contributor Author

@shiro615 shiro615 Jul 3, 2019

Choose a reason for hiding this comment

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

I've changed to remove them after running autogen.sh.

ci/travis_release_test.sh Outdated
Copy link
Member

@kou kou Jul 3, 2019

Choose a reason for hiding this comment

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

It just increase just 1min. We can always run this test now.
Could you remove omit from 02-source-test.rb.?

@kou kou force-pushed the release-replace-c-glib-after-running-autogen branch from 216fcda to 9a69f8e Compare July 5, 2019 00:17
kou
kou approved these changes Jul 5, 2019
View reviewed changes
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

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

+1
I've rebased on master. I'll merge this when CI is green.

@kou kou closed this in 9ffb9cd Jul 5, 2019
@shiro615 shiro615 deleted the release-replace-c-glib-after-running-autogen branch July 6, 2019 01:37
wesm pushed a commit that referenced this pull request Jul 13, 2019
@shiro615 @wesm
ARROW-5784: [Release][GLib] Replace c_glib/ after running c_glib/auto…
297c953 
…gen.sh in dev/release/02-source.sh

c_glib/ source archive is generated by `make dist` because includes configure script.
The current `dev/release/02-source.sh` build Arrow C++ and Arrow GLib to include the artifacts of GTK-Doc and then run `make dist`.  But it is slow.
So this PR run only `c_glib/autogen.sh` and then replace c_glib/.

Author: Yosuke Shiro <yosuke.shiro615@gmail.com>
Author: Sutou Kouhei <kou@clear-code.com>

Closes #4749 from shiro615/release-replace-c-glib-after-running-autogen and squashes the following commits:

9a69f8e <Yosuke Shiro> Remove an unnecessary environment variable
3a2550f <Yosuke Shiro> Remove omit from 02-source-test.rb
501a2dd <Yosuke Shiro> Remove autom4te.cache after running autogen.sh
46a4f89 <Sutou Kouhei> Use docker-compose
e357a88 <Yosuke Shiro> Exclude c_glib/autom4te.cache/* from RAT check
70cb4a7 <Yosuke Shiro> Remove an unnecessary diff
aa78680 <Yosuke Shiro> Enable test test_glib_configure on Travis CI
e04276e <Yosuke Shiro> Remove libraries for C++ build
56098ae <Yosuke Shiro>  Replace c_glib/ by c_glib/ after running autogen.sh
@asfimport asfimport mentioned this pull request Aug 1, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kou kou kou approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@shiro615 @codecov-io @wesm @kou