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

snap/ubuntu fixes #87

Merged
merged 4 commits into from Feb 15, 2021
Merged

snap/ubuntu fixes #87

merged 4 commits into from Feb 15, 2021

Conversation

iainlane
Copy link
Collaborator

@iainlane iainlane commented Feb 12, 2021

Hope it's ok to bundle these up...

Couple of things here

  • Fixing that crash we saw on Ubuntu runs, which was due to a bug when we close a package then go on to reopen it later.
  • Some snap build fixes that were needed when I tried a newer snapcraft. Please could you try building it in your way, to make sure I didn't break that... (maybe the CI will do this)

src/asgen/zarchive.d Outdated Show resolved Hide resolved
@ximion ximion force-pushed the master branch 12 times, most recently from 168fd1c to 0bd88bf Compare February 12, 2021 22:02
@ximion
Copy link
Owner

ximion commented Feb 12, 2021

Except for that nitpick, this looks good to me! It's very odd that these methods in DebPackage had no locking whatsoever - likely the package was only to be used from one thread originally, until we started hunting down icons all over the place.
Thank you for the debugging effort and Snap-modernization!

Iain Lane added 4 commits February 15, 2021 12:33
… temporary dir

When downloading remote packages, we place them into a package specific
temporary directory. This is so we can remove them once we think we're
done, so that peak space usage is minimised.

We decide if a package is downloaded or not by the 'local' filename
being set. If it's set, we return this path to callers and then they can
try to operate on it. The problem is, we were failing to clear this
variable when removing the temporary directory in some circumstances.
That was meaning that a stale path could be given out to callers, a path
to a file that doesn't exist any more.

Fix this by always clearing the local path when removing the temporary
directory. That means that the next time the file is requested it will
be re-downloaded if this is a remote path.
These operations are most thread unsafe; they are dealing with the
underlying filesystem. Make sure they are synchronized.
libarchive stores errno with the actual cause of the error. Print it.
Newer versions of snapcraft seem to have changed the cwd of the build
step. It's more robust to do this by absolute path anyway, so do that.

A couple of parts of the build system want fixing for that-

  - Run the tests from the root of the source tree, and
  - Find data assets relative to the cwd

Another alternative would be to do something like
`G_TEST_{BUILD,SRC}DIR` that GLib has - environment variables that can
be defined to help tests find their assets. But this works OK for now.

Also run the sedding on the .gir file in override-pull; if the build
step is run twice then this results in messed up files otherwise.
@ximion ximion merged commit bea4440 into ximion:master Feb 15, 2021
@iainlane iainlane deleted the snap-ubuntu-fixes branch February 15, 2021 18:14
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.

None yet

2 participants