mpv-player / mpv-build

🔨 Helper scripts to compile mpv on Linux
http://mpv.io
414 stars 108 forks source link

Support more use cases #97

Closed avih closed 5 years ago

avih commented 6 years ago

Currently the mpv-build scripts support the following use cases without making source-code modifications:

There are specifically some things which it doesn't support without source code modifications:

This issue was opened to discuss how to improve the mpv-build scripts to support more use cases.

Preferably, we can keep the discussion strictly at a user-facing level to design a front end which support desirable use cases, without getting into implementation details if possible.

Here's a rough and incomplete outline of what I had in mind:

And that's it. One config files, up to 3 config values per project: mode/repo-url/source-dir.

This will greatly enhance the use cases where mpv-build can be used without source code modifications.

Thoughts?

torque commented 6 years ago

As discussed on IRC, I mentioned the ability to apply patches after update but before build would be a useful functionality.

Some of this would be covered by the ability to change the repo URLs, though being able to apply patches would additionally cover the case of patches coming from multiple different sources.

avih commented 6 years ago

Adding use cases:

avih commented 6 years ago

Some of this would be covered by the ability to change the repo URLs, though being able to apply patches would additionally cover the case of patches coming from multiple different sources.

While trying to balance usefulness while avoiding feature-creep, I think this can be generalized nicely by allowing custom pre/post update/build script paths (up to 4 scripts overall) where the user can perform any actions, including applying patches.

Maybe also send the stage as argument such that the user can use the same script to cover up to four update/build stages according to its argument.

xantoz commented 6 years ago

I mentioned this in the IRC, but: I think it would be useful to move config stuff (such as what repo:s to use, what branches, maybe what is in the files mpv_options, ffmpeg_options, too) to environment variables. The scripts would expand these variables, falling back to the default value if it isn't defined, e.g: do_clone "mpv" "${MPV_REPO_URL:-https://github.com/mpv-player/mpv.git}"

Then you could just export the variable MPV_REPO_URL in your shell to use an alternate repo (such as your own working copy, on github, or locally). For a complete configuration you could save your setup to a shell file and source it before calling any of the scripts.

As an extra convenience we could support such a shell file (let's call it "config") that is sourced automatically by all scripts if it exists. In this file you could then define these variables in a more permanent manner. As an addition to the regular documentation an example config-file can be provided in the repo (say "config.example"), needing only renaming to become active ("config" is obviously in .gitignore).

Pros:

  1. This is precisely what environment variables were made for. Expanding but with a fallback is trivial
  2. Easy to temporarily change options on the cmd-line (e.g: MPV_BRANCH=my-branch ./rebuild -j4)
  3. Convenient to collect all config options in one file.
  4. No longer any need to have ./use-* scripts to change things.

Cons:

  1. With todays script structure perhaps not obvious when what variable takes effect (like the fact that you would need to ./update before doing ./build to have *_REPO_URL variables take effect (depending on implementation *_BRANCH variables too).

If this doesn't sound too clear, I'm going to take a jab at this so I can show what I mean using code, instead.

avih commented 6 years ago

@xantoz most of what you mentioned I had in mind already, however, recall that this is about supporting more use cases, while most of what you suggest are actually way to configure it. I.e. These are not use cases but rather ways to support some use cases.

So, what are the actual use cases you had in mind? Surely you don't want to use env vars for permanent (for you) repo URLs, Right? Typically you'd prefer to configure it once to your liking, and from then onward it will just behave according to your own configuration.

So, what's the use case for which you consider using env vars useful? switch repos on every build?

kevmitch commented 6 years ago

I think we should avoid dealing with actual patch files. If we really want to accommodate applying local modifications after update, we should handle this by re-basing a local branch on top of updates.

If we add more packages (shaderc anyone?) or decide to keep fribidi (I'm still not convinced this should be here since it seems to have been added on a whim), it might make sense to toggle using system or local versions, but I'm not sure about using system libass or ffmpeg since the existential purpose of these scripts is to avoid that.

xantoz commented 6 years ago

@avih that's why I mentioned saving your variable definitions in a file and sourcing it/having it be sourced automatically. See paragraph 2 and 3 of my previous message.

Env vars is a general and convenient way to support many different use cases.

avih commented 6 years ago

Here's roughly what I had in mind. It's not necessarily final but it demonstrates the framework, its simplicity and flexibility.

The simplest usage would be to just execute ./rebuild once in a while to get an updated mpv with all deps at their latest git master versions, possibly followed by ./install.

Another simple usage could be to execute ./setup release once, and then use occasional ./rebuild to build the latest release version of the deps and the latest release version of mpv.

Another usage could be to configure the build to use an mpv source dir at some custom location which is not touched during updates, work on mpv patches at this dir and occasionally run ./build mpv to build only mpv without any of the deps. Occasionally ./update && ./build could be invoked to update the deps according to the update policy (see below) and then build the deps and mpv.

Here are the details:

Each of the scripts clean/update/build do:
1. Sources <mpv-build-root>/scripts/config.default .
2. Sources <mpv-build-root>/user.conf if exists.
3. Performs the actions as described for each tool.
Global:
- <project-list> is alphanumeric project name arguments (e.g. ffmpfg mpv)
- If <project-list> is empty, the projects list is $config_projects
- <project-list> is processed at the order the names appear without dependency concerns.
- The source dir for each project FOO is $config_FOO_source or <mpv-build-root>/FOO if empty
- The libs output dir is always <mpv-build-root>/build_libs
- The build dir of each project FOO is defined by scripts/FOO-config and scripts/FOO-build

User facing scripts (clean/update/build/rebuild/setup):

Usage: clean [<projects-list>]
1. Sources script `$config_clean_pre clean-pre` if defined and exists.
2. For each FOO at <projects-list>, executes scripts/FOO-clean
3. rm -rf <mpv-build-root>/build_libs
4. Sources script `$config_clean_post clean-post` if defined and exists.

Usage: update [<projects-list>]
1. Sources script `$config_update_pre update-pre` if defined and exists.
For each FOO in <projects-list>:
2.1 If FOO's source dir doesn't exist, clone $config_FOO_git into it.
2.2 Updates FOO's source dir according to $config_FOO_update value:
  - "master":  fetch --all && git checkout origin/master
  - "release": fetch --all && git checkout <latest-release-tag>
  - "@BAR":    fetch --all && git checkout BAR
  - "none":    <does-nothing>
Where <latest-release-tag> is the tag with the highest human-sort value
between the tags which start with $config_FOO_release_prefix (could be empty).
3. Sources script `$config_update_post update-post` if defined and exists.

Usage: build [<projects-list>] [--] [<build-args>]
The separator "--" is not required if <build-args> is empty or starts with "-" (e.g. -j8).
1. Sources script `$config_build_pre build-pre` if defined and exists.
2. For each project FOO, executes `scripts/FOO-config && scripts/FOO-build <build-args>` .
1. Sources script `$config_build_post build-post` if defined and exists.

Usage: rebuild [<projects-list>] [--] [<build-args>]
Execute the following sequence:
1. update [<projects-list>]
2. clean [<projects-list>]
3. build [<projects-list>] [--] [<build-args>]

Usage: setup [--force] BAZ
Copies the file presets/BAZ to <mpv-build-root>/user.conf if it doesn't exist
(or if it exists and --force is used - need to refine how this behaves, for
instance maybe not require --force if user.conf is an identical copy of one of
the files at presets/*).
We will have presets/master and presets/release as part of the repo.
If someone wants to, we can maintain a list of files presets/mpv-x.y.z which
correspond to mpv versions, each with their recommended deps versions.

The default config file at the repo could look like this for instance:

#-----------
config_projects="libass ffmpeg mpv"

config_libass_git="https://github.com/libass/libass.git"
config_libass_update="master"

config_ffmpeg_git="https://github.com/FFmpeg/FFmpeg.git"
config_ffmpeg_update="master"
config_ffmpeg_release_prefix="n"

config_mpv_git="https://github.com/mpv-player/mpv.git"
config_mpv_update="master"
config_mpv_release_prefix="v"
#-----------

And a user config file would have the same format but could modify any value
and/or add values which are left as default (some repo source dirs, possibly
some pre/post scripts, etc).

Adding another supported lib BAZ is a matter of adding the files
scripts/BAZ-{config|build|clean}, and at the default config file adding its git
repo URL and its default update policy (probably master or release).

Using BAZ is a matter of adding it to $config_projects or just using it from the
command line as part of <projects-list>.

Libs which are not built after clean will be searched normally as system libs.

xantoz commented 6 years ago

@avih Skimming through it that seems pretty good. Better/more structured than what I had in mind.

Perhaps add config_<projectname>_configure variables for good measure (replacing the files ffmpeg_options, mpv_options, etc.)?

The ability to make custom presets seems useful (In my solution I imagined you could do something similar by making several config files, then manually sourcing the one you want. But that's not quite as neat and tidy).

LaserEyess commented 6 years ago

I don't know how I feel about the pre/post scripts, but for generality's sake that's probably a good idea and I can see why someone would potentially want that. In fact update-post could be a sane way of applying patches before calling ./build. The only other thing I would say is give the option to use system libraries as well. There are distros that ship ffmpeg's and libass's newer than 5 years ago, and with sane configuration options. The default should certainly be to build new ones but the user should get the option. I think you have that covered with <projects-list>, but I wanted to make sure I mention it. Otherwise I like it, and it would cover the use case I was most interested in: using a custom repository for building certain projects.

avih commented 6 years ago

in fact update-post could be a sane way of applying patches before calling ./build.

Indeed, that's one of the use cases for post. Also recall that it's optional. If you don't need these hooks, just don't ever set them and that's it. The config file user.conf could have one line in it which overrides some default value, and that's it, the rest will remain default.

The only other thing I would say is give the option to use system libraries as well

There is, just not as an option.

Simply set project_list="libass mpv" and the rest is searched at the system, in this case ffmpeg will be searched globally because anything which is not at the local build_libs is searched globally.

kevmitch commented 6 years ago

Perhaps add config__configure variables for good measure (replacing the files ffmpeg_options, mpv_options, etc.)?

Yes this was missing from the above description.

Usage: clean []

  1. Sources script $config_clean_pre clean-pre if defined and exists.

Do we really need to let the hooks have different names and locations? Can't they just be set locations that either exist or don't. If you want alternate hook scripts, you can use symlinks.

In general the hooks idea is good. It wards off feature creep by encouraging people do "do it yourself".

avih commented 6 years ago

Yes this was missing from the above description.

Indeed, I haven't mentioned it but I meant this part stays the same. I also didn't mention the debian thingy and possibly others - which I didn't plan to modify.

The modification I wanted is about update/clean/build .

Do we really need to let the hooks have different names and locations? Can't they just be set locations that either exist or don't. If you want alternate hook scripts, you can use symlinks.

Nothing is "needed", and they certainly can, but I wanted to leave the mpv-build dir as little touched by the user as possible. With my design, the only set location is <mpv-build-root>/user.conf .

Note also that all the hooks could point to the same script file, because the stage ("clean-pre" etc) is sent as an argument which the hook script can use to check and do different things. It's not unimaginable for instance that someone will want to apply patches at update-post and reset the repo on update-pre, and it would be easier to manage both at one file, but with your approach you force the user to maintain two files (you didn't mention if you want to keep the argument or not).

However, what's the reason you'd prefer set locations? Do you think it would make the code more complex? Or harder to document? or harder to use?

Generally I prefer to aim higher first. It's always possible to aim lower later with a simpler approach, but I think the approach I outlined is both simple and robust. Though clearly not the only viable approach.

kevmitch commented 6 years ago

If we went the fixed location route, we could keep the argument and rather than separate files, there could be multiple symlinks to the same file. Though there could be value in isolating absolutely every user tunable in a single file. Would this be able to execute multiple pre / post hooks?

I'll adapt the Debian scripts to whatever is implemented if necessary.

avih commented 6 years ago

Would this be able to execute multiple pre / post hooks?

Sure, in your hook script you can call others :)

avih commented 6 years ago

In general, personally I wouldn't want to modify files in a repo which I don't control. mpv-build is such repo, and you can't know what might become a conflict at the future. If we only use one set location for a user.conf file, the user can symlink it or just source their own config file which is outside the repo, so integration is cleaner IMHO.

avih commented 6 years ago

FWIW, I've implemented most/all of what I described above, and then refined it a bit. It's at my branch https://github.com/avih/mpv-build/commits/mpv-build-rewrite (warning: I might force push to this branch).

Feel free to try it out and give some feedback.

ghost commented 5 years ago

That seems to be going on for 2 years... so uh do with it whatever you want, you apparently care more than me about mpv-build. (mpv-build is pretty useless now that libass/ffmpeg distro packages are usually ok.)

avih commented 5 years ago

I'm still maintaining (when there's a need) the branch mentioned above, but I don't have immediate plans to merge back. If I ever get to that, I'll open a PR.