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

Updating the build process #60

Closed
wants to merge 5 commits into from
Closed

Updating the build process #60

wants to merge 5 commits into from

Conversation

poke
Copy link
Member

@poke poke commented May 14, 2012

Hey @nmaier, I have been working on improving the build process for myself lately and since I have been running very well with them, I wanted to share them so we maybe get them merged in. However as you are still the one that makes the final builds, I wanted to make sure that it won’t break the process for you; hence this code review request.

The changes started with simple adjustments to get Python 3 support in the scripts, as that’s what I’m primarily using. Since I didn’t want to run Ant for every single change, I hooked up the plugin generation within my editor. One of those unproblematic changes already made it into master: 3775e7c.

As I was working on the plugin generation scripts I also made some improvements which justified a separate file, as I was also merging it with the filter generation (to run through the plugins only once).

While doing that I also changed the way the filters are saved. Originally it would only write them to the generated xpi/ folder, but as the combined plugins are globally stored as well, I thought it would make more sense to do that for the filters as well, and provide these default preferences to the development environment as well.

One other thing was to speed up the “snapshot” generation for the nightlies I have been publishing to the repository with most changes. I’ve been able to completely integrate that into the Ant build though, which now gets the SHA1 from HEAD and makes a nice file name (and hopefully works across platforms).

And finally, I made the editing process for sandboxes way easier. It always bothered me having to decode and encode sandbox plugins for every change, although I’ve already been doing that with a JavaScript tool to speed it up. Now we simply have to change the JavaScript files within the sandboxes/ directory and run the build.

Again, these are just suggestions on improving the build process in general. If you have any problems with anything, I’m fine to keep using those tools for me personally, but I thought it would make sense to make it available for everybody. If you have further suggestions or want me to adjust some things (especially if something breaks), then of course I’ll be happy to adjust those in whatever way necessary.

Anyway, thanks for listening, and I’m happy to hear feedback :)

poke added 5 commits May 14, 2012 12:24
New target automatically creates an unsigned XPI with the SHA1 value of
the current Git HEAD in the file name. This eases the upload process for
manual nightlies in the GitHub repository easier.
Removed redundancy by combining both the filter generation and the
plugin combination. That way the plugins only need to be read once.

New script is also Python 3 compatible, and is tested on both Python 2.7
and 3.2. Both yield the same results, apart from unimportant ordering
differences due to the `mergeex` module.

Ant build script was amended to use the new script.
Plugin build script generates the filters preference defaults. That way
the development directory also offers default preference values for the
filters.

- Build task updated to copy the generated file instead of redirecting
  the output.
- Gitignore updated to ignore the additionally generated file.
Sandbox scripts are stored within `/plugins/sandboxes/` and are
automatically converted and merged into the plugin files. This allows an
easier editing process for built-in sandboxes as no manual decoding and
encoding process is required. All sandbox scripts can be edited directly
in JavaScript source files without worrying about the JSON structure or
its rules.

Script is Python 3 compatible and tested in Python 2.7 and Python 3.2.

Sandbox scripts are automatically assigned to a plugin `<name>.json` if
their file names are either `<name>.process.js` or `<name>.resolve.js`.
The `process` or `resolve` part decides as which plugin parameter the
script is set. It is possible to specify both for a single plugin.
All sandbox plugins have their `process` or `resolve` script extracted
into a separate sandbox script within the `sandboxes/` directory. That
way they will be considered for the sandbox generation build.
@ghost ghost assigned nmaier May 14, 2012
@nmaier
Copy link
Member

nmaier commented May 14, 2012

Except for the nit this LGTM

@nmaier
Copy link
Member

nmaier commented May 14, 2012

Oh, and your git-author seems unknown

@poke
Copy link
Member Author

poke commented May 14, 2012

Except for the nit this LGTM

Good to hear – whatever “nit” exactly means though ^^ – I’ll fix and push that then later.

Oh, and your git-author seems unknown

That’s weird? Looking at the commits it does show the correct author content though, and GitHub recognizes me as well. Maybe there is something wrong on your end?

@poke
Copy link
Member Author

poke commented May 14, 2012

Oh, I see now what you mean! It didn’t recognize me in the pull request view. Looks like a bug though…

@poke
Copy link
Member Author

poke commented May 18, 2012

Okay, I just spotted some issue, I missed before: Because the dtaac.js preferences file contains the filter preferences as well, Firefox seems to ignore the generated filters.js and just uses the – empty – filter settings from before. I’m not sure why this does not happen in the released XPI, but it does happen when using the extension from the working directory and the build/xpi/ directory.

Looking at that made me wonder; do we actually want to include the dtaac.js file? Was there an initial reason to disable the imagefap plugin? And looking at 272f56a, I guess the addition of the filters to the dtaac.js was to have those settings for the development environment, right? Do you know why the multiple definitions work for the final XPI, but not when used as a directory?

@nmaier
Copy link
Member

nmaier commented May 19, 2012

It is mere coincidence what pref()s are parsed last. It relies on the enumeration of the "directory", may it be an OS enumeration or ZipReader enumeration. My guess is that the ZipReader returns dtaac.js, filters.js while the OS returns it the other way round (by coincidence).

You better re-merge those two files. Having a single prefs file will make the startup slightly more efficient.

The imagefap filter is disabled due to several reasons, the most important one because AMO wants it (as a concession to the no-porn rule). The only alternative is dropping that thing altogether.... However back when it was added, it was the single most requested filter...

@poke
Copy link
Member Author

poke commented May 21, 2012

Okay, I updated the commits, implemented some filter merging – which won’t work for the workspace version, as Git wouldn’t like changing the file over and over ;) – and merged that in: aa2a6bc.

However I immediately had to disable the filter generation again, as I’ll explain in a follow-up issue; this is not related to the build process update though, it already existed before, but simply never occurred.

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