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
Add Ubuntu backend with langpack support #18
Conversation
| scope(exit) archive_read_free (ar); | ||
|
|
||
| dest.chdir; | ||
| scope(exit) cwd.chdir; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libarchive's API always extracts to the working directory. There's some thread safety problems here if things use relative paths. I only found one problem there in practice, and made it use an absolute path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's why I never use the disk writer of libarchive - actually, the archive decompressor classes should have all you need already.
There is also on extract-by-regex function which you could probably use (would need a flag to preserve directory structures, I think)
| @@ -212,7 +213,7 @@ class Config | |||
|
|
|||
| JSONValue root = parseJSON (jsonData); | |||
|
|
|||
| workspaceDir = dirName (fname); | |||
| workspaceDir = dirName (fname).absolutePath; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we always access things in the workspace directory as an absolute path, then (at least some of) the thread safety problems with libarchive don't happen.
b941b06
to
4b5a814
Compare
|
Pushed the fixes - please review! In the next iteration, if you can help me include |
| @@ -21,7 +21,7 @@ | |||
| "targetType": "executable", | |||
| "buildRequirements": ["disallowDeprecations"], | |||
|
|
|||
| "dflags-gdc": ["-Wl,--push-state,-no-as-needed", "-lcurl", "-Wl,--pop-state"], | |||
| "dflags-gdc": ["-Wl,--push-state,-no-as-needed", "-lcurl", "-Wl,--pop-state", "-L/home/laney/dev/canonical/release/appstream/build/src"], | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eh... ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On 30 August 2016 16:41:23 BST, Matthias Klumpp notifications@github.com wrote:
@@ -21,7 +21,7 @@
"targetType": "executable",
"buildRequirements": ["disallowDeprecations"],
- "dflags-gdc": ["-Wl,--push-state,-no-as-needed", "-lcurl",
"-Wl,--pop-state"],- "dflags-gdc": ["-Wl,--push-state,-no-as-needed", "-lcurl",
"-Wl,--pop-state",
"-L/home/laney/dev/canonical/release/appstream/build/src"],Eh... ;-)
heheh
| @@ -13,6 +13,7 @@ RUN apt-get install -yq git gcc gdc | |||
| RUN apt-get install -yq --no-install-recommends \ | |||
| cmake \ | |||
| intltool \ | |||
| libstemmer-dev \ | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed anymore, I updated the dependencies in master (and dropped most of them).
| @@ -595,7 +589,6 @@ unittest | |||
|
|
|||
| writeln ("TEST: ", "Extracting a tarball"); | |||
|
|
|||
| import bindings.asgenstdilb : mkdtemp; | |||
| import std.file : buildPath, tempDir; | |||
|
|
|||
| auto archive = buildPath (getcwd (), "test", "samples", "test.tar.xz"); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use getTestSamplesDir() here.
|
One more time? |
We want to do this for langpacks.
The first time that we encounter a package with an X-Ubuntu-Gettext-Domain key in its desktop file, we extract the langpacks and generate the locales that they reference. Then we can query these translations and stuff them into the component.
|
Should I wait with merging for libmo to be ready, or would it make sense for you to have this in master now? |
|
On 12 September 2016 05:32:37 WEST, Matthias Klumpp notifications@github.com wrote:
It would be nice for it to be merged, since I don't know when limbo will be mergable-it needs some fixes to submodule handling in meson first. |
|
Okay, so I think I will merge it today or tomorrow then and fix every nitpick I find afterwards :-) |
The Ubuntu backjend just inherits the Debian one and does a few extra things on top.
Improvement suggestions welcome - I'm sure I messed up the style in some places, and the approach feels like it could be cleaner somehow although I don't quite know what to do differently right now.
In particular I would have like to make the hook thing more general, but there's so much state that it's hard to know how to do that.
I'll make some other more specific comments on the diff.