Skip to content
This repository has been archived by the owner on Oct 26, 2021. It is now read-only.

Include LICENSE (or similar) files from dependencies #18

Closed
wants to merge 1 commit into from

Conversation

BetsyMcPhail
Copy link
Contributor

@BetsyMcPhail BetsyMcPhail commented Sep 23, 2021

As dependencies are built, their relevant files are gathered in a licenses
directory. The contents of this directory is then copied to
/wheel/pydrake/doc and is included as part of the wheel.

Resolves #15632

As dependencies are built, their relevant files are gathered in a licenses
directory. The contents of this directory is then copied to
/wheel/pydrake/doc and is included as part of the wheel.
bzip2 CopyLicense
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/src/bzip2/LICENSE ${LICENSE_DIR}/bzip2/LICENSE
DEPENDEES install
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the whitespace errors; thanks!

@@ -7,3 +7,9 @@ ExternalProject_Add(bzip2
BUILD_COMMAND make "CFLAGS=-fPIC -O2"
INSTALL_COMMAND make PREFIX=${CMAKE_INSTALL_PREFIX} install
)

ExternalProject_Add_Step(
Copy link
Contributor

Choose a reason for hiding this comment

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

While the general approach looks okay, I think this is repetitive enough that encapsulating it in a function would probably be useful.

Maybe something like:

extract_license(bzip2
  LICENSE
)

(We can take a list of files, the destination can be derived, and the base directory of the source can be derived from the external-project name.)

Comment on lines +14 to +15
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/src/eigen/COPYING.GPL ${LICENSE_DIR}/eigen/COPYING.GPL
COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_BINARY_DIR}/src/eigen/COPYING.LGPL ${LICENSE_DIR}/eigen/COPYING.LGPL
Copy link
Contributor

Choose a reason for hiding this comment

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

Hrm, we're not using any of these bits, are we?

@@ -7,3 +7,10 @@ ExternalProject_Add(tinyxml2
CMAKE_ARGS
${COMMON_CMAKE_ARGS}
)

# TODO: tinyxml2's LICENSE file is introduced in 7.1.0. We are using 7.0.1.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, maybe we should update? I think we're on 7.0.1 because I somewhat arbitrarily picked whatever version is current on Fedora 33 (which in this case is 7.0.1).

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, IIUC there is no need to distribute a license or notice for zlib-licensed software.

Specifically, the license is:

TinyXML-2 is released under the zlib license:

This software is provided 'as-is', without any express or implied warranty. In no event will the authors be held liable for any damages arising from the use of this software.

Permission is granted to anyone to use this software for any purpose, including commercial applications, and to alter it and redistribute it freely, subject to the following restrictions:

  1. The origin of this software must not be misrepresented; you must not claim that you wrote the original software. If you use this software in a product, an acknowledgment in the product documentation would be appreciated but is not required.
  2. Altered source versions must be plainly marked as such, and must not be misrepresented as being the original software.
  3. This notice may not be removed or altered from any source distribution.

I don't see anything in there requiring us to redistribute the license notice. We're probably fine just adding a note to this effect.

@mwoehlke-kitware
Copy link
Contributor

Also, FYI, we're using four-space indent, not two-space...

@mwoehlke-kitware
Copy link
Contributor

Here's an attempt (untested) at making a helper function for this:

function(extract_license PROJECT)
    set(command "")
    foreach(file IN LISTS ARGN)
        list(APPEND command
            ${CMAKE_COMMAND} -E copy
            ${CMAKE_BINARY_DIR}/src/${PROJECT}/${file}
            ${LICENSE_DIR}/${PROJECT}/${file}
        )
    endforeach()
    ExternalProject_Add_Step(
        ${PROJECT} CopyLicense
        ${command}
        DEPENDEES install
    )
endfunction()

Comment on lines +7 to +8

# TODO: LICENSE
Copy link
Contributor

Choose a reason for hiding this comment

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

Given we aren't actually building anything at present, this seems redundant?

@BetsyMcPhail BetsyMcPhail deleted the update-licenses branch September 28, 2021 19:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pip install: License notices
3 participants