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

Should have a way to specify default branch for repository #221

Closed
cosimoc opened this issue Jul 30, 2016 · 21 comments
Closed

Should have a way to specify default branch for repository #221

cosimoc opened this issue Jul 30, 2016 · 21 comments

Comments

@cosimoc
Copy link
Contributor

cosimoc commented Jul 30, 2016

Currently there's no way to specify a repository default branch when adding it.
While flatpak command line tools allow the user to specify the branch when installing an app from a repo that has it available on multiple branches, app stores like gnome-software would benefit from the concept of a default branch for a repository to help make that choice.
A nice implementation could involve having an e.g. xa.default_branch attribute on the repository summary file, together with the ability to specify/override the default branch in the repository configuration file when it's added.

@matthiasclasen
Copy link
Collaborator

Makes sense to me

@alexlarsson
Copy link
Member

Also, in such a setup it would make sense to not generate the appstream data for the non-default branches, as it is rarely useful to show the same info multiple times.

@alexlarsson
Copy link
Member

In particular, this is true if one is using extra branches to "emulate" tags of older versions.

@hughsie
Copy link
Collaborator

hughsie commented Aug 29, 2016

If you do include multiple tags with the same and different tags then gnome-software does the right thing in 3.22. We'll even require this if we implement Allans multiple-version mockups for 3.24.

@alexlarsson
Copy link
Member

Yeah, but downloading the appstream xml for those is kinda useless.

@dbnicholson
Copy link
Contributor

In our setup, i think we do want to create the appstream data for all the app and runtime refs even though some of them are duplicates since we want to allow developers to specify different default branches. Our development repo has both the development and pre-stable refs in it, of you follow what I mean.

What I was going to do was add an xa.default-branch key to the summary and repo config like the other keys. Does that make sense?

@alexlarsson
Copy link
Member

ostreedev/ostree#523

@cosimoc
Copy link
Contributor Author

cosimoc commented Oct 5, 2016

@alexlarsson Is that PR above really related to this issue?

@alexlarsson
Copy link
Member

No, i posted that on the wrong issue.

Anyway, git master has this now as a remote property, and a new DefaultBranch field in .flatpakrepo files.

It doesn't yet support getting default values from the summary file, as there is no nice place atm to add that in the code. We should look into adding that.

@cosimoc
Copy link
Contributor Author

cosimoc commented Oct 6, 2016

@alexlarsson Would it be a good idea to do that from flatpak-build-update-repo, similarly to what we do with the title property?

@cosimoc
Copy link
Contributor Author

cosimoc commented Oct 6, 2016

Oh, I see what you mean now that it does not support getting the default value from the summary file. gnome-software does it by default now in git master [1] and it would be nice to do the same when installing things from the repository. Is that what you meant?

[1] https://git.gnome.org/browse/gnome-software/commit/?id=5fa03af8e7f1ff60cc4af81a91d1a105cacf545a

@mariospr
Copy link
Member

@cosimoc I think both things are needed: in one hand you would need to extend build-update-repo to pass an additional --default-branch parameter to include that in the summary file in the server side as an extended attribute, and then you'd need use that value from the clients for common tasks when more than one branch is available, such as installing and app via the command line, or querying the default branch via the API (i.e. what GNOME Software would do).

Does this make any sense?

@cosimoc
Copy link
Contributor Author

cosimoc commented Oct 10, 2016

@mariospr Yeah, makes sense to me.

@mariospr
Copy link
Member

@cosimoc Thanks for the feedback. I've been looking into the code in flatpak and from what I can see flatpak is at the moment ignoring the second dictionary retrieved from the remote summary file (see OSTREE_SUMMARY_GVARIANT_FORMAT), which is where the extra metadata such as xa.title and now xa.default-branch would live.

I understand that we therefore need to extend this so that flatpak retrieves that info and takes it into account (maybe a find_remote_default_branc() function called from flatpak_dir_find_remote_ref(), or the like), so I'm working on that now and will continue tomorrow.

And interesting question that comes to my mind at this point, even if a bit off topic, is whether we shouldn't be retrieving the xa.title from the summary file too at some point, since at the moment calls to flatpak_remote_get_title() from the public API will only return the title of a repo if it's in the local configuration, ignoring whatever is defined in the remote server. Should flatpak also handle that?

@mariospr
Copy link
Member

Pull request here: #345

@mariospr
Copy link
Member

Open open question that is not contemplated in the current PR, though, before I forget about it: how should we deal with post remote-add updates to the default-branch in the summary file?

An option could be to check whether the default-branch key has been added to the local repo configuration every time an install operation is performed, and do nothing if the key is already there, but that won't work well for cases where we'd want to be able to push updates related to what the current default branch should be from the server as it would override any local configuration we might have defined (e.g. devs might want to have the default branch pointed to an unstable branch).

Or perhaps it's ok to assume the default-branch can get overriden locally for every install/update operation, assuming that repositories providing unstable/development versions of flatpaks will simply not define a default-branch at all?

@mariospr
Copy link
Member

mariospr commented Oct 12, 2016

Just pushed an additional commit that basically checks the remote server when installing a flatpak to see if a default branch has been defined there, pulling it if that's the case and has not previously been configured locally: 69b6400

@dbnicholson I think this should address your concerns in #345 (comment): this makes it possible to "push" default branches from the server side, while still making local overrides possible

@dbnicholson
Copy link
Contributor

I think what you have there is the most logical as it leaves the local config file or client supplied branch in charge. I am a bit worried about how can't automatically upgrade all users from the server side since the remote summary's default branch has the lowest precedence. However, maybe that's really a job for GNOME Software - if the remote summary default branch differs from the local default branch, offer to upgrade.

@mariospr
Copy link
Member

I think what you have there is the most logical as it leaves the local config file or client supplied branch in charge.

Thanks for the feedback, Dan.

I am a bit worried about how can't automatically upgrade all users from the server side since the remote summary's default branch has the lowest precedence. However, maybe that's really a job for GNOME Software - if the remote summary default branch differs from the local default branch, offer to upgrade.

With the current proposal I have now updated on the PR, the idea is that we would not automatically update (at least not for now) the local configuration with the info from the summary file on every install operation. Instead, I'm proposing now a new CLI command, remote-update, and a new public API for libflatpak that would allow us to do this update on demmand.

This way, we could code any logic we consider fit anywhere else (e.g. GNOME Software, update scripts...) to decide when and how we want to update the local configuration from the server.

See more details, and links to the new proposed patches here: #345 (comment)

@mariospr
Copy link
Member

Change my mind again: let's add a simple --update-metadata parameter to flatpak remote-modify instead of creating yet another command for something like this: 75b83a8

@alexlarsson
Copy link
Member

This is in now, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants