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

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

Closed
wants to merge 1 commit into from
Closed

Conversation

hfiguiere
Copy link
Collaborator

This also update the database.

Closes #56

@eszlari
Copy link
Contributor

eszlari commented May 6, 2020

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

@hfiguiere
Copy link
Collaborator Author

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
Copy link
Contributor

eszlari commented May 6, 2020

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:
lensfun/lensfun@11e3a27

@hfiguiere
Copy link
Collaborator Author

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

@eszlari
Copy link
Contributor

eszlari commented May 6, 2020

Then we will just switch to git master?

@paperdigits
Copy link
Collaborator

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

@hfiguiere
Copy link
Collaborator Author

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
Copy link
Contributor

eszlari commented May 6, 2020

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
Copy link
Member

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
Copy link

nanonyme commented May 6, 2020

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
Copy link
Collaborator

paperdigits commented May 6, 2020 via email

@hfiguiere
Copy link
Collaborator Author

hfiguiere commented May 6, 2020

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
Copy link
Collaborator Author

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

@paperdigits
Copy link
Collaborator

paperdigits commented May 6, 2020 via email

@bilelmoussaoui
Copy link
Member

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
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Collaborator Author

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.

@hfiguiere
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

  			"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
Copy link
Collaborator

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Member

bilelmoussaoui commented May 6, 2020

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
Copy link
Contributor

eszlari commented May 6, 2020

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
Copy link
Collaborator Author

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.

@hfiguiere hfiguiere closed this May 6, 2020
@hfiguiere hfiguiere deleted the lensfundb branch May 13, 2020 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updating the lens fun database
5 participants