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

Fix bundle install issues try2 #584

Merged
merged 3 commits into from
Oct 31, 2015

Conversation

tchx84
Copy link
Member

@tchx84 tchx84 commented Oct 11, 2015

No description provided.

The BundleRegistry presented different issues which are related to the use of
GFileMonitor to detect bundles installation by external programs.

One example, as mentioned in the mailing list [1], is sugar-install-bundle,
where its execution time was drastically slowed down due to the changes added
in a previous commit [2].

Another example, as mentioned in this ticket [3], where the registry misses to
install some bundles. This occurs because the GFileMonitor detects the bundle
root directory creation before the contents become available, e.g, it cannot find
the .info file,sSo the bundle gets ignored, never installed or never updated.

Due to problems like these we concluded [1] that we need an explicit way to tell
the registry to install or upgrade a bundle.

This patch adds ATTRIBUTE_CHANGED event to the GFileMonitor callback, along with the
CREATED event, so external scripts can trigger the bundle installation by doing just
one extra file system operation e.g, "touch /path/to/bundle". This simple solution
could be used by any external script installing bundles, from sugar-install-bundle
to packaging post-install scripts.

[1] http://lists.sugarlabs.org/archive/sugar-devel/2015-August/050699.html
[2] 4b4b2fba1c37a9ad92ed30eb669b68552b62415
[3] https://bugs.sugarlabs.org/ticket/4841#comment:17

Signed-off-by: Martin Abente Lahaye <tch@sugarlabs.org>
The BundleRegistry will detect this event and will install or update
the bundle. This fixes previous issues with BundleRegistry ignoring
bundle installation via sugar-install-bundle.

Signed-off-by: Martin Abente Lahaye <tch@sugarlabs.org>
@quozl
Copy link
Contributor

quozl commented Oct 12, 2015

Reviewed.

Applied patches.

Tested that no other source of ATTRIBUTE_CHANGED events occurs for the path.

Tested that a touch of bundle directory generated an ATTRIBUTE_CHANGED event.

Tested bulk removal and reinstall of all activities, using a list of packages to avoid dependencies: http://dev.laptop.org/~quozl/z/1ZlRXk.txt

Many activities were missing from the home view after reinstall. See shell.log: http://dev.laptop.org/~quozl/z/1ZlRWq.txt Calculate was missing.

Let me know if you need more testing, thanks!

@tchx84
Copy link
Member Author

tchx84 commented Oct 12, 2015

@quozl Thanks for testing. This touch-to-ensure-installation method is something new, so the packages will require to "touch" these directories, e.g, in a post-install script, to make use of it. (EDIT: sugar-install-bundle is not used at all in packages life-cycle).

@quozl
Copy link
Contributor

quozl commented Oct 12, 2015 via email

@samdroid-apps samdroid-apps added this to the 0.108 Features milestone Oct 16, 2015
@samdroid-apps
Copy link
Contributor

@quozl, what method did you use to test this?

@quozl
Copy link
Contributor

quozl commented Oct 16, 2015

  • be sure the home view activity ring is visible, and count the number of icons,
  • remove and install a large set of activity packages,
apt remove $(<packages-list)
apt install $(<packages-list)
  • count the number of icons in the ring,
  • check the shell.log

But don't bother testing, it won't be effective until the Debian and Ubuntu packages catch up with the feature. It will need to be added to release notes; that Sugar will refresh the application menus when the activity directory is touched, so a postinst should do that.

Alternatively, change a Debian or Ubuntu package to add the touch, and test with that.

@tchx84
Copy link
Member Author

tchx84 commented Oct 16, 2015

@quozl, yes, ill prepare Feature entry in the wiki.

@tchx84
Copy link
Member Author

tchx84 commented Oct 21, 2015

@samdroid-apps @quozl I just added a new entry to our features list in [1].

[1] http://wiki.sugarlabs.org/go/0.108/Feature_List

@tchx84 tchx84 added the feature label Oct 21, 2015
@quozl
Copy link
Contributor

quozl commented Oct 21, 2015

Thanks!

@quozl
Copy link
Contributor

quozl commented Oct 21, 2015

@samdroid-apps, this is ready for merging?

@samdroid-apps
Copy link
Contributor

@quozl did you test it?

@quozl
Copy link
Contributor

quozl commented Oct 30, 2015

@samdroid-apps, yes, as described above.

@quozl
Copy link
Contributor

quozl commented Oct 31, 2015

Thanks for the fix, I've cherry-picked it for an OLPC fork.

@samdroid-apps samdroid-apps merged commit f49a4a9 into sugarlabs:master Oct 31, 2015
@jvonau
Copy link
Contributor

jvonau commented Nov 23, 2015

There is no way for advanced users to really test this live on a currently released fedora without sugar-build in the picture and that colors the result because s-b is build with devel packages and not the more stripped down released packages. RPMS need to declare dependencies, sugar-build and olpc-os-builder are great tool to hide the true dependence chain. Someone needs to get 0.107 in fedora rawhide so others can test on other platforms. I suggested that SL could create official COPR builds for testing on the mailing list or would S/L prefer me to use my personal COPR account and contact the mailing list so others can test?

@quozl
Copy link
Contributor

quozl commented Nov 23, 2015

@jvonau, yes, sugar-build not useful for such tests. Yes, 0.107 in Fedora would be another test method. I'm not planning that myself. Too many other problems preventing my builds from proceeding with Fedora.

This patch is interesting to me because of my Ubuntu build, where every activity is packaged, where automatic upgrades occur, and where a new package kit control panel updater is being tested. That updater may be of interest to the wider Fedora Sugar community, but not to olpc-os-builder builds which use the activity updater that works on unpackaged activities, using bundles.

If you cannot get interest for rawhide, a manual packaging of 0.107 is a good first approximation.

@jvonau
Copy link
Contributor

jvonau commented Nov 24, 2015

@quozl When did OOB start patching out activity updater that is shipped with sugar since 0.100? That was true in the distant past, and I used that reasoning to support my idea of splitting the spec file into the current layout way back when.

@quozl
Copy link
Contributor

quozl commented Nov 24, 2015

@jvonau, don't know what you mean sorry, can you give more detail? My previous comment edited to reduce ambiguity.

@jvonau
Copy link
Contributor

jvonau commented Nov 24, 2015

Well if I recall correctly at one point sugar-update-control rpm was added
to the XO builds and sugar's updater was patched out in the spec file until
dsd merged the microformat stuff into sugar for the automatic updates.

On Tue, Nov 24, 2015 at 1:49 AM, James Cameron notifications@github.com
wrote:

@jvonau https://github.com/jvonau, don't know what you mean sorry, can
you give more detail? My previous comment edited to reduce ambiguity.


Reply to this email directly or view it on GitHub
#584 (comment).

@quozl
Copy link
Contributor

quozl commented Nov 24, 2015

@jvonau, sugar-update-control isn't in the OLPC XO builds now.

In the Sugar source there are three activity update backends; ASLO, new ASLO, and microformat. GSettings path org.sugarlabs.update backend selects which to use. The default in Sugar is ASLO.

In the Fedora-style packaging for OLPC OS on XO laptops, using Fedora 18, this is left unchanged. But in olpc-os-builder v7.0 branch the backend is configured to microformat. To provide control of activities presented for update.

In the Debian-style packaging for OLPC Ubuntu Sugar on other laptops, thanks to @tchx84, this is patched in a similar fashion tchx84/debian-pkg-sugar@fb84bf0 but the list is empty. Instead, a meta-package is used to select the activities from a package archive. We also have an aptdaemon based updater https://github.com/tchx84/sugar-apt-updater (not package kit as I had said before).

For one or more OLPC OS releases, probably 13.2.3, the ASLO updater didn't work due to a bug.

I don't see the connection to this merged pull request though, can you explain? This pull request was to fix a bug that happens with Debian-style package installs while Sugar is running. I don't know if it also occurs with Fedora-style packaging, and don't really care. 😁

@jvonau
Copy link
Contributor

jvonau commented Nov 25, 2015

@quozl I know most of the history on sugar-update-control and microformat since 10.3.1 https://dev.laptop.org/ticket/11064 so lets not try to lecture me on the basic history. Ok? Having a hand in olpc-os-builder's use of sugar-install-bundle via https://dev.laptop.org/ticket/10427 results in me having a passive interest in the subject. Good that you have Debian-style packaging in the works, I'm keeping an eye on SoaS and thus Fedora where the ASLO backend appears to be working at the moment even with the rpm based activities in place. However testing anything is a pain, as a volunteer I chose not direct my time unless it's easy to try out, and I not alone in that way of thinking. Yea OT here but it's kind of sad to see Fedora fall by the wayside given it was the birthplace of sugar.

@quozl
Copy link
Contributor

quozl commented Nov 25, 2015

@jvonau, sorry if it appeared to be lecturing, it wasn't my intent. My purpose was to make sure you and I had the same facts in the discussion. I really don't remember. It's great that you do. And in our encounters we continue to clash because of what I forget and you remember. But for me, every time, I have to go research. I welcome more involvement of Fedora experts in our OLPC OS builds, but frankly that hasn't happened yet. I'm all alone with very little chance of fixing more problems than I can count.

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

Successfully merging this pull request may close these issues.

None yet

4 participants