flathub / org.darktable.Darktable

https://flathub.org/apps/details/org.darktable.Darktable
5 stars 14 forks source link

Issue #56 - Provide a way to update the darktable lensfun database #57

Closed hfiguiere closed 4 years ago

hfiguiere commented 4 years ago

This also update the database.

Closes #56

eszlari commented 4 years ago

Why not just copy the db from a more recent upstream git commit, if you want to stay with the old release?

hfiguiere commented 4 years ago

From lensfun-update-data

Unfortunately, we have to take into account that the Git database may have a too new format for the locally installed Lensfun.

Hence we shouldn't rely on what's in git.

eszlari commented 4 years ago

If you compare the database in both versions (0.3.95 and git HEAD), you see, that all files still have the same version: <lensdatabase version="2"> (https://lensfun.github.io/manual/latest/db_versions.html).

The last version bump was 1 Jan 2016: https://github.com/lensfun/lensfun/commit/11e3a27bbfcfce304921704d20b4a0aaec96fd5f

hfiguiere commented 4 years ago

But what about the future? What if tomorrow they bump it?

eszlari commented 4 years ago

Then we will just switch to git master?

paperdigits commented 4 years ago

@eszlari we don't want to buld git master though.

hfiguiere commented 4 years ago

Then we will just switch to git master?

No, unless upstream does it.

The lack of recent release of lensfun and the fact that Darktable uses 0.3.95 (which, I read, was not the best idea) is already suboptimal.

eszlari commented 4 years ago

Does it actually make a difference in practice? Is it unstable / buggy?

There are other projects that don't like to make releases and where it is no problem to use git master. Also looking at the recent commits: most of them seem to be changes to the database, not the library, which is confirmed by this comment:

While there are many people regularly working on the calibrations, there is currently no active developer for the library. So I have to say AFAIK there is no schedule or plan to make a new release.

bilelmoussaoui commented 4 years ago

I don't think that including all the files is a suitable solution imho, why not just bump to the latest working commit for now and see when there's a new release of the library.

nanonyme commented 4 years ago

If copying from git, consider adding a simple data validator though which checks version since it's in the XML files. Then you can easily see when compat version changes.

paperdigits commented 4 years ago

Can we not specify the zip file as a simple archive in the manifest and unzip it to a specific location?

On May 6, 2020 8:09:35 AM PDT, "Hubert Figuière" notifications@github.com wrote:

@eszlari we don't want to buld git master though.

No, unless upstream does it.

The lack of recent release of lensfun and the fact that Darktable uses 0.3.95 (which, I read, was not the best idea) is already suboptimal.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/pull/57#issuecomment-624707039

hfiguiere commented 4 years ago

Which zip?

The output of lensfun-update-data is a bunch of files, that the update script here will put into a tarball so it is easier to manage with the build process of flatpak. Also the tarball is in the repo. I could avoid checking the db files in, but this is the only reliable way I found to see if there are actual changes to the database since a tarball would be different with different modification time of any of its content. There is no point in doing a new build if the data didn't change.

And lensfun-update-data vs what's in git I'll remind that the former is the sanctionned way to update the data for a distribution without updating the library. Which is why I picked that route.

Now to end the bike shedding, I want the shed colour to be fire engine red.

hfiguiere commented 4 years ago

BTW I could just merge the PR right now. I just wanted to make sure this was not done the wrong way.

paperdigits commented 4 years ago

I'm sorry you feel this is bikeshedding, I feel like it is a legitimate discussion.

On May 6, 2020 11:28:29 AM PDT, "Hubert Figuière" notifications@github.com wrote:

Which zip?

The output of lensfun-update-data is a bunch of files, that the update script here will put into a tarball so it is easier to manage with the build process of flatpak. Also the tarball is in the repo. I could avoid checking the db files in, but this is the only reliable way I found to see if there are actual changes to the database since a tarball would be different with different modification time of any of its content. There is not point in doing a new build if the data didn't change.

And lensfun-update-data vs what's in git I'll remind that the former is the sanctionned way to update the data for a distribution without updating the library. Which is why I picked that route.

Now to end the bike shedding, I want the shed colour to be fire engine red.

-- You are receiving this because you commented. Reply to this email directly or view it on GitHub: https://github.com/flathub/org.darktable.Darktable/pull/57#issuecomment-624814520

bilelmoussaoui commented 4 years ago

BTW I could just merge the PR right now. I just wanted to make sure this was not done the wrong way.

I do think we should wait until we come up with a proper solution instead of just wanting to fix the issue for users. See https://github.com/flathub/flathub/wiki/App-Maintenance#required-files

cc @barthalion @TingPing

paperdigits commented 4 years ago

Can we specify lensfun-db.tar.bz2 (the zip file, sorry for the colloquialism) as a source file in the flatpak manifest, then decompress it to the proper place in the flatpak repo?

bilelmoussaoui commented 4 years ago

Can we specify lensfun-db.tar.bz2 (the zip file, sorry for the colloquialism) as a source file in the flatpak manifest, then decompress it to the proper place in the flatpak repo?

Yes, that's perfectly doable and flathub-external-checker can update it automatically as well

hfiguiere commented 4 years ago

I'm trying to provide the best to have an update database in the flatpak so that:

I do not see other alternative even in the suggestion in this thread.

hfiguiere commented 4 years ago

Can we specify lensfun-db.tar.bz2 (the zip file, sorry for the colloquialism) as a source file in the flatpak manifest, then decompress it to the proper place in the flatpak repo?

This is already what is being done.

But the proiblem is that the tar.bz2 file was generated by the update script I mention because there is no such thing available otherwise.

hfiguiere commented 4 years ago

Can we specify lensfun-db.tar.bz2 (the zip file, sorry for the colloquialism) as a source file in the flatpak manifest, then decompress it to the proper place in the flatpak repo?

Yes, that's perfectly doable and flathub-external-checker can update it automatically as well

Except that this file is not available for download.

paperdigits commented 4 years ago

Except that this file is not available for download.

Yes it is: https://wilson.bronger.org/lensfun-db/version_2.tar.bz2

Gnome dictionary does this:

{
            "name": "icons",
            "buildsystem": "simple",
            "build-commands": [
                    "install -Dm644 accessories-dictionary-24.png /app/share/icons/hicolor/24x24/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-32.png /app/share/icons/hicolor/32x32/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-48.png /app/share/icons/hicolor/48x48/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-64.png /app/share/icons/hicolor/64x64/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-128.png /app/share/icons/hicolor/128x128/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-256.png /app/share/icons/hicolor/256x256/apps/accessories-dictionary.png",
                    "install -Dm644 accessories-dictionary-512.png /app/share/icons/hicolor/512x512/apps/accessories-dictionary.png"
            ],
            "sources": [
                {
                    "type": "file",
                    "path": "accessories-dictionary-24.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-32.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-48.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-64.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-128.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-256.png"
                },
                {
                    "type": "file",
                    "path": "accessories-dictionary-512.png"
                }
            ]
        },

Could we do similar?

paperdigits commented 4 years ago

Perhaps something like this

{
            "name": "lensfun-db",
            "buildsystem": "simple",
            "sources": [
                {
                    "type": "archive",
                    "url": "https://wilson.bronger.org/lensfun-db/version_2.tar.bz2",
                    "sha256": "something-something"
                }
            ],
            "build-commands": [ "cp /extracted-dir/ /correct/path/to/lensfun-db/" ]
        },
hfiguiere commented 4 years ago

Except that this file is not available for download.

Yes it is: https://wilson.bronger.org/lensfun-db/version_2.tar.bz2

  1. I didn't find an indication of where to find it (I might have missed it, though)
  2. this is just a tarball for which we don't even know which version it is based on the URL, which mean that expecting to download it and match the sha256 will randomly break the build, and make the build not reproducible since for a specified revision of the git repository for the flatpak the build result could be different.

Gnome dictionary does this:

Did you look at the .json in this PR?

Could we do similar?

You mean copy 60 files by hand, install a tarball?

hfiguiere commented 4 years ago
              "type": "archive",
              "url": "https://wilson.bronger.org/lensfun-db/version_2.tar.bz2",
              "sha256": "something-something"

So the day the tarball change, the build fail and there is no way to do it again.

paperdigits commented 4 years ago

So the day the tarball change, the build fail and there is no way to do it again.

Yes so we update the sha256 and build again.

bilelmoussaoui commented 4 years ago

I'm trying to provide the best to have an update database in the flatpak so that:

* it's 100% reliably reproductible

* we don't break things deliberately like by pulling the master branch of the dependency library

* we happen to do the officially sanctionned way by upstream, like a distro would do

* it doesn't unecessarily rebuild things

I do not see other alternative even in the suggestion in this thread.

            "type": "archive",
            "url": "https://wilson.bronger.org/lensfun-db/version_2.tar.bz2",
            "sha256": "something-something"

So the day the tarball change, the build fail and there is no way to do it again.

That's the exact reason why we have flathub-external-data-checker. If the url of an application that we can't re-distribute directly, the bot updates the URL and their checksums each hour automatically and send/merge the pull requests as well.

hfiguiere commented 4 years ago

That's the exact reason why we have flathub-external-data-checker. If the url of an application that we can't re-distribute directly, the bot updates the URL and their checksums each hour automatically and send/merge the pull requests as well.

But this is not the case of something that can't be redistributed. This is something that can be checked, redistributed, etc.

I'll ask the question. What is wrong with the PR? So far I only got told there are alternative for which none of the parameters I listed are fullfilled.

This solution even fits the parameters list on the app maintenance wiki page since this is handling a specific problem for a specific solution. I really wish it had been easier, but I haven't found a better way.

bilelmoussaoui commented 4 years ago

That's the exact reason why we have flathub-external-data-checker. If the url of an application that we can't re-distribute directly, the bot updates the URL and their checksums each hour automatically and send/merge the pull requests as well.

But this is not the case of something that can't be redistributed. This is something that can be checked, redistributed, etc.

I'll ask the question. What is wrong with the PR? So far I only got told there are alternative for which none of the parameters I listed are fullfilled.

This solution even fits the parameters list on the app maintenance wiki page since this is handling a specific problem for a specific solution. I really wish it had been easier, but I haven't found a better way.

In such case, you can just generate those files in a separate repository, create a tarball and use it from there if it can be redistributed.

eszlari commented 4 years ago

What is wrong with the PR?

It's an over engineered solution to a problem that doesn't (yet) exist.

The manifests on Flathub should strive to be maintainable. What if tomorrow, you are hit by a bus? I think everybody should be able to understand a manifest as easy as possible and submit PRs.

Just use a git commit and add a comment to the manifest like:

/* check lensdatabase version */
hfiguiere commented 4 years ago

The manifests on Flathub should strive to be maintainable. What if tomorrow, you are hit by a bus?

They'd be people in deeper trouble than a flathub repository.

Also there is a README.md in the PR for that exact reason. If anything is unclear in it, I'd love to fix it.

Anyway it seems that is consensus that you don't like it. Feel free to reopen reuse if you feel there is anything usable.