You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by majetideepak <gi...@git.apache.org> on 2017/11/27 23:49:33 UTC

[GitHub] orc pull request #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarbal...

GitHub user majetideepak opened a pull request:

    https://github.com/apache/orc/pull/193

    ORC-267: INSTALL NOTICES and LICENSE to binary tarball

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/majetideepak/orc ORC-267

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/orc/pull/193.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #193
    
----
commit 39d6a59a01406abe409a59297ac0eae7f445a80a
Author: Deepak Majeti <de...@hpe.com>
Date:   2017-11-27T23:48:48Z

    ORC-267: INSTALL NOTICES and LICENSE to binary tarball

----


---

[GitHub] orc pull request #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarbal...

Posted by jcrist <gi...@git.apache.org>.
Github user jcrist commented on a diff in the pull request:

    https://github.com/apache/orc/pull/193#discussion_r154717285
  
    --- Diff: CMakeLists.txt ---
    @@ -105,6 +105,11 @@ INCLUDE(ThirdpartyToolchain)
     set (EXAMPLE_DIRECTORY ${CMAKE_SOURCE_DIR}/examples)
     
     add_subdirectory(c++)
    +
    +install(
    +  FILES LICENSE NOTICE
    +  DESTINATION .)
    --- End diff --
    
    If I do `make install`, these just get placed in my root directory. Other apache projects (e.g. arrow, parquet-cpp) don't install the `LICENSE` or `NOTICE` (the header files all have the license information), so why is this needed? This is why I removed these in #192, sorry if there was a good reason for installing these.


---

[GitHub] orc issue #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarball

Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on the issue:

    https://github.com/apache/orc/pull/193
  
    Legally, the binary artifacts from the Apache projects are supposed to include them. For tarballs it is traditionally at the root of the project. Especially for RPM and DEB packaging, you wouldn't want them at the root. 
    
    How about we move it to doc/orc/{NOTICE,LICENSE}?


---

[GitHub] orc issue #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarball

Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on the issue:

    https://github.com/apache/orc/pull/193
  
    Yeah, I agree. I'd rather leave things consistent and share/doc/orc makes sense.
    
    Arguably we should also add the html for the site there too.


---

[GitHub] orc pull request #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarbal...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/orc/pull/193


---

[GitHub] orc issue #193: ORC-267: INSTALL NOTICES and LICENSE to binary tarball

Posted by jcrist <gi...@git.apache.org>.
Github user jcrist commented on the issue:

    https://github.com/apache/orc/pull/193
  
    From http://www.apache.org/dev/apply-license.html#new it looks like they recommend storing `LICENSE` and `NOTICE` in the root of the tar archive of a distribution (as you've done here). Unfortunately cpack doesn't make it easy to distinguish between files in `make install` and `make package`.
    
    Neither `parquet-cpp` nor `arrow` include `LICENSE` or `NOTICE` on `make install` or `make package` (`parquet-cpp` only, arrow doesn't use `cpack`). For at least the python wrappers for these libraries, the actual release artifacts are generated via an external tool that adds in the LICENSE stuff.
    
    This separation of build/package makes sense to me, but may not for this project (not sure how y'all handle releases). Perhaps the LICENSE/NOTICE could be added to the `tar.gz` file after `make package` manually via another make target? Adding to `doc/orc/{NOTICE,LICENSE}` might be equally fine? I'm not really sure what's best here, either is fine with me.


---