Skip to content

[SPARK-8027] [SPARKR] Add maven profile to build R package docs #6567

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

Closed
shivaram wants to merge 1 commit into from
Closed

[SPARK-8027] [SPARKR] Add maven profile to build R package docs #6567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Add maven profile to build R package docs 
Also use that profile in create-release.sh
  • Loading branch information
@shivaram
shivaram committed Jun 2, 2015
commit 8dc8ecf980a039ade098776b5211b5b686b1272e
23 changes: 23 additions & 0 deletions core/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,29 @@
</plugins>
</build>
</profile>
<profile>
<id>sparkr-docs</id>
Copy link
Member

@srowen srowen Jun 2, 2015

Choose a reason for hiding this comment

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

Why is this in core though? It doesn't look like the right place.
Shouldn't we have an R/pom.xml that handles this?

<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>exec-maven-plugin</artifactId>
<executions>
<execution>
<id>sparkr-pkg-docs</id>
<phase>compile</phase>
<goals>
<goal>exec</goal>
</goals>
</execution>
</executions>
<configuration>
<executable>..${path.separator}R${path.separator}create-docs${script.extension}</executable>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>

</project>
16 changes: 8 additions & 8 deletions dev/create-release/create-release.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,14 @@ if [[ ! "$@" =~ --skip-package ]]; then

# We increment the Zinc port each time to avoid OOM's and other craziness if multiple builds
# share the same Zinc server.
make_binary_release "hadoop1" "-Psparkr -Phadoop-1 -Phive -Phive-thriftserver" "3030" &
make_binary_release "hadoop1-scala2.11" "-Psparkr -Phadoop-1 -Phive -Dscala-2.11" "3031" &
make_binary_release "cdh4" "-Psparkr -Phadoop-1 -Phive -Phive-thriftserver -Dhadoop.version=2.0.0-mr1-cdh4.2.0" "3032" &
make_binary_release "hadoop2.3" "-Psparkr -Phadoop-2.3 -Phive -Phive-thriftserver -Pyarn" "3033" &
make_binary_release "hadoop2.4" "-Psparkr -Phadoop-2.4 -Phive -Phive-thriftserver -Pyarn" "3034" &
make_binary_release "mapr3" "-Pmapr3 -Psparkr -Phive -Phive-thriftserver" "3035" &
make_binary_release "mapr4" "-Pmapr4 -Psparkr -Pyarn -Phive -Phive-thriftserver" "3036" &
make_binary_release "hadoop2.4-without-hive" "-Psparkr -Phadoop-2.4 -Pyarn" "3037" &
make_binary_release "hadoop1" "-Psparkr -Psparkr-docs -Phadoop-1 -Phive -Phive-thriftserver" "3030" &
Copy link
Member

@srowen srowen Jun 2, 2015

Choose a reason for hiding this comment

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

Yeah, this is starting to make the standard build get out of hand. Now there are two required profiles to build R? these need to be part of R's module.

Copy link
Contributor Author

@shivaram shivaram Jun 2, 2015

Choose a reason for hiding this comment

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

Hmm - I agree that the number of flags is getting out of hand, but there are a couple of things I tried to separate out in the SparkR build that can probably be simplified

The reason we need this change was that R has an inbuilt help system where you can access a package's man pages. Now these man pages use another R package (roxygen2) to be generated. I tried to keep the binary build separate from the man pages build so that users could build them separately if required.

Now another way to do this might be to check if roxygen2 is installed in the binary build script and just build the man pages if it exists. This might help us get rid of the profile sparkr-docs -- Does that sound like a better solution ? I will make a follow up PR today

Copy link
Contributor

@pwendell pwendell Jun 2, 2015

Choose a reason for hiding this comment

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

Personally I don't mind if this is kept behind a flag, because most developers won't necessarily want to install a bunch of extra packages to just try and get SparkR building. We could just merge it under the sparkr profile if we want though, don't have strong feelings about it.

One other note, I've been prototyping a refactoring of these build scripts locally that I plan to contribute back after the 1.4 release. So any changes should be made there too:

https://github.com/pwendell/spark-utils/tree/master/new-release-scripts

make_binary_release "hadoop1-scala2.11" "-Psparkr -Psparkr-docs -Phadoop-1 -Phive -Dscala-2.11" "3031" &
make_binary_release "cdh4" "-Psparkr -Psparkr-docs -Phadoop-1 -Phive -Phive-thriftserver -Dhadoop.version=2.0.0-mr1-cdh4.2.0" "3032" &
make_binary_release "hadoop2.3" "-Psparkr -Psparkr-docs -Phadoop-2.3 -Phive -Phive-thriftserver -Pyarn" "3033" &
make_binary_release "hadoop2.4" "-Psparkr -Psparkr-docs -Phadoop-2.4 -Phive -Phive-thriftserver -Pyarn" "3034" &
make_binary_release "mapr3" "-Pmapr3 -Psparkr -Psparkr-docs -Phive -Phive-thriftserver" "3035" &
make_binary_release "mapr4" "-Pmapr4 -Psparkr -Psparkr-docs -Pyarn -Phive -Phive-thriftserver" "3036" &
make_binary_release "hadoop2.4-without-hive" "-Psparkr -Psparkr-docs -Phadoop-2.4 -Pyarn" "3037" &
wait
rm -rf spark-$RELEASE_VERSION-bin-*/

Expand Down