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

Brushes hosted in mypaint-brushes and libmypaint as a normal dependency #538

Closed
wants to merge 6 commits into from

Conversation

@Jehan
Copy link
Member

@Jehan Jehan commented Dec 26, 2015

I rebased my change from pull request #302. Unfortunately this closed #302 and I can't reopen it, so I create this new pull request.

Apart from rebasing, I changed the implementation by checking mypaint-data through pkg-config, and not using a git submodule anymore.
Indeed using a git submodule, we would reinstall the mypaint-data package specially for mypaint, which goes against the very idea of having it as a separate shared package usable by other apps.

I also updated mypaint-data (https://github.com/Jehan/mypaint-data), which is now set to use the latest brush data from current mypaint repo, and it will now be installed in a proper separate directory $datadir/mypaint-data/.
I have tested against my mypaint repo installed in a separate prefix and it works well. I have also already prepare the code for GIMP which is ready to be committed if and when you will accept the pull request. The discovery through pkg-config in GIMP works well as well.

@Jehan
Copy link
Member Author

@Jehan Jehan commented Jan 30, 2016

@achadwick Hi. I was wondering how we were doing on this discussion. Last we spoke on pull request #302, you were waiting for the 1.2 release. Now that it is done, we would really appreciate the existence of a separate mypaint-data package, shared between all programs using libmypaint. :-)

@achadwick
Copy link
Member

@achadwick achadwick commented Feb 1, 2016

Just taking a look at this as requested. I do not want to merge this as it stands.

  • I want to avoid using the "mypaint-data" name. There is an existing Debian/Ubuntu package with that name. I'm going to suggest "mypaint-brushes" instead, if that's OK.
  • There have been updates to the brushes since April 2015. That should be captured.
  • I notice that the brushes repo doesn't contain the complete history of the MyPaint brushes. Making an ice core like that from the existing mypaint repo'berg might be some interesting surgery but I think it can be done...
  • The COPYING files contains the wrong license: GPLv2, whereas the brushes are PD/CC0 for the "program code" part, and CC-something-reasonable for the thumbnails.
  • The LICENSE file is out of date (its replacement in mypaint describes licenses much better)
  • I would prefer it not to contain a brushes subdirectory; instead, just present the brush collection folders at the top level. That way it can be submodule-ified as "brushes" initially (much as we present libmypaint as "brushlib" right now)
  • The submodule addition should use a relative path.

This PR is no longer valid, sadly.

@achadwick achadwick closed this Feb 1, 2016
@achadwick
Copy link
Member

@achadwick achadwick commented Feb 1, 2016

Sorry, closed too soon. I can work on this if you prefer though.

@achadwick achadwick reopened this Feb 1, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented Feb 1, 2016

I want to avoid using the "mypaint-data" name.

On a previous comment, pr #302, you actually said the opposite. Quoting you from this other pr:

Name is great, slots in nicely with the debian package's mypaint-data-extras :D

This said, I don't really care. That was only to tease you. :p
I believe that all we need for now on GIMP are the brushes. I only proposed "mypaint-data" in case more needed to be extracted out mypaint repository in some potential future. But if you think that it won't happen (or if it does, you prefer to just create new packages, like "mypaint-palettes", or whatever), I'm totally fine with renaming to mypaint-brushes. :-)

There have been updates to the brushes since April 2015. That should be captured.

As I say on the first comment, I have rebased late december to account for changes. I can check again and update the repository, no problem.

I notice that the brushes repo doesn't contain the complete history of the MyPaint brushes. Making an ice core like that from the existing mypaint repo'berg might be some interesting surgery but I think it can be done...

It obviously can, as everything in git. I went for the easy way but I can try and reproduce the brushes history if this is a requirement.

The COPYING files contains the wrong license: GPLv2, whereas the brushes are PD/CC0 for the "program code" part, and CC-something-reasonable for the thumbnails.

Ok. As I was saying on #302, I was actually unsure on the licensing of brushes.
Can you tell me what is the "something reasonable" part? :-) By-SA maybe?

I would prefer it not to contain a brushes subdirectory; instead, just present the brush collection folders at the top level.

Ok. I was doing it this way for the case where it was called mypaint-data, hence could have more than only the brushes in some possible future. Now if we call it mypaint-brushes, this is of course not a problem.

This said, I must say that I don't agree with such a file organization, even if the package was called mypaint-brushes because it makes the repository less organized in my opinion. Same as when you have a program, you usually have your source files under src/ or your-program-name/ or some other naming but not under the root.
This said, I see you have source files under the root for mypaint and libmypaint, so I guess it's only a matter of preferences. :-)

The second reason is that it will make it even more complicated to recreate the git history that you asked before! If I init the repository with a single commit, no problem. But if I also have to save the history while in same time modify every commit to pretend it happened in the root instead of in brushes/, that becomes very complicated.
Unless I just add a commit in the end to move all files from brushes/ to the root (but then the history is a lot less useful to trace brush history).

That way it can be submodule-ified as "brushes" initially (much as we present libmypaint as "brushlib" right now)

As I said in the initial comment of this pull request, I propose you don't do submodules anymore. This is basically like embedding the sub-package in yours, then mypaint is installing brushes, and libmypaint, etc. So you would be making your own installation of mypaint-brushes in the same time as mypaint. What is the point of making it a separate package then? :-)

The mypaint-brushes package should be a normal dependency, like any other: you test for its existence and if it is not installed, you ask the builder to install it first. No submodule, just a dependency. This is what I do in my current pull request. See the proposed commit b714038 which implements this.

Same should go for libmypaint by the way, if you really want it to make a proper separation with MyPaint.

@Jehan Jehan force-pushed the Jehan:brushes-data branch from ec55c56 to 829b73d Feb 2, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented Feb 2, 2016

@achadwick Please have a look at https://github.com/Jehan/mypaint-brushes

This is a new repository named mypaint-brushes. It is up-to-date with current mypaint (just make a diff -r to compare and you'll see). It contains the full log of what happened to brushes, and only that.

Brushes are still under a brushes/ directory, mainly for the "saving history" reason I told you about. There are 152 commits in the history. Rewriting history to pretend the work was done on the root since the start would likely mean to stop at every commit, and that would just be crazy. The easy way is to make a git mv commit now, but that removes some of the whole point of saving the repo history. If really you wish to have all brushes on the root, I can do this though.

COPYING file now contains the plain text from the CC0 license. Since we have not decided yet what should be CC-something-reasonable for the thumbnails, I wrote the following text in the LICENSE file for now:

The "program code" part (myb file) of all brushes are licensed by their
respective authors and released as CC0/public domain.
Thumbnail license is still to be determined.

Finally I also updated my mypaint branch to take the new package name into account. I tested, it just works. I still don't use a submodule. As I explained in my previous message, it should be a dependency as any other, in my opinion. :-)

@Jehan
Copy link
Member Author

@Jehan Jehan commented Feb 2, 2016

Oh I didn't realize you had new Licensing files. I updated mypaint-brushes where I did the same.

@odysseywestra
Copy link
Member

@odysseywestra odysseywestra commented Mar 29, 2016

@Jehan Any progress on this pull request?

@Jehan
Copy link
Member Author

@Jehan Jehan commented Mar 29, 2016

@odysseywestra Well I did everything which was asked by @achadwick and had clean commits as well as a mypaint-brushes repository with all history. Now I see there are conflicts again, but I am not going to fix these until I am told that when I do, there will be a merge soon. Each time I work on these, it takes me hours to check that it all works fine.

So just tell me when you want this merged, and I will work then. :-)
We are really looking forward to this for the GIMP integration.

@odysseywestra
Copy link
Member

@odysseywestra odysseywestra commented Mar 30, 2016

@Jehan I've open a thread on our Community Forums about our use of submodules and what we can do to wean ourselves off of using them. Your more than welcome to input your thoughts about this matter if you want.

http://community.mypaint.org/t/using-an-alternative-to-submodules/267

@Jehan
Copy link
Member Author

@Jehan Jehan commented Mar 30, 2016

@odysseywestra Thanks. Wrote my thoughts then. ;-)

@achadwick achadwick added this to the MyPaint v1.3.0 milestone Apr 17, 2016
@achadwick achadwick self-assigned this Apr 17, 2016
@achadwick
Copy link
Member

@achadwick achadwick commented Apr 17, 2016

Now would be a good time to go ahead with this. Sorry to keep you waiting

@Jehan - you now have RW access to libmypaint for the autotools stuff, and I'll be happy to integrate your brushes repo before 1.3.0 (and give you the same rights over it).

(This may break stuff, so I'll bump back our 1.3.0 milestone. The MyPaint 1.2.x Windows and GTK 3.20 stuff is still breaking, so I'll be working on that in the interim...)

@Jehan Jehan force-pushed the Jehan:brushes-data branch from 829b73d to e83cf2f May 17, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented May 17, 2016

@achadwick I have rebased my mypaint repository to fix any merge conflict.

Also since I added pkg-config support for the mypaint-brushes detection, this can be used for libmypaint too! :-)

@Jehan Jehan changed the title Brushes hosted in mypaint-data Brushes hosted in mypaint-brushes and libmypaint as a normal dependency May 17, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented May 17, 2016

@achadwick I have just pushed a new commit to my branch. Now mypaint does not use any submodule anymore there and detects libmypaint (and its build/link flags) with pkg-config! :-)

I tested, mypaint works well. I really think now there is nothing to retain you from merging this all. And in the same time, get the mypaint-brushes repository into the mypaint account and take ownership of it!
This would really make my day. :-)

@Jehan Jehan mentioned this pull request May 17, 2016
4 of 4 tasks complete
@achadwick
Copy link
Member

@achadwick achadwick commented May 17, 2016

Eek, I'm doing something very like 66d68e1 in a private branch right now, with a ton of other MyPaint-specific things besides. If you don't mind, I'm just going to pick the bits of it I haven't done yet. Would you be OK with removing that commit from this PR once its workalike is in master?

Sorry to mess you around 🙇

@Jehan
Copy link
Member Author

@Jehan Jehan commented May 17, 2016

Ok no prob. I'll remove later when it's in master. Just keep me updated. :-)

@achadwick
Copy link
Member

@achadwick achadwick commented May 22, 2016

@Jehan
Ready to go ahead with this, as of 6c4da2c 😄

@Jehan Jehan force-pushed the Jehan:brushes-data branch from 66d68e1 to e83cf2f May 22, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented May 22, 2016

I think you meant 679cc2b, no? :-)
Anyway last commit removed, as planned.

@Jehan Jehan force-pushed the Jehan:brushes-data branch from e83cf2f to 7fc679b May 22, 2016
@Jehan
Copy link
Member Author

@Jehan Jehan commented Sep 10, 2017

Ok. All updated!

mypaint-brushes

The repository mypaint-brushes is now up-to-date with what is currently on mypaint repo (a diff -r mypaint/brushes/ mypaint-brushes/brushes/ will show it) while still having the whole history of git related to brushes: https://github.com/Jehan/mypaint-brushes

I also updated the pkg-config file so that this data package is called mypaint-brushes-2.0. It will get installed under $prefix/share/mypaint-data/2.0/brushes/. As you see, the installation path is also versioned, which will allow to have several official sets of MyPaint brushes installed without conflicting with each other (cf. the recent discussion on some brushes with new settings).

For info, to answer on issue #842, I don't intend on maintaining the repository, forking or whatever and never did. The goal has always been to give ownership to this repo to the MyPaint project (I just did it on my name because I don't have rights to create a new project under the mypaint organization of course). So just take this repo! :-)

mypaint

I also rebased mypaint itself with master so that it does not conflict anymore. I see that you now use a setup.py so I updated this. It builds, runs well and find the brushes without any issue. I tested. :-)

@Jehan Jehan mentioned this pull request Sep 10, 2017
0 of 6 tasks complete
@Jehan
Copy link
Member Author

@Jehan Jehan commented Sep 10, 2017

P.S.: I see the CI checks failed, but that's normal since they can't find the new repository mypaint-brushes which is now a dependency of mypaint. The CI will have to be updated once this is merged. :-)

@Jehan
Copy link
Member Author

@Jehan Jehan commented Sep 25, 2017

Any news? ;-(

@briend
Copy link
Member

@briend briend commented Sep 25, 2017

This sounds really cool. So is the idea to create a 1.3 branch to run in parallel with master... and perhaps one day there will be 1.3, 2.0, etc?

@odysseywestra
Copy link
Member

@odysseywestra odysseywestra commented Sep 25, 2017

@achadwick could you please address this? We don't need to merge in the changes to MyPaint master yet, but we do need to have a separate repo for the brushes. This will open up a way for other projects to use the brushes without having to download MyPaint master resources.

@Jehan
Copy link
Member Author

@Jehan Jehan commented Oct 28, 2017

Damn, I don't know how I always miss the notifications even though I am subscribed to this report. I just saw the messages. Anyway…

@briend Yeah basically the brushes would be their own project. IMO, they don't necessarily need to have the same release numbers as libmypaint. On the other hand, having at least a common base in the versionning scheme would help for dependency (for instance, mypaint-brushes v1.x would work for libmypaint v1.x; and so on for v2.x…). Anyway the versionning scheme is mostly up to you, if you want it to follow libmypaint versionning or not. As long as there is some logics which prevent projects from having weird behaviors, that's all good.

In any case, it would be good to have very soon a first release of mypaint-brushes which work for libmypaint 1.x, so that we can add the dependency in GIMP. Then master would be for libmypaint v2.x and you won't have to be afraid to "break" the brushes anymore, and you can also freely add or remove brush options, and so on. Just freeze a release of brushes for libmypaint 1. :-)

@Jehan
Copy link
Member Author

@Jehan Jehan commented Oct 28, 2017

So I have pushed a new branch v1.3.x (using the current branch naming I see on mypaint/libmypaint) on https://github.com/Jehan/mypaint-brushes. This branch could be used for releases targetting libmypaint v1, whereas master is currently following brushes of libmypaint master (i.e. future v2).

This has been branched out just before the brush tweaks (viewzoom, speed1/2, etc.).

@Jehan
Copy link
Member Author

@Jehan Jehan commented Nov 16, 2017

Any chance this could be reviewed? ;-(

@Jehan
Copy link
Member Author

@Jehan Jehan commented Dec 31, 2017

Hello everyone! This has been 2 years now.
We have discussed with Mitch (GIMP maintainer). We really need mypaint-brushes as a separate repository for GIMP, and we really need it versionned (because of potential breakage). We are pushing to have GIMP 2.10 released soon. Hopefully in the next few months. And we need to prepare, which also mean we need to have Linux distributions out there starting to package mypaint-brushes are a separate package which will be a GIMP dependency, etc.

So I propose that we temporarily take the lead on the mypaint-brushes by requiring this package in GIMP (through the pkg-config). Note though that I don't want to actually take the lead! I never intended (and really don't want) to maintain this repository. My goal has always been for MyPaint project to take this repo and make the rules (hence report sitting here for 2 years).
This is still true. So please, as soon as you review this and decide on taking on the repo, just do it. :-)

@Jehan
Copy link
Member Author

@Jehan Jehan commented Jan 1, 2018

This is now done: https://git.gnome.org/browse/gimp/commit/?id=f8e1d3b9c83cc3d55c8f1ac8a1c02f36b278a4e2

We really hope that MyPaint project will take care of this repository soon to live right next to MyPaint and libmypaint.

@Jehan
Copy link
Member Author

@Jehan Jehan commented Feb 8, 2018

For info, we have had a few requests to add new brushes (https://github.com/Jehan/mypaint-brushes/issues) but I don't want to merge them because I don't want to step on your prerogatives. As far as I am concerned, MyPaint project ultimately owns this repository and I hope you will soon, and therefore that you can decide whether to accept these new brush sets or not.

A GIMP contributor also asked if we could have exclusive MyPaint brushes packed in GIMP, but I'd really prefer for all new brushes to be shared amongst all projects using libmypaint so that it profits to everyone. No need for exclusivity when we can share. Yet for the same reason as above, I won't accept his brushes right now.

Essentially mypaint-brushes contents is freezed right now (only potential bugs in the build system can be done). I'm a bit sad.
How can I have the situation go forward? I really want to help, if needed, if it can unblock the situation! :-)

@odysseywestra
Copy link
Member

@odysseywestra odysseywestra commented Feb 8, 2018

We need @achadwick to address this since he one of the only people that can do this.

@jbicha
Copy link

@jbicha jbicha commented Apr 1, 2018

@achadwick Ping!

Distros really don't like to have avoidable duplication in their archives. Now that gimp 2.10 RC depends on mypaint-brushes, it would be helpful if mypaint would depend on it too instead of bundling them.

@Jehan Jehan force-pushed the Jehan:brushes-data branch from 3d60ba9 to dc15ed5 Apr 1, 2018
@Jehan
Copy link
Member Author

@Jehan Jehan commented Apr 2, 2018

@achadwick I have just rebased the branch and fixed what needed to be.
Current master now works well with mypaint-brushes master using pkg-config to find the data.

Both libmypaint and mypaint-brushes are now versionned, which means that they can be shared by any program and they can be installed side by side without conflict (for instance GIMP 2.10 will use libmypaint + mypaint-brushes v1 whereas I assume next MyPaint release will use v2).

I would really appreciate if this could be reviewed and finally merged. I have rebased this branch so many times over the years that I must have already spent equivalency of days (maybe even weeks) working on it. I would be grateful if this could be finally completed.
Thanks!

@briend
Copy link
Member

@briend briend commented Apr 2, 2018

I wonder if we could tackle the CI issues on this PR so that all the checks pass? That might help @achadwick a bit. I think we'd just need to submit a new mypaint-brushes package to MSYS2 here:
https://github.com/Alexpux/MINGW-packages
Tweak the msys2-build.sh and maybe a few dozen other things :-D

@Jehan
Copy link
Member Author

@Jehan Jehan commented Apr 2, 2018

@briend Yeah the various CI check just needs mypaint-brushes to be installed first so that MyPaint can find it as it would find any dependency. There is no mypaint-brushes v2 release yet though. Since there is no libmypaint 2 release either, nor any MyPaint release depending on these, I didn't feel the need to do a release of the brushes (there is only a release for the v1 brushes, before all the changes with new or changed properties, etc.). A package for mypaint-brushes would have to track the master branch (same as I guess it is tracking also master branch of libmypaint).

Anyway if anyone could do such packages, hence fixing the CI, I would be extremely grateful! I don't know much about these systems and have already so much to do.

@briend
Copy link
Member

@briend briend commented Nov 28, 2018

@Jehan I've forked this branch and rebased. Seems to work well. I need to do a bit more testing and make sure python3 works. I also would like to build an MSYS2 package. I created a release for mypaint-brushes 2.0 so that should help. I don't know if MSYS2 will accept any more git tracking packages. They rejected one I submitted for a python library and asked for a release

@briend
Copy link
Member

@briend briend commented Dec 7, 2018

merged! let's see what's broken. MUCH thanks @Jehan!

@briend briend closed this Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

5 participants
You can’t perform that action at this time.