Skip to content
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.

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

Build is forcing all contributions to be copyrighted to Codenvy #3281

Closed
gorkem opened this issue Dec 5, 2016 · 27 comments
Closed

Build is forcing all contributions to be copyrighted to Codenvy #3281

gorkem opened this issue Dec 5, 2016 · 27 comments
Labels
kind/enhancement A feature request - must adhere to the feature request template. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@gorkem
Copy link
Contributor

gorkem commented Dec 5, 2016

maven license check plugin is failing the build if the copyright owner for a file is different from Codenvy. I do not think we should be adding such restrictions on an open source project.

@tolusha
Copy link
Contributor

tolusha commented Dec 6, 2016

@gorkem
Hello. You can configure exclusion in the pom.xml of your module.

            <plugin>
                <groupId>com.mycila</groupId>
                <artifactId>license-maven-plugin</artifactId>
                <configuration>
                    <excludes>
                        ...
                    </excludes>
                </configuration>
            </plugin>

@ibuziuk
Copy link
Member

ibuziuk commented Dec 6, 2016

@tolusha but it means that all companies will be facing this issue while contributing to the project and will need to configure excludes for every single change. Why there should be a check of copyright owner at all ? AFAIK, according to the Eclipse rules the one who creates a file put the copyright owner in the license header. Other contributors that make changes in the file should add info in the "Contributors" section.

@skabashnyuk
Copy link
Contributor

@ibuziuk There is an Eclipse foundation requirement to have license on all files. We did that with mycila license-maven-plugin. If you know how better it can be configured please introduce new PR. I have only one rule - it have to be done automatically with maven. Without automation this requirement is hard to achieve.

@ibuziuk
Copy link
Member

ibuziuk commented Dec 6, 2016

@skabashnyuk fair enough ;-) I will work on this issue

@gorkem
Copy link
Contributor Author

gorkem commented Dec 6, 2016

@ibuziuk take a look at http://www.mojohaus.org/license-maven-plugin/ it looks like it has better configuration options.

@bmicklea bmicklea added the kind/enhancement A feature request - must adhere to the feature request template. label Dec 6, 2016
@skabashnyuk
Copy link
Contributor

@ibuziuk
Copy link
Member

ibuziuk commented Dec 6, 2016

@skabashnyuk yup, I have seen this upstream issue. IMO it is a bit different from what we are trying to achieve - {INITIAL COPYRIGHT OWNER} should be any company

@ibuziuk
Copy link
Member

ibuziuk commented Dec 8, 2016

After some investigation I came to the conclusion that what we are trying to achieve is not possible to configure via mycila plugin. It is doable with mojohaus one though. BUT... there are a few things that should be taken in consideration:

  1. The mojohaus plugin is more flexible in configuration due to it's Header model. Basically, license header consists of several parts which are separated by special characters. As a result, the valid license header generated by default would look the following way:
/*-
 * #%L
 * %%
 * Copyright (C) 2012 - 2016 Codenvy, S.A.
 * %%
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 * #L%
 */

The good thing here is that plugin validates only "License" part - "Copyright" part can be changed safely. However, because of the plugin's Header Model ( #%L - start of the header / #L% - end of the header) copyrights on all files in the Che project would be treated as absent and must be regenerated.

  1. Hideous special characters in the license header. Special characters are used for distinguishing one part of the header from another. It is possible to override default ones but its not possible to omit them.

  2. Plugin support limited number of supported file extensions and has no custom mapping (extension -> comment type) mechanism. For the Che project at least Dockerfile, Stylus and TypeScript files are affected.
    Supported extensions - http://www.mojohaus.org/license-maven-plugin/examples/example-comment-style-list.html
    Related issue - Question: how to define <extraExtensions> for files without file extension mojohaus/license-maven-plugin#38

The fact that changing copyrights on all existing files is required makes using this plugin of little avail for our case.

@skabashnyuk
Copy link
Contributor

@ibuziuk can you show me java file before and after?
What about other types of files, xml, sql, svg etc?

@ibuziuk
Copy link
Member

ibuziuk commented Dec 8, 2016

@skabashnyuk I have created a test project while playing with this mojohaus plugin - https://github.com/ibuziuk/license-demo

Here is how the java file looks like after executing license:update-file-header goal - https://github.com/ibuziuk/license-demo/blob/master/src/main/java/com/ibuziuk/App.java ("before" state is the same file without license header)

Other files look the following way after executing this goal - https://github.com/ibuziuk/license-demo/tree/master/src/resources

As you can see, license header was not added to some files (Dockerfile, ts, svg) because plugin simply does not support those extensions. The list of supported extensions can be retrieved by executing license:comment-style-list

 * apt        : header transformer with apt comment style            , extensions : [apt]
 * ftl        : header transformer with free marker comment style    , extensions : [ftl]
 * html       : header transformer with html comment style           , extensions : [xhtml, html, htm]
 * java       : header transformer with java comment style           , extensions : [java, groovy, css, cs, as, aj, c, h, cpp, js, json]
 * jsp        : header transformer with jsp comment style            , extensions : [jsp, jspx]
 * mysql      : header transformer with mysql comment style          , extensions : [mysql]
 * php        : header transformer with php comment style            , extensions : [php]
 * properties : header transformer with properties file comment style, extensions : [properties, sh, py, rb, pl, pm]
 * rst        : header transformer with rst comment style            , extensions : [rst]
 * sql        : header transformer with sql comment style            , extensions : [sql]
 * xml        : header transformer with xml comment style            , extensions : [pom, xml, mxlm, dtd, fml, xsl, jaxx, kml, gsp, tml]
 

Assuming that the plugin does not support custom mapping (extension -> comment type) mechanism, Unsupported files would have to be updated manually.

@skabashnyuk
Copy link
Contributor

That's is not quite what I expect.
Can you confirm that java,xml etc header are not change?
Because AFAIK we made it as it requested in docs https://eclipse.org/legal/copyrightandlicensenotice.php
and if it change for a single characters it have to be agreed with legal team.

Second: in addition to *.java, *.xml license plugin now checks *.svg, *.ts, *.go - it have to be automated too. Project is too large to make some checks manually.

@ibuziuk
Copy link
Member

ibuziuk commented Dec 9, 2016

@skabashnyuk sorry, do not really understand what exactly should be confirmed :(
for java, xml headers are added and look just like in the demo project - https://github.com/ibuziuk/license-demo. However, the style is different from the default eclipse foundation copyright (mainly due to special characters)
The license part is identical though:

  All rights reserved. This program and the accompanying materials
  are made available under the terms of the Eclipse Public License v1.0
  which accompanies this distribution, and is available at
  http://www.eclipse.org/legal/epl-v10.html

svg, ts, go are not supported extensions. It is easy to contribute to the upstream project though (I have contributed ts support already). My main concern about using this plugin, apart from ugly special characters, is the fact that headers on all files in the Che project would be treated as absent and must be regenerated after license plugin switch.

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Dec 9, 2016

I may looks like "bookworm" but license should be 100% identical to current as described here https://eclipse.org/legal/copyrightandlicensenotice.php
if it's not I can't guarantee that this change is acceptable for Eclipse Legal department. It have to be agreed with them first. Can you show for instance output of git diff or diff I want to see the exact difference.

@ibuziuk
Copy link
Member

ibuziuk commented Dec 9, 2016

but license should be 100% identical to current as described here. if it's not I can't guarantee that this change is acceptable for Eclipse Legal department

@skabashnyuk in this case mojohause plugin does not suits. Here is the diff:

-/*-
- * #%L
- * %%
- * Copyright (C) 2012 - 2016 Codenvy, S.A.
- * %%
+/*******************************************************************************
+ * Copyright (c) 2012-2016 Codenvy, S.A.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
  * http://www.eclipse.org/legal/epl-v10.html
- * #L%
- */
+ *
+ * Contributors:
+ *   Codenvy, S.A. - initial API and implementation
+ *******************************************************************************/

The other solution would be contributing to mycila plugin (support of wildcards / range). Need to investigate how difficult would it be

@skabashnyuk
Copy link
Contributor

You may ask license@eclipse.org directly. Not sure that it will be faster when contribute to mycila plugin.

@ibuziuk
Copy link
Member

ibuziuk commented Dec 9, 2016

@skabashnyuk apart from the fact that headers are not identical, are you ok with switching from mycila plugin to mojohaus one (taking in consideration all drawbacks) ? Basically, some eclipse projects use this plugin for handling headers[1], so I do not think this style is prohibited by foundation (IMO only license itself matters, not the style). I would not mind asking Eclipse IP directly about the plugin, but if we are not going to use it I see no sense in doing this. So, it would be great to hear your opinion on this matter first

[1]http://git.eclipse.org/c/emf/org.eclipse.emf.eson.git/tree/plugins/org.eclipse.emf.eson.ui/src/org/eclipse/emf/eson/ui/EFactoryLog.java

@ibuziuk
Copy link
Member

ibuziuk commented Dec 9, 2016

BTW, I have provided patches for svg & go support which have been already merged

@ibuziuk
Copy link
Member

ibuziuk commented Dec 15, 2016

@skabashnyuk apart from the fact that the mojohaus license formatting might not be accepted by Eclipse Legal department, are you ok with using it ? Just trying to figure out what our strategy is:

  1. Change the plugin (need to figure out if header would be accepted by foundation)
  2. Continue using mycila (continue using excludes for non-Codenvy contributors and contibute wildcards / range support upstream).
    Could you please give an opinion on this topic ?

@skabashnyuk
Copy link
Contributor

Name of the plugin is not important. It can be a fork of mycila plugin or any other plugin what can check license. It's easy to approve build tools in Eclipse organization.
I do care about

  1. It have to automatically check the license of known files: java, xml, svg, sql, ts, go (may be some others not sure)
  2. It should to have format goal.
  3. Format of License should be accepted by Eclipse legal team.
  4. It should be a way to configure multiple contributors like this https://github.com/codenvy/artik-ide/blob/master/pom.xml#L4-L12

Ideally but not mandatory it should be possible to configure Codenvy proprietary license like this
https://github.com/codenvy/codenvy/blob/master/pom.xml#L2-L17

@ibuziuk
Copy link
Member

ibuziuk commented Dec 20, 2016

@skabashnyuk I wrote to license@eclipse.org and got clarification from Mike Milinkovich that new license format is valid for Eclipse Foundation:

/*
* #%L
* org.package.name
* %%
* Copyright (C) 2009 - 2014 Company
* %%
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
* #L%
*/

the proposed copyright and license header is fine. It meets the Eclipse Foundation's requirements

For now not all formats are supported by the plugin, so we can not use it in the current shape. Could you please specify what files apart from java, xml, svg, sql, ts, go should be supported? I am going to contribute support of all required extensions upstream.

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Dec 21, 2016

and got clarification from Mike Milinkovich that new license format is valid for Eclipse Foundation:

Can you give me a link to this conversation?

Could you please specify what files apart from java, xml, svg, sql, ts, go

that's is ok to start.

To move forward - can you make a pr that replace existed plugin with one you want to activate. So I can try and take a look a situation on my computer?

@ibuziuk
Copy link
Member

ibuziuk commented Dec 21, 2016

@skabashnyuk I have forwarded email to you. For now there is no support of svg, ts, go - need to wait for the next version of the plugin. If more extensions are required it would be great to know in advance. There is no custom mapping (extension -> comment type) mechanism and every new extension need to be contributed upstream. I can work on the initial PR after NY

amisevsk added a commit to amisevsk/che that referenced this issue Dec 24, 2016
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk added a commit to amisevsk/che that referenced this issue Dec 30, 2016
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@skabashnyuk
Copy link
Contributor

@ibuziuk do you have an example of license plugin configuration that we can test?

@ibuziuk
Copy link
Member

ibuziuk commented Jan 4, 2017

@skabashnyuk no, have to work on other stuff. I will probably take this item to the next sprint

amisevsk added a commit to amisevsk/che that referenced this issue Jan 5, 2017
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk added a commit to amisevsk/che that referenced this issue Jan 5, 2017
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
amisevsk added a commit to amisevsk/che that referenced this issue Jan 5, 2017
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
JPinkney pushed a commit to JPinkney/che that referenced this issue Aug 17, 2017
…heck

The current license checking maven plugin does not allow
for multiple copyright owners on source files. This commit
adds files modified for ServerEvaluationStrategy to an excludes
list so that builds can continue normally.

This commit should be undone once issue eclipse-che#3281 is resolved.

Signed-off-by: Angel Misevski <amisevsk@redhat.com>
@skabashnyuk
Copy link
Contributor

@gorkem do you still interesting on this issue?

@gorkem
Copy link
Contributor Author

gorkem commented Jan 18, 2018

@skabashnyuk I guess the issue is still there. It is just the copyrights are aligned now :)

@che-bot
Copy link
Contributor

che-bot commented Sep 4, 2019

Issues go stale after 180 days of inactivity.
lifecycle/stale issues rot after an additional 7 days of inactivity and eventually close.
Mark the issue as fresh with /remove-lifecycle stale.
If this issue is safe to close now please do so.

Add lifecycle/frozen label to not flag this issue as being stale.

@che-bot che-bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 4, 2019
@che-bot che-bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 6, 2019
@benoitf benoitf added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 6, 2019
@che-bot che-bot closed this as completed Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

7 participants